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

CLDR-13221 format for currency symbol at decimal point #1137

Merged
merged 1 commit into from
Mar 23, 2021

Conversation

macchiati
Copy link
Member

Checklist

@macchiati macchiati requested review from sffc and pedberg-icu March 22, 2021 03:37
docs/ldml/tr35-numbers.md Outdated Show resolved Hide resolved
docs/ldml/tr35-numbers.md Outdated Show resolved Hide resolved

Before | After | Decimal
-------|-------|-------
¤#,##0.00|#,##0.00¤|#,##0¤00
Copy link
Member

@sffc sffc Mar 23, 2021

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can the pattern #,##0¤00 ever actually occur in CLDR data? If it can, then we'll need to change the EBNF for number patterns (assuming there is one).

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I thought we had an EBNF for these, but I can't find one. These number patterns continue becoming more and more complex to parse. I'm going to need to implement a parser in ICU4X. Before I do that, I would really like to have a formal grammar that I can implement.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

/[0#]¤[0#]/ does not occur in any CLDR data (current or past), so I don't think there is any real compatibility concern.

I think an EBNF is an excellent idea. Please file a ticket for that and we can work on it (for next release).

@macchiati macchiati requested a review from sffc March 23, 2021 15:29
Copy link
Member

@sffc sffc left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Some more questions

docs/ldml/tr35-numbers.md Outdated Show resolved Hide resolved

Before | After | Decimal
-------|-------|-------
¤#,##0.00|#,##0.00¤|#,##0¤00
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I thought we had an EBNF for these, but I can't find one. These number patterns continue becoming more and more complex to parse. I'm going to need to implement a parser in ICU4X. Before I do that, I would really like to have a formal grammar that I can implement.

docs/ldml/tr35-numbers.md Outdated Show resolved Hide resolved
docs/ldml/tr35-numbers.md Outdated Show resolved Hide resolved
@macchiati macchiati requested a review from sffc March 23, 2021 17:41
Copy link
Member

@sffc sffc left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thank you! This is much better.

I see now what you were trying to do in the table. I think the new table layout is clearer, even though it loses the vertical alignment of the "before" and "after" patterns.

@macchiati macchiati force-pushed the macchiati-CLDR-13221 branch from 0f6a37d to 9a86dc4 Compare March 23, 2021 19:58
@jira-pull-request-webhook
Copy link

Hooray! The files in the branch are the same across the force-push. 😃

~ Your Friendly Jira-GitHub PR Checker Bot

@macchiati macchiati requested a review from sffc March 23, 2021 20:10
@macchiati
Copy link
Member Author

Need another review because of squash to fix commit message (sigh)

@macchiati macchiati merged commit fa00b2c into maint/maint-39 Mar 23, 2021
@macchiati macchiati deleted the macchiati-CLDR-13221 branch March 23, 2021 21:22
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.

2 participants