Change :math to :offset and remove draft status
#1073
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
In the 2025-05-12 teleconference, the group agreed to make the name of this function less generic. A number of alternative names were proposed in the call. The name `:offset` was used here, partially as a placeholder while we seek consensus. We agreed that the new function would be limited to adjusting the value of the operand by small integer values. _Digit Size Option_, used here, would make that from `0` to `999`. It has been proposed that this be restricted to `1` to `99`. In this pull request, note that I added the function name to subsections. The current spec doesn't link properly to subsections because all functions use the same section ID. I did not change the other functions to match, pending discussion.
aphillips
requested review from
catamorphism,
eemeli,
gibson042,
mihnita and
macchiati
macchiati
reviewed
May 12, 2025
| ##### Operands | ||
| > [!NOTE] | ||
| > The purpose of this _function_ is to supply compatibility with | ||
| > ICU's `PluralFormat` and its `offset` feature, also found in ICU MessageFormat. |
There was a problem hiding this comment.
Suggested change
| > ICU's `PluralFormat` and its `offset` feature, also found in ICU MessageFormat. | |
| > ICU's `PluralFormat` and its `offset` feature, also found in ICU MessageFormat. | |
| > It is needed for migration, because it is commonly used for messages | |
| > that need to convey the first few items of a list plus the count of the remainder, | |
| > such as in the example above. |
There was a problem hiding this comment.
I think we're inverting things a bit here. As I see it, the primary purpose of this function is the same as that of plural offset in MF1, to enable the expression of messages with plural variance that depends on an offset count. Supporting migration from MF1 arises from that; it ought not be presented as "the purpose" of the function as a whole.
macchiati
reviewed
May 12, 2025
Comment on lines
+192
to
194
| The _options_ on `:offset` are exclusive with each other, | ||
| and exactly one _option_ is always required. | ||
| The _options_ do not have default values. |
There was a problem hiding this comment.
Suggested change
| The _options_ on `:offset` are exclusive with each other, | |
| and exactly one _option_ is always required. | |
| The _options_ do not have default values. |
Delete, there is only one, and it is required below.
There was a problem hiding this comment.
But this PR is not changing the inclusion of both add and subtract options?
macchiati
reviewed
May 12, 2025
| and exactly one _option_ is always required. | ||
| The _options_ do not have default values. | ||
|
|
||
| The following _options_ are REQUIRED to be available on the function `:math`: | ||
| The following _options_ are REQUIRED to be available on the function `:offset`: | ||
|
|
||
| - `add` |
There was a problem hiding this comment.
It's actually a subtract, since it is the remaining count.
macchiati
reviewed
May 12, 2025
| @@ -222,18 +223,18 @@ If the _operand_ of the _expression_ is an implementation-defined numeric type, | |||
| such as the _resolved value_ of an _expression_ with a `:number` or `:integer` _annotation_, | |||
| it can include option values. | |||
| These are included in the resolved option values of the _expression_. | |||
| The `:math` _options_ are not included in the resolved option values. | |||
| The `:offset` _options_ are not included in the resolved option values. | |||
There was a problem hiding this comment.
At least two reasons:
- Clarity; the function processes and "consumes" the option, as its resolved value includes the offset. This also allows for an implementation to present the resolved value of an
:offsetand a:numberusing the same representation. - We've not identified any use cases that would benefit from a consumer of an
:offsetvalue to know that an offset was applied to the value, or what that offset was.
eemeli
approved these changes
May 13, 2025
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
In the 2025-05-12 teleconference, the group agreed to make the name of this function less generic. A number of alternative names were proposed in the call. The name
:offsetwas used here, partially as a placeholder while we seek consensus.We agreed that the new function would be limited to adjusting the value of the operand by small integer values. Digit Size Option, used here, would make that from
0to999. It has been proposed that this be restricted to1to99.In this pull request, note that I added the function name to subsections. The current spec doesn't link properly to subsections because all functions use the same section ID. I did not change the other functions to match, pending discussion.