Skip to content
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 contextual options to expression-attributes design doc #780

Open
wants to merge 10 commits into
base: main
Choose a base branch
from

Conversation

eemeli
Copy link
Collaborator

@eemeli eemeli commented May 6, 2024

The core idea is to provide explicit and well-defined support for u:id, u:locale, and u:dir when used as function or markup options, and to thereby clarify that expression attributes have no runtime impact.

@eemeli eemeli added design Design principles, decisions formatting labels May 6, 2024
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.

Partial review

exploration/u-options.md Outdated Show resolved Hide resolved
exploration/u-options.md Outdated Show resolved Hide resolved
exploration/u-options.md Outdated Show resolved Hide resolved
exploration/u-options.md Outdated Show resolved Hide resolved
exploration/u-options.md Outdated Show resolved Hide resolved
aphillips and others added 3 commits May 6, 2024 08:24
Co-authored-by: Eemeli Aro <eemeli@mozilla.com>
Co-authored-by: Addison Phillips <addison@unicode.org>
@eemeli eemeli changed the title Add design doc for u: options Add design doc for contextual options May 6, 2024
exploration/contextual-options.md Outdated Show resolved Hide resolved
exploration/contextual-options.md Outdated Show resolved Hide resolved
exploration/contextual-options.md Outdated Show resolved Hide resolved
exploration/expression-attributes.md Show resolved Hide resolved
exploration/expression-attributes.md Show resolved Hide resolved
@mihnita
Copy link
Collaborator

mihnita commented May 12, 2024

TLDR: I am happy with namespaced attributes to represent global/universal options (non-function dependent)
But that makes the attributes even less useful.
So I would suggest we drop them.

@aphillips
Copy link
Member

In the 2024-05-20 call, we agreed not to merge this PR but to have @eemeli fold this into exploration/expression-attributes.md design.

@eemeli eemeli changed the title Add design doc for contextual options Add contextual options to expression-attributes design doc May 22, 2024
@eemeli
Copy link
Collaborator Author

eemeli commented May 22, 2024

Refactored the PR to be a change/expansion of the expression-attributes design doc rather than a separate one.

@eemeli eemeli requested review from mihnita and aphillips May 22, 2024 14:34
exploration/expression-attributes.md Outdated Show resolved Hide resolved
exploration/expression-attributes.md Outdated Show resolved Hide resolved
exploration/expression-attributes.md Outdated Show resolved Hide resolved

> Example identifying two literal numbers:
>
> ```
> The first number was {1234 :number @id=first} and the second {56789 :number @id=second}.
> The first number was {1234 :number u:id=first} and the second {56789 :number u:id=second}.
Copy link
Member

Choose a reason for hiding this comment

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

This change doesn't belong here in the expression attributes design. Keep the u: stuff for the design option in which we suggest u:

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

That design option is here presented as the proposed one.

exploration/expression-attributes.md Show resolved Hide resolved
> ```

- `dir` — An override for the LTR/RTL/auto directionality of the expression.

> Example explicitly isolating the directionality of a placeholder:
> Example explicitly isolating the directionality of a placeholder
> for a custom user-defined function:
Copy link
Member

Choose a reason for hiding this comment

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

why is a custom function wanted here? I used the implicit :string function on purpose before.

I agree that a custom function example is a good idea, just not maybe here.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Because the design proposed here uses u: options for contextual effects, and those require an annotation.

and must be supported by all implementations.
Common options or attributes should work the same way in different functions.

Special options or attributes should not conflict with other option names.
Copy link
Member

Choose a reason for hiding this comment

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

"Special" or "common"?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Special, because that can account for implementations-specific things that may be "common" within their scope, but not universally. So e.g. @amzn:marketplace=US or amzn:marketplace=US.

Comment on lines 218 to 219
Solve the two disparate use cases separately,
so that their namespaces are not comingled.
Copy link
Member

Choose a reason for hiding this comment

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

Why are you changing the proposed design section instead of just adding the additional options? We can make changes to the proposed design when we've chosen one.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Because I'm proposing that we support both u: options and @ attributes?

I would like to advance this discussion towards a solution, and so I'm here proposing this as a potentially viable solution for us to agree on.


As should be obvious, the current situation is not tenable in the long term, and should be resolved.

### Do not provide any guidance
Copy link
Member

Choose a reason for hiding this comment

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

How is this different from "do nothing"?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

"Do nothing" leaves the caution in the syntax.md file; this would remove that.

Co-authored-by: Addison Phillips <addison@unicode.org>
exploration/expression-attributes.md Outdated Show resolved Hide resolved
exploration/expression-attributes.md Outdated Show resolved Hide resolved
Co-authored-by: Tim Chevalier <tjc@igalia.com>
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.

Lots of these comments are related to the formatting and structuring of the design doc rather than to the technical arguments.

exploration/expression-attributes.md Outdated Show resolved Hide resolved
Comment on lines +284 to +288
This would mean not defining anything for default registry functions either,
effectively requiring implementation-specific options like `icu:locale`.

Other functions could use their own definitions and handling for similar options,
such as `locale` or `x:lang`.
Copy link
Member

Choose a reason for hiding this comment

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

I think spelling out the pros/cons here might be useful?

Pros

  • implementations would be free to define these options/attributes to suit local implementation needs

Cons

  • users could not count on these options to be available in each function
  • users would have to learn different requirements/restrictions
    on option use for each function and for each separate implementation

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

