Skip to content

Provide normative guidance on function/option/option value impl. #925

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 16 commits into from
Nov 14, 2024

Conversation

aphillips
Copy link
Member

See #924 for the origin of this PR.

@aphillips aphillips added functions Issue pertains to the default function set normative Issue affects normative text in the specification LDML46.1 MF2.0 Draft Candidate labels Nov 6, 2024
@aphillips aphillips requested review from eemeli and macchiati November 8, 2024 17:27
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 left two comments. They are not showstoppers, but I wanted to get your read before approving.

spec/registry.md Outdated
although care needs to be exercised to ensure interoperability
and to avoid collisions with future standardization.
There is no namespace mechanism for _option_ values,
however, the [stability policy](#stability-policy) for this specification
Copy link
Member

Choose a reason for hiding this comment

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

I don't see that the stability policy that this points to only allows a-z, A-Z, and 0-9 in option values. Does that mean we couldn't have a standard future function option that took fairly liberal string options like prefex=|β/γ| ?

Copy link
Member Author

@aphillips aphillips Nov 8, 2024

Choose a reason for hiding this comment

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

The stability policy says specifically:

Updates to this specification will only reserve, define, or require function names or function option names consisting of characters in the ranges a-z, A-Z, and 0-9. All other names in these categories are reserved for the use of implementations or users.

String option values can include wide ranging literals. This is meant to restrict enumerated keywords. Note too that the restriction is to option/function names, not values. We should probably consider amending this policy in an appropriate way. I will add an issue and agenda item to that effect.

Copy link
Collaborator

Choose a reason for hiding this comment

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

For clarity, we should probably update the stability policy text to refer to "identifiers" rather than "names" in that phrase.

Co-authored-by: Mark Davis <mark@unicode.org>
Copy link
Collaborator

@eemeli eemeli left a comment

Choose a reason for hiding this comment

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

I continue to think that the proposed text is too prescriptive.

Thus far, we've been describing what future versions of the spec may do, as in the stability policy part that's been quoted in the discussion here a few times:

Updates to this specification will only reserve, define, or require function names or function option names consisting of characters in the ranges a-z, A-Z, and 0-9. All other names in these categories are reserved for the use of implementations or users.

Note the second sentence there, which is specifically reserving a part of the non-namespaced function and option names for implementations and users, beyond even what's left to them in the namespaced identifiers. Yes, this text was originally written before we'd defined : namespaces, but it also was not modified in #834, where much of the text around it was refactored.

This PR presents a different type of requirement on implementations: We're no longer stating what future spec versions might do, but mandating how they MUST behave and what they might allow their users to do.

The proposed text also requires the swallowing of errors even in places where errors are non-fatal, disallowing them from being emitted even via a side-channel. This does not match our existing behaviour e.g. in option resolution, where we specifically allow for emitting errors for unresolved values while still requiring that the option resolution as a whole succeeds. We ought to allow an implementation to signal that it doesn't support an option or option value while still emitting some formatted output, if we want to include the options currently proposed in #915.

spec/registry.md Outdated
Comment on lines 29 to 32
Implementations MAY _accept_ _functions_ not defined in this specification.
Functions not defined by any version of this specification MUST use an implementation-defined _namespace_.
In addition, implementations SHOULD provide mechanisms for users to
register and use user-defined _functions_ and their associated _functional handlers_.
Copy link
Collaborator

Choose a reason for hiding this comment

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

Would this not mean that any user-defined function "MUST use an implementation-defined namespace"? That seems rather arbitrary.

I do not think that we ought to place any such restrictions on user-defined functions.

Copy link
Member Author

Choose a reason for hiding this comment

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

So... SHOULD use an implementation defined namespace, right?

Copy link
Collaborator

Choose a reason for hiding this comment

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

That's weird too; the namespace used by a user-defined function should be entirely user-definable, and we should continue to allow for user functions using no namespace at all.

Copy link
Member Author

Choose a reason for hiding this comment

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

Functions not defined by any version of this specification SHOULD use an implementation-defined or user-defined namespace.

Does that fix it? (I added or user-defined)

I agree with you that impls/users can do whatever they want, but we should provide guidance not to use un-namespaced functions that we don't define.

Copy link
Member Author

Choose a reason for hiding this comment

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

(mutated the text: have a look)

spec/registry.md Outdated
Comment on lines 43 to 45
Implementation-defined values SHOULD use a distinguishing character
or character sequence, such as by prefixing with a `_` U+005F LOW LINE,
to ensure that they don't collide with future standardization.
Copy link
Collaborator

Choose a reason for hiding this comment

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

If this is desirable, then we should update the stability policy to ensure that future standardization won't be able to use a _ in option values.

Copy link
Member Author

Choose a reason for hiding this comment

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

Agreed. I will make a PR for that.

Copy link
Collaborator

Choose a reason for hiding this comment

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

See #929.

Co-authored-by: Eemeli Aro <eemeli@mozilla.com>
@aphillips
Copy link
Member Author

@eemeli

Note the second sentence there, which is specifically reserving a part of the non-namespaced function and option names for implementations and users, beyond even what's left to them in the namespaced identifiers. Yes, this text was originally written before we'd defined : namespaces, but it also was not modified in #834, where much of the text around it was refactored.

We explicitly want to allow user-defined functions to have non-namespaced options and non-weird option values. That's why that text was (and is) like that. This section is saying what users/implementations can do to standard and optional functions in the registry. If my proposed text is too strong and you think we should SHOULD instead of MUST, suggest that?

This PR presents a different type of requirement on implementations: We're no longer stating what future spec versions might do, but mandating how they MUST behave and what they might allow their users to do.

I think the requirements here were implicit in our stance about the standard function registry: you can't claim conformance if you don't implement these functions with (at least) these options. The stability policy says that we will never remove any of these. But we might add to the list over time. We don't require that implementations provide additional functions or that they support user-defined ones.

The proposed text also requires the swallowing of errors even in places where errors are non-fatal, disallowing them from being emitted even via a side-channel. This does not match our existing behaviour e.g. in option resolution, where we specifically allow for emitting errors for unresolved values while still requiring that the option resolution as a whole succeeds.
The proposed text does not require errors to be swallowed. It requires that specific ones not be produced. That's instead of trying to require that each and every functions and each and every option be positively implemented or have some specific effect.

Notice that these requirements are highly targeted. For standard functions we prohibit Unknown Function. That is, the function must exist. For standard options we prohibit Bad Option for that identifier. We don't prohibit Bad Option for a bad value (this might be clarified).

We ought to allow an implementation to signal that it doesn't support an option or option value while still emitting some formatted output, if we want to include the options currently proposed in #915.

This might be possible? I would tend to add an Unsupported Operation error instead of trying to smoosh everything into Bad Option. It's the difference between searching high-and-low for why my bog standard message doesn't work ("I'm sure I spelled currencyDisplay correctly!!") and finding out that the value variant is unimplemented on this platform.

aphillips and others added 2 commits November 10, 2024 08:41
@aphillips
Copy link
Member Author

I'm toying with adding:

Note

An implementation can emit a Bad Option error for an option value
that is ill-formed, unsupported, or violates some constraint

The potential problem is the word "unsupported"

@macchiati
Copy link
Member

I think some examples would help.

In all cases, we want to very significantly reduces the chances of collisions. We could explore — in the future — reducing it further by putting a requirement like java does, based on domain names, but that is clearly way beyond what we should be discussing now.

Non-standard function

  • We don't want this to be :list, but rather mozilla:list
  • It doesn't need namespaced options or option values, since it defines all of them.

Non-standard option on standard function

  • We don't want this to be :number invert, but rather :number mozilla:invert
  • It doesn't need namespaced option values, since it defines all of them

Non-standard option value on standard function standard option

  • We don't want* this to be :number style=rational, but rather :number style=mozilla:rational

* A key feature for me is that we should distinguish enum options (whose option values are expected to be one of a fixed set of identifiers) from non-enum options, like regex=|[a-z]{3,8}(-[a-z]{3,8})*|. The former would be easy to namespace option values for, while the latter get trickier. I think this distinction is too last-minute for making into 46.1, but would make our approach more cohesive.

@aphillips
Copy link
Member Author

I agree with most of your comment @macchiati except:

Non-standard option value on standard function standard option

  • We don't want* this to be :number style=rational, but rather :number style=mozilla:rational

An option value can be a literal or a variable.

We don't permit : in unquoted literals nor do we have namespacing in option values as a feature. I don't think we need to introduce namespacing either. Non-standard option values will produce Bad Option errors on implementations that don't recognize the value. The suggestion of introducing a character like underscore that we (the Unicode "we") promise not to use in standard/optional (reserved name) values could help, e.g. :number style=mozilla_rational or :number style=_moz_rational. But I don't think we need to overbake it here.

@macchiati
Copy link
Member

Sorry, I should have written style=|mozilla:rational|

However, you have a good point. Having a namespace option in option values is unnecessary, because and implementation can always use a namespaced option instead.

That is, instead of :number style=rational, they can use :number mozilla:style=rational

That is much cleaner, and means they are not polluting the standard option's values. What we can the do is simply impose the requirement that:

OLD

Implementations MAY accept additional option values for options defined here,
although care needs to be exercised to ensure interoperability
and to avoid collisions with future standardization.
There is no namespace mechanism for option values,
however, the stability policy for this specification

NEW
Implementations MUST not implement additional option values for standard options of standard functions. Instead, the implementations can use a standard function with a namespaced option that has the additional option values. For example:

1) :number style=rational // must no be used unless some version of this standard has rational as an option value
2) :number foo:style=rational // legitimate, since there is a different namespace.

