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

[FEEDBACK] The re-use of :function between "annotation" and "output" positions can be a bit confusing #818

Open
lucacasonato opened this issue Jul 9, 2024 · 5 comments
Labels
Preview-Feedback Feedback gathered during the technical preview resolve-candidate This issue appears to have been answered or resolved, and may be closed soon.

Comments

@lucacasonato
Copy link
Contributor

lucacasonato commented Jul 9, 2024

During the last call I gave some feedback that I found it confusing that :functions are used in two (or maybe three) somewhat distinct ways, without any obvious syntax difference. @stasm asked me to open an issue.

Render functions / formatting functions

Inside of a placeholder expression in a simple message or quoted pattern, and also in the expression in a local declaration, the function acts as a rendering function, that determines how a value is formatted into the output. In code, this would be equivalent to:

formatted_value = f(value)

As far as I understand, the formatted_value could be a string, but it could also a more complex value that the renderer then will render in some way - like calling toString() on it in JS for example.

So really it'd be more something like

formatted_value = f(value)

formatted_value.toString()

I think this mostly makes sense and is relatively clear. (I do find it confusing that this essentially allows you to use annotation expressions in a way that is more or less identical to markup, by having the renderer special case some shapes of formatted_value, but I digress.)

Match selectors

Match selectors also look like placeholder expressions. Here however my first intuition tells me that the function is effectively being called with two arguments: the value, and an expected value (the key in the variant). It also does not return a string or more complex value for the renderer, but it returns a boolean.

matched = f(value, key)

After thinking about it some more, I think what it is really trying to model is:

// selector
formatted_value = f(value)

// variant
matched = formatted_value.match(key)

I think that makes much more sense - but this is not explained at all anywhere. If this is the mental model that we think developers should have, we should try to make that clearer.

This mental model is however undermined by the fact that annotations are required in expressions in selectors. If they were not (which I am not advocating for, because it would probably lead to worse tooling), the second mental model would work quite well - it could be simply explained.

What is also very unclear to me, is what it means to have formatting options on functions annotations in expressions in selectors. What does it mean to match on a :datetime style=short, are we matching on the underlying string value, or on the object representing date-time?

Input declarations

Input declarations also use the same placeholder expression syntax. Here however, the syntax initially looks like :function is a pure type annotation on the input variable. So in pseudocode:

fn message(value: function) {
  ...
}

This however is not at all how this behaves. The input declaration behaves much more like a local declaration that assigns the same variable as it operates on:

value = f(value)

I think that expressing this as .local $value = {$value :function} is actually much clearer. This is not valid syntax right now however.

EDIT: @aphillips corrected me - my understanding here was wrong. Indeed this does really behave just like a type annotation, which I find makes it even more confusing. For example, this is allowed right now:

.input {$count :number style=percent}
{{{$count}}}

But it does not actually format anything in percent style.

I would find the syntax less confusing if the behaviour were identical to this imaginary local declaration:

.local $count = {$count :number style=percent}
{{{$count}}}

Alternatively, a different syntax for type annotations would also work to disambiguate this from the other annotation expression things.

EDIT 2: I had misunderstood. My original understanding was correct

@aphillips
Copy link
Member

Thanks for the feedback. I commented in #819 about why .input {$count :mumble} {{{$name}}} is allowed (because we committed to allowing--but not requiring--lazy evaluation).

The spec tries to make clear that functions can be formatting, selection... or some functions can do both formatting and selection. The ability to do both is intentional and important: you want the selector to be selecting the same way that the formatter is formatting.

Important: this is partially false:

It also does not return a string or more complex value for the renderer, but it returns a boolean.

Matching (selectors) filter and stack rank the list of patterns (variants).

Currently none of the date/time functions perform selection, so you should focus on string or number functions for examples. These functions describe the interaction of formatting and selection options. An example of this would be in the :integer function:

.input {$percent :integer select=plural style=percent signDisplay=always groupingUsed=true}
.match {$percent}
0     {{😢 You got zero percent correct.}}
95    {{Almost! You were {$percent} percent correct.}}
100   {{Wow! You were {$percent} percent correct.}}
one   {{You were {$percent} percent correct.}}
many  {{You were {$percent} percent correct.}}
*     {{You were {$percent} percent correct.}}

