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.
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
Add u: options namespace #846
Add u: options namespace #846
Changes from 2 commits
0117fee
f969ab6
3a3d4ab
6c20950
087bd0d
9c307cb
af2d4e9
b4b2921
680d25d
01fed13
de56e4c
0c9414b
File filter
Filter by extension
Conversations
Jump to
There are no files selected for viewing
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.
This seems like special pleading on our part. The
u:
namespace is "just a namespace"?Perhaps:
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.
It's not "just a namespace", because it needs special powers to affect the function context, though.
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 think that the "function context" context distinction is an implementation detail.
In most formatters implementations the locale and the options on how to format are passed to the constructor as parameters, at the same time.
I would rather look at this as "universal function parameters" that might be recognized and honored by several / all 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.
I think the problem here is that the current set of
u:
options behaves that way, but we might introduce one that isn't function context affecting in the future? If our intention to is to require that all such options be context-affecting, we should set that as a requirement in the design doc and elsewhere. We might need to contemplate an additional namespace in the future as well, although I can't think of any universal options just at the moment that we wouldn't just put in the default namespace.Note: I am not disagreeing with doing this. Just making sure we're consistent and clear about it.
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.
Perhaps quote the design document here?
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 first paragraph is mostly fine, but as #634 is only a PR at this time, it's way too early to refer to the RGI, for instance, or talk about incubation. We can add this language later, when we get consensus on these processes and their names.
I would also find it a bit odd to say e.g. that this "can contain functions", when AFAIK no such
u:
function has been proposed by anyone.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.
Permission to do something does not obligate us to actually do it. No one has proposed a
u:
function, but there's no reason to forbid them.Are you disagreeing with the design in #634? More specifically, do you disagree with my thinking about how to organize the registries? I do think we should lock-step these efforts to some degree. We'll probably rename RGI somewhere along the way, for example. But I think we'll likely end up with the same concepts at least.
We could replace my line 9 above with something like:
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 am in general agreement about having definitions for non-required functions and options, but I'm honestly not really sure what the current state of #634 is, and whether e.g. the registry development process added to the PR last Friday is final, or whether it's being iterated on. When we last discussed this on a call in May, it was "Still in progress".
I'm also not sure whether the current design doc proposal's language "Implementations are not required to implement any values found in this registry and may adopt or ignore registry entries at their discretion." ought to be strengthened into something more like a SHOULD.
My preference would be to not lockstep processes that do not strictly need lockstep, and to focus first on the concrete (i.e. this PR), and use that experience to inform the registry maintenance plan. On which we ought to have a separate discussion, if/once it's in a stable state.
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.
Why functions?
The idea for the parameters is "u: parameters are potentially recognized / honored by all functions"
What would be the meaning of a "u:" function?
If we think "u:function" is a Unicode function, it is redundant.
All functions in the standard registry are Unicode functions, since this is a Unicode spec.
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.
Perhaps:
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 proposed language doesn't quite match the accepted design, though. These are not intended to be options that are handled by each function, but by the implementation, and they e.g. apply changes to the function context, so that function authors explicitly do not need to do anything to enable their use.