This lets any parser of the message containing (2) know that there is a special option supported by 'foo'.

@aphillips
Copy link
Member Author

That is, instead of :number style=rational, they can use :number mozilla:style=rational

That is much cleaner, and means they are not polluting the standard option's values. What we can the do is simply impose the requirement that:

Meh. Using namespaced options that overlap built-in values leads to conflicts with the non-namespaced variety ({$n :number style=foo ns:style=bar}) as well as the need for messages to be refactored to add/remove namespaces (let's avoid breaking people's translation memories to the degree possible).

I tend to think that namespaced options are fine for implementations to add a feature like icu:skeleton to a function.

For options that we define, we probably should have a "policy" (not in the stability policy) that we will try to avoid adding option values (to avoid breaking existing conformant implementations) but not guarantee that we won't.

So... that means I'd tend to say something like:

Note

Implementations can support option values not defined by this specification.
However, such values might become defined with a different meaning in the future,
including with a different, incompatible name
or using an incompatible value space.
Supporting implementation-specific option values for standard or optional functions is NOT RECOMMENDED.

If an implementer feels the need for other/different values, they should come to us and ask for it.

- Define _Unsupported Operation_ error allowance
- Remove mentions of the Default Registry
- Convert note to text
- Incorporate suggestions
@aphillips
Copy link
Member Author

