Skip to content
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

Bug: Scientific notation should allow uppercase E #5164

Closed
hsmyers opened this issue Feb 21, 2019 · 8 comments · Fixed by #5242
Closed

Bug: Scientific notation should allow uppercase E #5164

hsmyers opened this issue Feb 21, 2019 · 8 comments · Fixed by #5242
Labels

Comments

@hsmyers
Copy link

hsmyers commented Feb 21, 2019

Choose one: is this a bug report or feature request? This is a Bug report.

Input Code

your (code) => Z = math.complex(1.6E-100)

Expected Behavior

I expected a clean compile.

Current Behavior

coffee --map --bare --compile --output lib/ ./
C:\Users\hsmyers\Documents\LilyPond\mug\drop.coffee:65:21: error: exponential notation in '1.6E-100' must be indicated
with a lowercase 'e'
Z = math.complex(1.6E-100)
............................................^

Possible Solution

It would be better to accept either case as this would conform to both JS and normal scientific use patterns. It would also mean one less error message!

use of a case sensitive regex

modify the regex used to be case insensitive

Context

slight annoyance

Context is the normal give and take of the code/compile/deploy cycle.

Environment

  • CoffeeScript version: 2.3.2
  • Node.js version: v11.2.0
@vendethiel
Copy link
Collaborator

Ok.

@hsmyers
Copy link
Author

hsmyers commented Feb 21, 2019

BUG report.

Code:
Z = math.complex(1.6E-100)
Expected this to compile without error.
Current behavior:
error: exponential notation in '1.6E-35' must be indicated with a lowercase 'e'
Z = math.complex(1.6E-35)
Possible solution:
Take both, the rest of the world does!

C:>coffee --version
CoffeeScript version 2.3.2

C:>node --version
v11.2.0

@phil294
Copy link

phil294 commented Feb 22, 2019

@hsmyers You should open a new issue and actually fill out the template (not just open it and answer, put the information in the starting post as described)

I think you should do that because your point is valid, in JS, 1 + 2E-3 works, but in coffee fails.

@vendethiel vendethiel reopened this Feb 22, 2019
@hsmyers
Copy link
Author

hsmyers commented Feb 22, 2019 via email

@hsmyers hsmyers changed the title Scientific notation disagreement BUG: Scientific notation disagreement Feb 22, 2019
@GeoffreyBooth GeoffreyBooth changed the title BUG: Scientific notation disagreement Bug: Scientific notation should allow uppercase E Apr 7, 2019
@rdeforest
Copy link
Contributor

In 2012 uppercase E was deemed an anti-feature in #2060.

The issue was originally opened because TC39 had deemed the octal constant 0O123 to be visually ambiguous and were therefore discussing limiting radix prefixes in ES6 to lowercase letters (0o123). Since CS didn't have any backwards compatibility restrictions the CS crew decided that they would prohibit them. While 0E isn't a radix prefix, they decided to prohibit it anyway "for consistency".

I'd be interested in submitting a PR to allow 0E but I'd like to get the go-ahead from someone on the core team first.

@GeoffreyBooth
Copy link
Collaborator

It seems like this wouldn’t be a breaking change? Other than that I don’t really have an opinion on this; would people then want the other letters to be allowed as uppercase too?

Part of CoffeeScript’s mission is to hide some of the bad parts of JavaScript, and 0O definitely feels like a bad part; but E doesn’t strike me as such. That said, I wasn’t part of the earlier debate.

@rdeforest
Copy link
Contributor

It's a trivial change so I put together a PR for the team to accept or reject at their discretion.

@Inve1951
Copy link
Contributor

I'm opposed to uppercase letters in number literals for the abovementioned reasons. E should not be an exception. The 0O ambiguity aside, it's still harder to parse since there's no visual separation.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging a pull request may close this issue.

6 participants