Specification refactor "v2" #441
Conversation
There was a problem hiding this comment.
This reads much better than the previous update. I found it easier to evaluate as a whole, here: https://github.com/unicode-org/message-format-wg/blob/aphillips-specification-refactor/spec/syntax.md
Comments below from a first pass; I may have more thoughts on this later. Not sure if I noticed all the small changes that are applied here.
spec/syntax.md
Outdated
| * The number of _keys_ on each _variant_ MUST be equal to the number of _selectors_. | ||
| * At least one _variant_ MUST exist whose _keys_ are all equal to the "catch-all" key `*`. |
There was a problem hiding this comment.
Running Prettier on this will replace the * with - and add a space before the list. Might be easier to just use its styling for markdown?
There was a problem hiding this comment.
Good callout and one for the style guide.
spec/syntax.md
Outdated
| A **_function_** is a used to evaluate, format, select, or otherwise process an _operand_, | ||
| or, if lacking an _operand_, its _options_. |
There was a problem hiding this comment.
This implies that a function with an operand does not care about its options, and that a function without an operand requires options. The previous reference to processing its annotation was really clumsy, but less incorrect.
This sentence should probably be completely refactored.
There was a problem hiding this comment.
Note that this sentence doesn't make sense as written. Refactored this section.
spec/syntax.md
Outdated
| > **Note** | ||
| > _External variables_ names are constrained by this namespace. |
There was a problem hiding this comment.
Strictly speaking not true. Anything not fitting name just won't be matched by any variable in the message.
There was a problem hiding this comment.
That's correct. I ran into this issue when drafting this section and it occurred to me that we needed to deal with the fact that name is constrained but callers will not be constrained. I'll refactor this sentence to be correct, but it is the formatting spec that needs instructions on handling of external variables, which I recorded in #442.
spec/syntax.md
Outdated
| >``` | ||
|
|
||
|
|
||
| > Expression examples: |
There was a problem hiding this comment.
Did you have a place in mind for these expression examples, and/or a reason why not to include them in this doc?
There was a problem hiding this comment.
Along with "complex messages", I just pushed them out of the way for the nonce. I'm reluctant to delete them without looking more closely at the expression section. I'll refactor these into the text.
Co-authored-by: Eemeli Aro <eemeli@mozilla.com>
Co-authored-by: Eemeli Aro <eemeli@mozilla.com>
Co-authored-by: Eemeli Aro <eemeli@mozilla.com>
We didn't describe options anywhere!
There was a problem hiding this comment.
-
We should drop the ToC. It requires unnecessary manual maintenance, and it's replicating built-in functionality that GitHub provides for rendered markdown via the outline button at the far right of the Preview/Code/Blame toolbar.
-
I think adding the
<dfn>tags is jumping the gun. We do ultimately need to transform our spec format to whatever is required for the UTS/UTR format that we'll be publishing it in, but it'll be just as easy for automation to then find the definitions as<b><i>...</i></b>compared to<b><i><dfn>...</dfn></i></b>. But if adding it, see line comments fixing a few mistakes.
| A _function_'s entry in the _function registry_ will define | ||
| whether the _function_ is a _selector_ or formatter (or both), | ||
| whether an _operand_ is required, | ||
| what form the values of an _operand_ can take, | ||
| what _options_ and _option_ values are valid, | ||
| and what outputs might result. | ||
| See [function registry](./) for more information. |
There was a problem hiding this comment.
Why is this description of the function registry embedded here in the syntax spec? Also, won't we eventually be making the _function registry_ a link to the relevant part of the spec, so shouldn't need an explicit markdown link as well?
Co-authored-by: Eemeli Aro <eemeli@mozilla.com>
Co-authored-by: Eemeli Aro <eemeli@mozilla.com>
Co-authored-by: Eemeli Aro <eemeli@mozilla.com>
Co-authored-by: Eemeli Aro <eemeli@mozilla.com>
Co-authored-by: Eemeli Aro <eemeli@mozilla.com>
This is the second attempt at the specification refactoring. I started with all of our PRs merged Monday.
This is a much more significant reformulation. It is intended to be read as prose as well as specifying individual productions.
I organized the sections "top down", with some sub-sections containing others, e.g.:
There is then a set of productions about Expressions and then finally the remaining terms, such as keywords, whitespace, and such. You might wish to view this rendered first rather than diving into the diff specifically.
I have followed commentary about separating "abstract" vs. "concrete" (e.g. in #440). This proposal embraces the use of ABNF constructs whose purpose is "abstract". We don't have to do this. We can define terms in the spec that do not appear in the ABNF. E.g.:
(and then use
expressionin the ABNF exclusively), a la:This PR does not include a repaired TOC and I have not yet removed the requirements to elsewhere. I moved some examples to the end, with an intention of moving them out altogether.
Have a read. 📚