@eemeli can you check and see if the changes reflect your understanding?

Copy link
Collaborator

@eemeli eemeli left a comment

Choose a reason for hiding this comment

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

Mostly this looks good, but option values are not "standard" or "optional", and given that we include

Accepting a function or its options does not mean that a particular output is produced.

we don't need to distinguish the two.

aphillips and others added 6 commits November 13, 2024 13:41
Co-authored-by: Eemeli Aro <eemeli@mozilla.com>
Co-authored-by: Eemeli Aro <eemeli@mozilla.com>
Co-authored-by: Eemeli Aro <eemeli@mozilla.com>
Co-authored-by: Eemeli Aro <eemeli@mozilla.com>
Co-authored-by: Eemeli Aro <eemeli@mozilla.com>
Co-authored-by: Eemeli Aro <eemeli@mozilla.com>
@aphillips aphillips requested a review from eemeli November 14, 2024 16:04
@aphillips
Copy link
Member Author

Merging per agreement in 2024-11-11 call.

@aphillips aphillips merged commit 4181354 into main Nov 14, 2024
1 check passed
@aphillips aphillips deleted the aphillips-normative-function-guidance branch November 14, 2024 16:40
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 LDML46.1 MF2.0 Draft Candidate normative Issue affects normative text in the specification
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants