Skip to content

Require select option to be set by a literal value #1016

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.

Sign up for GitHub

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

Jump to bottom
Merged
merged 2 commits into from
Feb 17, 2025
Merged

Require select option to be set by a literal value #1016

merged 2 commits into from
Feb 17, 2025

Conversation

eemeli
Copy link
Collaborator

@eemeli eemeli commented Feb 17, 2025

For context, see #1006

As pointed out by @aphillips in a comment on the ICU4X-TC feedback doc:

[It's] nearly impossible to localize a message in which [the select] option is passed by reference, since tools and translators can't tell how to translate the messages or how to explode (expand/contract) the list of keywords in a given locale.

We can't really make using a variable for select a syntax or data model error unless we add a really special case for it in the Syntax part, but this is nearly that.

Note that as shown in the test cases, formatting the following message would result in bad-option and bad-selector errors:

.local $sel = {1 :number select=exact}
.local $bad = {$sel :number}
.match $bad
1 {{ONE}}
* {{operand select {$bad}}}

@eemeli eemeli added functions Issue pertains to the default function set Agenda+ Requested for upcoming teleconference errors Issues related to the errors section of the spec LDML47 LDML 47 Release (Stable) labels Feb 17, 2025
aphillips
aphillips approved these changes Feb 17, 2025
View reviewed changes
@eemeli eemeli mentioned this pull request Feb 17, 2025
Closed
sffc
sffc reviewed Feb 17, 2025
View reviewed changes
test/tests/functions/number.json
"exp": "literal select {1}"
},
{
"src": ".local $bad = {exact} {{variable select {1 :number select=$bad}}}",
Copy link
Member

Choose a reason for hiding this comment

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

Suggestion: add a test similar to this one but with .input

@aphillips aphillips merged commit 4dbdc35 into main Feb 17, 2025
2 checks passed
@aphillips aphillips deleted the literal-select branch February 17, 2025 18:34
@eemeli eemeli mentioned this pull request Feb 17, 2025
Merged
catamorphism added a commit to catamorphism/icu that referenced this pull request Mar 1, 2025
@catamorphism
ICU-23059 ICU4C MF2: Spec test updates
4fa63b3 
Update spec tests to current version from message-format-wg

  - Update parser for changed name-start grammar rule
  - Validate number literals in :number implementation (since parser no longer does this)
  - Disallow `:number`/`:integer` select option set from variable

    See unicode-org/message-format-wg#1016

    As part of this, un-skip tests where the `bad-option` error is
    expected, and implement validating digit size options
    (pending PR unicode-org#2973 is intended
    to do this more fully)
@catamorphism catamorphism mentioned this pull request Mar 1, 2025
Merged
6 tasks
catamorphism added a commit to catamorphism/icu that referenced this pull request Mar 1, 2025
@catamorphism
ICU-23059 ICU4C MF2: Spec test updates
e137728 
Update spec tests to current version from message-format-wg

  - Update parser for changed name-start grammar rule
  - Validate number literals in :number implementation (since parser no longer does this)
  - Disallow `:number`/`:integer` select option set from variable

    See unicode-org/message-format-wg#1016

    As part of this, un-skip tests where the `bad-option` error is
    expected, and implement validating digit size options
    (pending PR unicode-org#2973 is intended
    to do this more fully)
eemeli added a commit to messageformat/messageformat that referenced this pull request Mar 2, 2025
@eemeli
feat(mf2)!: Require select option to be set by a literal value (unico…
f6ace5a 
…de-org/message-format-wg#1016)
@eemeli eemeli mentioned this pull request Mar 2, 2025
Merged
catamorphism added a commit to catamorphism/icu that referenced this pull request Mar 3, 2025
@catamorphism
ICU-23059 ICU4C MF2: Spec test updates
82f4fb9 
Update spec tests to current version from message-format-wg

  - Update parser for changed name-start grammar rule
  - Validate number literals in :number implementation (since parser no longer does this)
  - Disallow `:number`/`:integer` select option set from variable

    See unicode-org/message-format-wg#1016

    As part of this, un-skip tests where the `bad-option` error is
    expected, and implement validating digit size options
    (pending PR unicode-org#2973 is intended
    to do this more fully)
catamorphism added a commit to catamorphism/icu that referenced this pull request Mar 13, 2025
@catamorphism
ICU-23059 ICU4C MF2: Spec test updates
6da8982 
Update spec tests to current version from message-format-wg

  - Update parser for changed name-start grammar rule
  - Validate number literals in :number implementation (since parser no longer does this)
  - Disallow `:number`/`:integer` select option set from variable

    See unicode-org/message-format-wg#1016

    As part of this, un-skip tests where the `bad-option` error is
    expected, and implement validating digit size options
    (pending PR unicode-org#2973 is intended
    to do this more fully)
catamorphism added a commit to catamorphism/icu that referenced this pull request Mar 27, 2025
@catamorphism
ICU-23059 ICU4C MF2: Spec test updates
e59284f 
Update spec tests to current version from message-format-wg

  - Update parser for changed name-start grammar rule
  - Validate number literals in :number implementation (since parser no longer does this)
  - Disallow `:number`/`:integer` select option set from variable

    See unicode-org/message-format-wg#1016

    As part of this, un-skip tests where the `bad-option` error is
    expected, and implement validating digit size options
    (pending PR unicode-org#2973 is intended
    to do this more fully)
catamorphism added a commit to unicode-org/icu that referenced this pull request Mar 27, 2025
@catamorphism
ICU-23059 ICU4C MF2: Spec test updates
d0e30ac 
Update spec tests to current version from message-format-wg

  - Update parser for changed name-start grammar rule
  - Validate number literals in :number implementation (since parser no longer does this)
  - Disallow `:number`/`:integer` select option set from variable

    See unicode-org/message-format-wg#1016

    As part of this, un-skip tests where the `bad-option` error is
    expected, and implement validating digit size options
    (pending PR #2973 is intended
    to do this more fully)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@sffc sffc sffc left review comments

@aphillips aphillips aphillips approved these changes

@zbraniecki zbraniecki Awaiting requested review from zbraniecki

@macchiati macchiati Awaiting requested review from macchiati

Assignees
No one assigned
Labels
Agenda+ Requested for upcoming teleconference errors Issues related to the errors section of the spec functions Issue pertains to the default function set LDML47 LDML 47 Release (Stable)
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@eemeli @aphillips @sffc