Notice that the selector does not use the formatted string for matching values. It uses the resolved value of $percent, which, because it is style=percent is multiplied by 100. So signDisplay=always or groupingUsed=true do not change how :integer selects (but do affect what is formatted) (Note that the select=plural is not required since plural is the default selector)

This message is interesting because the keys 0 and 100 match the values 0.0 and 1.0 for $percent, but the key * also matches those values (in the en-US locale) at a lower quality level. Because the * key is special, I also included 95 and many keys. In a Polish language locale, 95 and many both match the input value 0.95, but 95 matches better. This can matter greatly if you have two selectors.

Some formatting options (signDisplay, useGrouping for example) do not in any way affect selection. But allowing "dual annotation" (for both selection and formatting) saves users typing later. Your mental model is close to what we want users to have. In most cases, we want people to configure the formatter (look at how many times I used it in the message above!) and then just select patterns against the declaration as appropriate, with minimal extra work... but only if it doesn't confuse people.

@lucacasonato
Copy link
Contributor Author

Firstly, I realize my original message had some errors, sorry for the confusing examples.

Thanks for the explanation on formatting vs selection, and the stack ranking behaviour of selection. The behaviour here is now more clear to me.

I still however find the syntax to be rather self-inconsistent. Namely the fact that expressions in .input declarations contain a variable reference that behaves like it is being bound to, while an identical expression in a match selector does not bind to this variable reference. I think a good example of this is the following message:

.input {$var :number signDisplay=always}
.match {$var :number style=percent}
* {{ {$var} }}

Playground

Here, the $var referenced in the wildcard matcher variant references a resolved value bound to $var in the input declaration.

However, the $var in the match selector is not bound.

I find not the individual behaviours here confusing - they both make sense. What I find confusing is the syntax of .input. It binds to a variable that is passed in as a reference. A more logical syntax in my eyes would be something where it is clear that the annotation in the .input declaration is not a type annotation, but it binds to the passed variable name with a new resolved value, as produced by the annotation in the expression.

Or alternately:

.local $var = {.input $var :number signDisplay=always}

Here .input is not a declaration, but a new expression type. It's just like a variable expression, except that the variable name is resolved from the input data, and not from locally bound variables. This would then allow having a local declaration with the same name in the binding and the variable expression. The $ sign, or even the entire $var could be omitted here, with .input itself acting as the reference for var from the input data.

This syntax could even be simplified allowing one to emit the .input entirely, if one is willing to relax design document regarding variable mutability to continue to disallow binding multiple local declarations with the same name, but allow a local declaration bound with a name to have a variable expression containing a variable reference with that same name, which would always be resolved through the input data:

.local $var = {$var :number signDisplay=always}

I think the above case would make implementations marginally more complex. But because rebinding is still disallowed, and shadowing is already possible, I don't think it's incredibly complex.

To reiterate my concern one more time: I find it highly confusing that the variable name in the variable expression is both a variable name that is bound to to create a shadowed variable binding in the context of the message, and a variable that refers to data from the input. This is especially the case because an identical expression in a .match does not bind to the passed variable to create a shadow. I think this behaviour makes sense in .match, but it is just absolutely not self consistent.

@aphillips
Copy link
Member

@lucacasonato Thanks. The WG is discussing this very problem in e.g. #824 and the associated (unresolved) design document selection-declaration.

@aphillips
Copy link
Member

We went with Option F in the balloting for selection-declaration, so .match no longer has expressions (and thus functions). This should align function usage more closely with your expectations. We also today (2024-09-10) discussed our understanding of "resolved value". Combined with Option F, we might have addressed the remainder of the problem here? Specifically, each function MUST define their resolved value (so that .local "assignments" have a clear meaning). This hasn't been implemented in the spec yet, so not yet marking this for resolution, but we seem to be making progress 😃

@aphillips aphillips added the resolve-candidate This issue appears to have been answered or resolved, and may be closed soon. label Nov 9, 2024
@aphillips
Copy link
Member

I think in the interim we have addresses this with the resolved value work, so marking as a potential r-c.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Preview-Feedback Feedback gathered during the technical preview resolve-candidate This issue appears to have been answered or resolved, and may be closed soon.
2 participants