Skip to content

Add digit separators #1160

New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # to your account

Open
wants to merge 11 commits into
base: master
Choose a base branch
from
Open

Conversation

seizethedave
Copy link

@seizethedave seizethedave commented Jun 22, 2024

This PR adds digit separators (1_000) to Jsonnet's numeric constants.

Accompanying issue with format proposal: #1155

@sbarzowski
Copy link
Contributor

I'm in favor of this. We need to also:

  1. Update the docs. At least . Ideallly also the tutorial.

See https://github.com/google/jsonnet?tab=readme-ov-file#locally-serving-the-website for working on documentation.

  1. Add some end-to-end examples. Just dump some .jsonnet files in test_suite and run https://github.com/google/jsonnet/blob/master/test_suite/refresh_golden.sh.

  2. We'll need to update go-jsonnet implementation (separate PR). It should be straightforward.

@seizethedave
Copy link
Author

Thanks @sbarzowski, I'll get cracking on those.

@seizethedave seizethedave marked this pull request as ready for review July 4, 2024 21:00
@seizethedave
Copy link
Author

@sbarzowski I think I'm ready for another round of feedback on this one. I suspect these might need a little more help:

  • a more formal treatment of the numeric grammar in the docs
  • note how std.parseInt and related functions will not honor numbers with underscores
    Thanks!

@seizethedave
Copy link
Author

Anything I can do to push this forward @sbarzowski? Thanks!

@johnbartholomew johnbartholomew self-assigned this Feb 23, 2025
@johnbartholomew
Copy link
Collaborator

Sorry for all the delays on this. I do hope to review this soon; it seems like a useful improvement.

@seizethedave
Copy link
Author

@johnbartholomew I'll try to find a few minutes to get this PR back into a healthy/mergable state. If you have any pre-review feedback or changes I can get those going.

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

Successfully merging this pull request may close these issues.

4 participants