-
-
Notifications
You must be signed in to change notification settings - Fork 36
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
Conversation
…functions See #924 for the origin of this PR.
Co-authored-by: Eemeli Aro <eemeli@mozilla.com>
There was a problem hiding this 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 |
There was a problem hiding this comment.
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=|β/γ| ?
There was a problem hiding this comment.
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.
There was a problem hiding this comment.
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>
There was a problem hiding this 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
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_. |
There was a problem hiding this comment.
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.
There was a problem hiding this comment.
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?
There was a problem hiding this comment.
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.
There was a problem hiding this comment.
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.
There was a problem hiding this comment.
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
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. |
There was a problem hiding this comment.
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.
There was a problem hiding this comment.
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.
There was a problem hiding this comment.
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>
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?
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.
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).
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 |
Co-authored-by: Eemeli Aro <eemeli@mozilla.com>
I'm toying with adding: Note An implementation can emit a Bad Option error for an option value The potential problem is the word "unsupported" |
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
Non-standard option on standard function
Non-standard option value on standard function standard option
* 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. |
I agree with most of your comment @macchiati except:
An option value can be a We don't permit |
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 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
NEW
This lets any parser of the message containing (2) know that there is a special option supported by 'foo'. |
Meh. Using namespaced options that overlap built-in values leads to conflicts with the non-namespaced variety ( I tend to think that namespaced options are fine for implementations to add a feature like 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. 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
@eemeli can you check and see if the changes reflect your understanding? |
There was a problem hiding this 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.
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>
Merging per agreement in 2024-11-11 call. |
See #924 for the origin of this PR.