-
Notifications
You must be signed in to change notification settings - Fork 209
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Refactor/parseutil #1022
Refactor/parseutil #1022
Conversation
Since the indentation has changed on |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Overall, a good move. I have a few comments in a couple of areas, however. See what you think.
@dpvc I've refactored the The |
Also should this here MathJax-src/ts/util/lengths.ts Line 130 in 8565f9d
not return 0em . The rounding function below does that. Then it would be the same as Em in ParseUtil.ts
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
See my comments below about the ParseUtil
object.
I've created the single |
@dpvc If I understand correctly, you want to be able to make changes to |
In the sense that it can be altered via the I have wanted to be able to do this with other namespaces (like the
I'm thinking that |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks good. I have one suggestion for saving some characters, but otherwise, ready to go.
Refactors the
ParseUtil
module:Map
out of unit conversion mapping and exports it. This makes it extensiblepi
).util/lengths.ts
module (see my email from yesterday).namespace
and adapts allimport
statements in modules usingParseUtil
. This is the majority of files!tests
for this for all the length methods, that I have used during conversion. Note, these already usespnpm
. To run the tests, doThe tests are in a separate commit and can be removed.