Conversation
eemeli
requested review from
aphillips,
catamorphism,
macchiati,
mihnita and
sffc
Co-authored-by: Addison Phillips <[email protected]>
Co-authored-by: Addison Phillips <[email protected]>
aphillips
left a comment
aphillips
left a comment
There was a problem hiding this comment.
One comment on the design doc, which is optional.
Thank you for wrapping up percent!!
Co-authored-by: Addison Phillips <[email protected]>
This comment was marked as off-topic.
This comment was marked as off-topic.
|
@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 @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. |
spec/functions/number.md
Outdated
| 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. |
There was a problem hiding this comment.
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."
There was a problem hiding this comment.
@catamorphism I revised the text to match your suggestion; PTAL?
4 participants
Closes #956
As discussed on the preceding weekly calls, it looks like the
least worstbest option for us is to add a dedicated:percentfunction, to match what we're doing splitting:currencyand:unitfrom: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.