Prettier and lint #976
Prettier and lint #976
Conversation
There was a problem hiding this comment.
I didn't look at every file, but took a sampling and commented on a few things in hopes that there is some level of control that can be used to adjust them. But if not, we can go with how this is. I wonder if the Emacs typescript mode can be configured to match this format?
The only important change I had was in the package.json file.
| "@typescript-eslint/eslint-plugin": "^6.1.0", | ||
| "eslint": "^8.45.0", | ||
| "eslint-config-prettier": "^8.8.0", | ||
| "eslint-plugin-jsdoc": "^46.4.4", | ||
| "mathjax-modern-font": "^4.0.0-beta.3", | ||
| "mhchemparser": "^4.2.1", | ||
| "mj-context-menu": "^0.9.1", | ||
| "prettier": "^3.0.0", |
There was a problem hiding this comment.
These should be dev dependencies, not plain dependencies (so they don't get installed when mathjax-full gets installed).
| align: 'top', // placement of magnified expression | ||
| backgroundColor: 'Blue', // color for background of selected sub-expression | ||
| backgroundOpacity: 20, // opacity for background of selected sub-expression | ||
| braille: false, // switch on Braille output | ||
| flame: false, // color collapsible sub-expressions | ||
| foregroundColor: 'Black', // color to use for text of selected sub-expression | ||
| foregroundOpacity: 100, // opacity for text of selected sub-expression | ||
| highlight: 'None', // type of highlighting for collapsible sub-expressions | ||
| hover: false, // show collapsible sub-expression on mouse hovering | ||
| infoPrefix: false, // show speech prefixes on mouse hovering | ||
| infoRole: false, // show semantic role on mouse hovering | ||
| infoType: false, // show semantic type on mouse hovering | ||
| keyMagnifier: false, // switch on magnification via key exploration | ||
| magnification: 'None', // type of magnification | ||
| magnify: '400%', // percentage of magnification of zoomed expressions | ||
| mouseMagnifier: false, // switch on magnification via mouse hovering | ||
| speech: true, // switch on speech output | ||
| subtitles: true, // show speech as a subtitle | ||
| treeColoring: false, // tree color expression | ||
| viewBraille: false, // display Braille output as subtitles | ||
| voicing: false, // switch on speech output | ||
| } | ||
| align: 'top', // placement of magnified expression | ||
| backgroundColor: 'Blue', // color for background of selected sub-expression | ||
| backgroundOpacity: 20, // opacity for background of selected sub-expression | ||
| braille: false, // switch on Braille output | ||
| flame: false, // color collapsible sub-expressions | ||
| foregroundColor: 'Black', // color to use for text of selected sub-expression | ||
| foregroundOpacity: 100, // opacity for text of selected sub-expression | ||
| highlight: 'None', // type of highlighting for collapsible sub-expressions | ||
| hover: false, // show collapsible sub-expression on mouse hovering | ||
| infoPrefix: false, // show speech prefixes on mouse hovering | ||
| infoRole: false, // show semantic role on mouse hovering | ||
| infoType: false, // show semantic type on mouse hovering | ||
| keyMagnifier: false, // switch on magnification via key exploration | ||
| magnification: 'None', // type of magnification | ||
| magnify: '400%', // percentage of magnification of zoomed expressions | ||
| mouseMagnifier: false, // switch on magnification via mouse hovering | ||
| speech: true, // switch on speech output | ||
| subtitles: true, // show speech as a subtitle | ||
| treeColoring: false, // tree color expression | ||
| viewBraille: false, // display Braille output as subtitles | ||
| voicing: false, // switch on speech output | ||
| }, |
There was a problem hiding this comment.
Can anything be done about these removed spaces? This is one of the few places where I think the layout has been made arguably worse.
| N, | ||
| T, | ||
| D, |
There was a problem hiding this comment.
It seems a bit unnecessary to put these on separate lines. I wonder if there is any control over this (like one-letter named could be put on the same line)?
| veryverythinmathspace: 1/18, | ||
| verythinmathspace: 2/18, | ||
| thinmathspace: 3/18, | ||
| mediummathspace: 4/18, | ||
| thickmathspace: 5/18, | ||
| verythickmathspace: 6/18, | ||
| veryverythickmathspace: 7/18, | ||
| negativeveryverythinmathspace: -1/18, | ||
| negativeverythinmathspace: -2/18, | ||
| negativethinmathspace: -3/18, | ||
| negativemediummathspace: -4/18, | ||
| negativethickmathspace: -5/18, | ||
| negativeverythickmathspace: -6/18, | ||
| negativeveryverythickmathspace: -7/18, | ||
| veryverythinmathspace: 1 / 18, | ||
| verythinmathspace: 2 / 18, | ||
| thinmathspace: 3 / 18, | ||
| mediummathspace: 4 / 18, | ||
| thickmathspace: 5 / 18, | ||
| verythickmathspace: 6 / 18, | ||
| veryverythickmathspace: 7 / 18, | ||
| negativeveryverythinmathspace: -1 / 18, | ||
| negativeverythinmathspace: -2 / 18, | ||
| negativethinmathspace: -3 / 18, | ||
| negativemediummathspace: -4 / 18, | ||
| negativethickmathspace: -5 / 18, | ||
| negativeverythickmathspace: -6 / 18, | ||
| negativeveryverythickmathspace: -7 / 18, |
There was a problem hiding this comment.
Here's another spot where the original spacing is better. I also think fractions of integers should be allowed to not have spaces around the fraction bar; 1/2 seems preferable to me than 1 / 2. But I guess I can live with it. IN that case, the tslint comment above and below can be removed.
It might be good to check for other tslint comments that can be removed as well.
| if (!match) { | ||
| return size; | ||
| } | ||
| let m = parseFloat(match[1] || '1'), unit = match[2]; | ||
| let m = parseFloat(match[1] || '1'), | ||
| unit = match[2]; |
There was a problem hiding this comment.
I suppose this should be made a separate let.
| if (UNITS.hasOwnProperty(unit)) { | ||
| return m * UNITS[unit] / em / scale; | ||
| return (m * UNITS[unit]) / em / scale; |
There was a problem hiding this comment.
Not sure why these extra parentheses are needed.
| } | ||
| if (RELUNITS.hasOwnProperty(unit)) { | ||
| return m * RELUNITS[unit]; | ||
| } | ||
| if (unit === '%') { | ||
| return m / 100 * size; // percentage of the size | ||
| return (m / 100) * size; // percentage of the size |
| return a.length !== b.length | ||
| ? b.length - a.length | ||
| : a === b | ||
| ? 0 | ||
| : a < b | ||
| ? -1 | ||
| : 1; |
There was a problem hiding this comment.
I have to say, I find this layout confusing. Shouldn't the second ? be indented, and the third indented even further?
dpvc
force-pushed
the
develop
branch
3 times, most recently
from
b77df61
to
1f851dd
Compare
This is the first formatting change, just using prettier. There are a few things that you might find shocking, for instance, how it breaks up formatting of comments (e.g, in
output/common.ts).Anyway, have a look. I am currently linting, working through errors that are easy to resolve like comments.