Change open/close from function to expression #466
Conversation
There was a problem hiding this comment.
As proposed, this PR is a pretty direct s/function/expression/ replacement in the middle half of the Expression » Annotation » Function section of the syntax spec. Here's how it currently looks like.
This kinda doesn't make sense, with half of the function explanation suddenly talking about expression instead. If we do want to describe open/close/standalone with reference to the expression as a whole, these paragraphs should be extracted into a separate section.
I'm also not convinced that this change is beneficial as:
- the description of open/close with respect to function is valid,
- our formatting and registry is completely unaffected by the proposed changes and still considers the open/close sigils only during function lookup, and
- I don't see how this is "Bringing the spec description in sync with the abnf".
There was a problem hiding this comment.
I agree with @eemeli that the title is probably wrong. The ABNF clearly makes the +- sigils part of function names and is quiet about open/close expressions. If we were to truly bring the spec into alignment with the ABNF, this would effectively say that +foo, -foo and $foo are not distinct from one another, or, if they are distinct, that the distinction has no effect on the parsing of the message.
We need to decide about what open and close mean and what, if any, restrictions go on them, how open and close "communicate" with each other or get paired, and so on. Or we need to reserve these differences to future versions or to implementations.
| @@ -412,32 +412,32 @@ what _options_ and _option_ values are valid, | |||
| and what outputs might result. | |||
| See [function registry](./) for more information. | |||
|
|
|||
| _Functions_ can be _standalone_, or can be an _opening element_ or _closing element_. | |||
| _Expressions_ can be _standalone_, or can be an _opening element_ or _closing element_. | |||
There was a problem hiding this comment.
If I were to make this change, I would make "element" into expression too:
| _Expressions_ can be _standalone_, or can be an _opening element_ or _closing element_. | |
| An _Expression_ can be _standalone_, or can be an _opening expression_ or _closing expression_. |
| An **_<dfn>opening element</dfn>_** is a _expression_ that SHOULD be paired with a _closing element_. | ||
| A **_<dfn>closing element</dfn>_** is a _expression_ that SHOULD be paired with an _opening element_. |
There was a problem hiding this comment.
... and I would do that globally...
| > A _message_ with two markup-like _expressions_, `button` and `link`, | ||
| > which the runtime can use to construct a document tree structure for a UI framework: | ||
| > | ||
| > ``` | ||
| > {{+button}Submit{-button} or {+link}cancel{-link}.} | ||
| > {{button +html}Submit{button -html} or {link +html}cancel{link -html}.} |
There was a problem hiding this comment.
This is a metacomment rather than one on this particular change.
I tend to want the examples in our spec to be valid functions in the function registry or to be clearly from the "foo" family of example markup. If we don't provide HTML functions in the default registry or an optional extended registry, I'd prefer to use examples that are suggestive but not real. The original example did that better than providing the +html markup.
I do see that you want to show functions with operands, so perhaps consider examples.
| - `:` for a _standalone_ expression | ||
| - `+` for an _opening element_ | ||
| - `-` for a _closing element_ |
There was a problem hiding this comment.
Looking at the current ABNF, expressions are what goes into the placeholder, including the case when there's no function. Consequently, I think this PR should instead talk about function annotations, which is our way of saying "function invocations" or "function application".
In other words, open/close are not a property of the function (an abstract entity that needs to be called to do anything), but instead are a property (in the current design) of the function's application/invocation/call, i.e. annotation.
Replacing #447
Merging was too messy, and my change was in main, not in a branch.