Skip to content

Conversation

@gibson042
Copy link
Collaborator

Fixes #1026

@gibson042 gibson042 requested a review from eemeli February 20, 2025 19:27
1. Else:
1. If the _option_ value consists of a _literal_:
1. Include that information in `rv`.
1. Mark `rv` as a literal option value.
Copy link
Member

Choose a reason for hiding this comment

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

I like this wording change very much

Copy link
Collaborator

Choose a reason for hiding this comment

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

This change does seem good.

1. Else:
1. If the _option_ value consists of a _literal_:
1. Include that information in `rv`.
1. Mark `rv` as a literal option value.
Copy link
Collaborator

Choose a reason for hiding this comment

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

This change does seem good.

To allow for _function handlers_ to ensure that certain _option_ values are set by _literals_,
the _resolved value_ of each _option_ value MUST include information about
whether the _option_ value is a _literal_ or a _variable_.
Note that this information is irrelevant for a _resolved value_ that is not used as the value of an _option_.
Copy link
Collaborator

Choose a reason for hiding this comment

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

Hmm. Is it perhaps not clear that "option value" is a syntax feature? This phrase seems to imply that a resolved value could be an option value, and that doesn't really make sense.

We probably ought to explicitly define option value as a term in the Syntax section.

Copy link
Member

Choose a reason for hiding this comment

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

We probably ought to explicitly define option value as a term in the Syntax section.

Absolutely!

Copy link
Member

Choose a reason for hiding this comment

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

Let's do that in a separate PR.

@aphillips
Copy link
Member

Does this need to be in 47? It looks 47-ish, as I believe it is non-normative and small. Changing to option value as a term is also a candidate.

@macchiati
Copy link
Member

Non-normative changes: typos, clarifications, even new definitions, etc we can do; we just need to notify the TC.

Normative changes that are important need approval by the TC.

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.

I am okay with the wording proposed, pace we work in option value. I made a wording suggestion, however, that might improve the paragraph?

@aphillips aphillips added fast-track Editorial change permitted to use fast-track merge rules editorial Issue is non-normative LDML47 formatting Issue pertains to the formatting section of the spec labels Feb 20, 2025
Copy link
Member

@macchiati macchiati left a comment

Choose a reason for hiding this comment

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

I suggest one further change, but not a blocker.

Co-authored-by: Mark Davis <[email protected]>
@aphillips aphillips merged commit 527521a into unicode-org:main Feb 21, 2025
1 check passed
@eemeli eemeli added this to the LDML 47 milestone Jul 23, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

editorial Issue is non-normative fast-track Editorial change permitted to use fast-track merge rules formatting Issue pertains to the formatting section of the spec

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Option resolution does not correctly handle transitivity

4 participants