Skip to content

Add :percent #1094

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

Merged
merged 8 commits into from
Aug 11, 2025
Merged

Add :percent #1094

merged 8 commits into from
Aug 11, 2025

Conversation

eemeli
Copy link
Collaborator

@eemeli eemeli commented Aug 4, 2025

Closes #956

As discussed on the preceding weekly calls, it looks like the least worst best option for us is to add a dedicated :percent function, to match what we're doing splitting :currency and :unit from :number.

This PR adds that function, initially as Draft. It also updates the related design doc to match this choice, and updates the test suite.

To make it into LDML 48, this PR will need to land by 2025-08-18 at latest.

@eemeli eemeli added this to the LDML 48 milestone Aug 4, 2025
@eemeli eemeli added the functions Issue pertains to the default function set label Aug 4, 2025
Co-authored-by: Addison Phillips <addison@unicode.org>
@eemeli eemeli requested a review from aphillips August 5, 2025 07:40
Co-authored-by: Addison Phillips <addison@unicode.org>
Copy link
Member

@aphillips aphillips left a comment

Choose a reason for hiding this comment

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

One comment on the design doc, which is optional.

Thank you for wrapping up percent!!

Co-authored-by: Addison Phillips <addison@unicode.org>
@macchiati

This comment was marked as off-topic.

@eemeli
Copy link
Collaborator Author

eemeli commented Aug 6, 2025

@catamorphism Thank you for catching the loose use of "resolved value". I've now replaced the places where that's used with "numeric value", which is what we use in describing :offset.

@macchiati I've moved your comment to be its own new issue #1095, as it goes beyond the scope of this PR, and we should discuss and address it separately.

@eemeli eemeli requested a review from catamorphism August 6, 2025 06:50
of the _operand_ of the annotated _expression_
together with the resolved options' values.
The _resolved value_ is not altered by `:percent`,
that is, its numerical value is not multiplied by 100.
Copy link
Collaborator

Choose a reason for hiding this comment

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

Sorry to nitpick, but this is still a little confusing; the resolved value returned by the function can't be altered, because the function handler (logically) creates a new resolved value to return. The input resolved value is immutable.

Perhaps something like: "The numerical value of the operand is the same as the numerical operand of the resolved value of the expression: it is not multiplied by 100."

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

@catamorphism I revised the text to match your suggestion; PTAL?

@eemeli eemeli requested a review from catamorphism August 10, 2025 13:48
@eemeli eemeli merged commit bb8323d into main Aug 11, 2025
3 checks passed
@eemeli eemeli deleted the percent branch August 11, 2025 16:47
eemeli added a commit to messageformat/messageformat that referenced this pull request Aug 11, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
functions Issue pertains to the default function set
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[FEEDBACK] Please move style percent to its own function
4 participants