At the moment, these alternative descriptions are not formatted as pro/con lists, and I would rather keep them internally consistent. Therefore, I'd much prefer not adding such a list to just one option, and I don't want to refactor the entire PR again to include such a reformatting. We could do such a reformatting as a follow-on change.

Other functions could use their own definitions and handling for similar options,
such as `locale` or `x:lang`.

Formatted parts for markup would not be able to directly include an identifier.
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 follow this statement. Why wouldn't I be able to have a {#b id=foo}? Or a {$x :number id=foo}?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

This statement is specifically about formatted parts for markup, not expressions. Functions can define how expressions get formatted, but markup is not configurable. Sure, you could pass markup an id=foo option, but that option would be only present in the general bag of options in the formatted part, and would potentially conflict with other use.

For example, it's rather common for HTML elements to include an id value, which is already relevant for the CSS or JS executing on that document, but an identifier may be separately relevant for the post-processing of a formatted message, such as adding in parameters that were not included in the translation.

In other words, if we're translating a message like

Would you like to {#a u:id=continue}accept{/a} or {#a u:id=cancel}reject{/a} the offer?

we need a way to disambiguate the two formatted {#a} spans from each other, hence the u:id. That way the message can end up first formatted as:

[
  { type: 'literal', value: 'Would you like to ' },
  { type: 'markup', kind: 'open', source: '#a', name: 'a', id: 'continue' },
  { type: 'literal', value: 'accept' },
  { type: 'markup', kind: 'close', source: '/a', name: 'a' },
  { type: 'literal', value: ' or ' },
  { type: 'markup', kind: 'open', source: '#a', name: 'a', 'id': 'cancel' },
  { type: 'literal', value: 'reject' },
  { type: 'markup', kind: 'close', source: '/a', name: 'a' },
  { type: 'literal', value: ' the offer?' }
]

And from there, we can construct an HTML snippet with additional properties:

Would you like to <a href="continue?token=123abc">accept</a> or <a href="cancel?token=123abc">reject</a> the offer?

But if there's no common u:id definition, then the identifier ends up mixed together with the other markup options, and it gets much trickier to build tooling that's universal across different implementations and function authors, and doesn't risk bleeding through into the output HTML, where a property like id="continue" could very easily conflict with some other operations and expectations of the page.

Copy link
Member

Choose a reason for hiding this comment

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

Okay, I see what you're saying now. My example omitted the u: by mistake. I still think that the text you have the design document is super-mysterious, though. Note that attributed strings can have overlapping markup (that would be illegal in HTML). For example:

Would you like to {#span type=foo u:id=1}click {#span type=bar u:id=2}here{/span u:id=1} to get {/span u:id=2} illegal stuff.

Perhaps:

Suggested change
Formatted parts for markup would not be able to directly include an identifier.
Lack of a built-in identifier would inhibit tools support for markup,
particularly when formatted to parts,
when the relationship of open and close markup is ambiguous.
> For example, the two `{/a}` markup elements in this mess:
>```
> Would you like to {#a u:id=continue}accept{/a} or {#a u:id=cancel}reject{/a} the offer?
>```

Comment on lines 299 to 300
Define at least `locale` and `dir` as options for default registry functions,
with handling internal to each function implementation.
Copy link
Member

Choose a reason for hiding this comment

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

I think you should address how id could work here. Presumably the id property of the resolved value could be queried?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

I'm honestly not sure how to make id sensibly work, if we're not defining it generally in the spec but only specifically for each standard registry function. That would mean not having any specific field for it in the formatting-internal resolved values, and intermingling it with the other resolved options.

I hope we do not end up with this option, and so can avoid needing to answer all the questions that it would introduce.

Copy link
Member

Choose a reason for hiding this comment

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

I agree that we should not end up with this option. I'd probably address it by saying:

Suggested change
Define at least `locale` and `dir` as options for default registry functions,
with handling internal to each function implementation.
Define the `locale`, `dir`, and `id` options for all default registry functions
and establish a policy of including them in all future default registry functions,
with the hope that other functions and implementations will adopt this as a best practice.
Actually implementation would be internal to each function.

exploration/expression-attributes.md Show resolved Hide resolved
Comment on lines 327 to 328
Allow expression attributes to influence the formatting context,
but do not directly pass them to user-defined functions.
Copy link
Member

Choose a reason for hiding this comment

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

This is too suppositional.

Suggested change
Allow expression attributes to influence the formatting context,
but do not directly pass them to user-defined functions.
Expression attributes are stored in or modify the formatting context,
rather than passed directly to functions (including user-defined functions).
Function implementations can access the values of attributes via the formatting
context, but are not required to do so.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

I'm not sure what you mean by "suppositional". I think what you're describing is one possible way of implementing this alternative, but it's not the only one. Should we split this option into two, one of which only defines an explicit short list of attributes with well-defined runtime effects, and the other allows for values to be stored in the expression's formatting context?

Copy link
Member

Choose a reason for hiding this comment

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

I said "suppositional" because it's hard to say what "influencing" the formatting context means?

I agree that my suggestion is one possible way of implementing this and that there could be others. However, by expressing it concretely, I hope to enable us to have a concrete conversation about which option to pursue.

I probably wouldn't go to the trouble of splitting the option up. I'd just note that my suggestion is one route and limiting the attributes to the built-in set is another?

exploration/expression-attributes.md Outdated Show resolved Hide resolved
Co-authored-by: Addison Phillips <addison@unicode.org>
@eemeli eemeli requested a review from aphillips June 9, 2024 17:04
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
design Design principles, decisions formatting
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants