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

[locale.numpunct,locale.moneypunct] Canonicalize local grammar presen… #3174

Merged
merged 1 commit into from Oct 6, 2019

Conversation

jensmaurer
Copy link
Member

…tation.

Fixes #3100.

@jwakely
Copy link
Member

jwakely commented Aug 15, 2019

The change looks good to me, but if you're changing this anyway how about addressing two pre-existing issues.

  • Why do we have "integer" and "floatval" rather than "intval" and "floatval" or something like that?

  • Why do we need plusminus? Can't we just define sign as one of + and -?

Finally, does it matter that sign and units are defined as part of the integer grammar, but used in the floatval one?

@jensmaurer
Copy link
Member Author

Fixed the two issues mentioned in the bullets. Leaving the "sign" and "units" issue as-is; the two grammar snippets are right next to each other.

@jensmaurer
Copy link
Member Author

Leaving for @zygoloid to apply.

@zygoloid zygoloid added the needs rebase The pull request needs a git rebase to resolve merge conflicts. label Oct 6, 2019
@zygoloid
Copy link
Member

zygoloid commented Oct 6, 2019

After rebasing, could you attach an image of the "after" presentation? Thanks!

…tation.

In [locale.numpunct], rename the 'integer' non-terminal
to 'intval', consistent with 'floatval'.
Also remove the superfluous 'plusminus' non-terminal.
@jensmaurer jensmaurer removed the needs rebase The pull request needs a git rebase to resolve merge conflicts. label Oct 6, 2019
@jensmaurer
Copy link
Member Author

Rebased. Here are the pictures:
numpunct
money

@zygoloid zygoloid merged commit 8685db2 into cplusplus:master Oct 6, 2019
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.

fix [locale.numpunct] to use the same presentation for its grammar as the rest of the standard
3 participants