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

$(a: float) now works consistently in nim js, avoiding printing floats as ints #14134

Merged
merged 3 commits into from
Apr 27, 2020

Conversation

timotheecour
Copy link
Member

@timotheecour timotheecour commented Apr 27, 2020

/cc @Araq

echo 2.0 # 2.0
var b = 2.0
echo b # before PR: 2, after PR: 2.0
  • it would be nice to merge $ now works for unsigned intergers with nim js #14122 before this PR so I can avoid creating a new tests/js/tdollar_float.nim

  • once this is green-lighted but before merge, I'll use the single argument applyFormat in jsgen (as I did for mFloatToStr) everywhere appropriate to make code DRY

after PR

I'm thinking to introduce a compiler flag (eg --ecmascript:es5) to let users choose which js version they want to target. The Number.isInteger I'm using in this PR is available on all browsers but not on IE, but there is a polyfill we can easily introduce for that, say in std/jspolyfills. The workaround implementation should be used or not depending on the desired js version in --ecmascript; BigInt could for example be introduced for int for --ecmascript:es2020

ci failures unrelated

@timotheecour timotheecour changed the title fix https://github.com/timotheecour/Nim/issues/133; $(a: float) works in nim js like in other backends $(a: float) now works consistently in nim js, and matches other backends Apr 27, 2020
@metagn
Copy link
Collaborator

metagn commented Apr 27, 2020

#7605 is the same as the issue in your repo

@timotheecour
Copy link
Member Author

thanks! updated the PR msg; this PR will allow you to not change the test (or, better, to increase test coverage by using both float+int) in tpegs.nim in your PR #4247

@timotheecour timotheecour changed the title $(a: float) now works consistently in nim js, and matches other backends $(a: float) now works consistently in nim js, avoiding printing floats as ints Apr 27, 2020
@Araq Araq merged commit bb982c6 into nim-lang:devel Apr 27, 2020
@timotheecour timotheecour deleted the pr_dollar_float_js branch April 27, 2020 23:10
Araq pushed a commit that referenced this pull request Apr 28, 2020
* StringStream & more stdlib modules support for JS/NimScript

* change back pegs test in line with #14134
EchoPouet pushed a commit to EchoPouet/Nim that referenced this pull request Jun 13, 2020
…ats as ints (nim-lang#14134)

* fix timotheecour#133; $(a: float) works in nim js like in other backends

* fix tests

* fix test for windows that prints 1.1e17 differently than other OS
EchoPouet pushed a commit to EchoPouet/Nim that referenced this pull request Jun 13, 2020
…#14095)

* StringStream & more stdlib modules support for JS/NimScript

* change back pegs test in line with nim-lang#14134
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

var a = 2.0; echo a prints 2 in nim js Inconsistent float formatting in javascript.
3 participants