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 design doc for expression attributes #458
Add design doc for expression attributes #458
Changes from all commits
8f37b9c
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.
As mentioned elsewhere, I would prefer a prefix to name content.
Thought: will there be classes or packages of attributes that can be plugged in, e.g.:
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 would tighten this up. I would imagine that we'll add to the spec text similar to:
(I didn't say "an API" on purpose)
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.
What you're proposing is actually the opposite of what I'm proposing. The formatting context as currently specified is an explicit set of fields, extensible by an implementation but not custom functions:
So the proposal here would allow an implementation to modify the formatting context as it's applied to an expression, but it would not allow for a custom function definition "to query the set of attributes".
If it makes sense for an attribute to influence the behaviour of a custom function, and that attribute isn't an explicit part of the implementation's formatting context, why should its value be communicated via an expression attribute rather than a function option?
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.
A formatting context would be "a way to query the set of attributes"? I like your wording:
Although my question would be: how do I ensure that my custom annotation reaches my function's runtime via the context?
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 wording is from our current formatting spec, actually.
By using a function option to convey that information, rather than an expression attribute.
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.
My thinking here goes:
The answer cannot be function options or I would have used a function option and not an expression attribute.
Apparently the answer is: the attributes appear in the formatting context and we can just say 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.
In that situation, why not use a function option? If it's something affecting the behaviour of a single function and its value can be defined in the expression, why not use a function option for it?
We have not defined the shape of the formatting context with any specificity, and I don't think we should -- it's really an implementation detail. If an implementation wants to make its formatting context extensible in some way and for a custom expression attribute to affect it, I think that's very much an implementation detail and outside the scope of this 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.
We don't have to define the shape of the formatting context with any specificity in order to say that implementations must provide access to any attribute values in a message's context. We already have at least one contextual value (the locale) that we require the implementation to ensure.
So I agree that we don't want to define exactly how a function gets access to the context. We might not even require that there be a way for it to be obtained. But we can still require that any contextual mechanism (using whatever shape) provides access to the attribute.
If we don't follow my thinking through, wouldn't we be introducing a feature that doesn't work in at least some implementations?
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.
To rephrase and reiterate my own position on this and the currently proposed phrase, I do not think that functions should have access to the expression attributes or their values.
I do not think it would be a good idea for implementations to be required to provide access to any attribute values in a message's context. As we already have function options, I do not think that expression attributes should be duplicating their functionality. Allowing custom functions to access expression attributes would do exactly that.
Message formatting happens in some context that has properties such as the current locale, timezone, etc. that have an effect on the results. In the spec, we refer to this as the "formatting context", but do not exhaustively define its shape or properties. This is implementation-defined, and implementations may internally allow for its extensibility.
The general use case I have in mind for expression attributes that have a formatting impact is to provide a way to locally override formatting context values or error handling behaviour.
What is the use case for allowing custom functions to access custom attributes?
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.
What is the use case for allowing custom attributes? If we allow them, don't we need to provision them? If we don't allow them, won't implementations re-invent them to suit their own needs?
On some level, all functions are custom functions. They are all described through the function registry and made available to message writers.
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.
Actually, it's easy--but it isn't machine readable. In addition, there is no guarantee that resource-level commentary will be preserved during the translation process or be operable to MT or CAT tools. This is particularly true if the translation process segments inside the MF pattern.
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 hard if there is more than one expression with the exact same syntax within a message.
For example, consider this message:
With that, how can I assign the equivalent of
@example=2
to the{$count}
in the*
variant, but not the match selector or the placeholder in theone
variant?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.
Actually you just did an admirable--but not machine readable--job of it:
My first sentence is more of a bit of humor, really. We're in violent agreement here. The problem I wanted to highlight was the separation represents a probably loss of functionality as soon as you parse the message.