-
Notifications
You must be signed in to change notification settings - Fork 2k
Treat Infinity and NaN as reserved words #4219
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
Conversation
src/grammar.coffee
Outdated
| AlphaNumeric: [ | ||
| o 'NUMBER', -> new NumberLiteral $1 | ||
| o 'INFINITY', -> new InfinityLiteral $1 | ||
| o 'INFINITE_NUMBER', -> new InfiniteNumberLiteral $1 |
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.
I liked the old names. Let's not distinguish between Infinity and 2e308. They should each be an InfinityLiteral.
src/nodes.coffee
Outdated
| super 'NaN' | ||
|
|
||
| compileNode: -> | ||
| [@makeCode '(0/0)'] |
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.
I went with the simple approach of always wrapping in parentheses here. Do we want to only do it if strictly needed? (If so, how?)
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.
If you wanted to do it the better way, you could either create an Op node that represents the division and call compile on that, or you could just add the parens yourself by checking o.level. But this works fine for a first implementation if you just feel like leaving it. It's up to you.
|
|
||
| compileNode: (o) -> | ||
| code = [@makeCode '0/0'] | ||
| if o.level >= LEVEL_OP then @wrapInBraces code else code |
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.
I settled on using @makeCode instead of new Op since that allows NaN to be rendered as 0/0 instead of 0 / 0.
|
@michaelficarra Any more comments? :) |
|
Yeah, I had just wanted to clone it and try some things out myself. LGTM. Merging. |
Treat Infinity and NaN as reserved words
Fixes #4218.