-
-
Notifications
You must be signed in to change notification settings - Fork 36
Fix normative vs. non-normative keywords in formatting.md #411
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
Conversation
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.
Good start. Some comments about normative language.
Co-authored-by: Addison Phillips <addisonI18N@gmail.com>
Noticed and fixed a few more uses of non-normative "must" and added a Reserved Syntax error. Its emission criteria is defined under Expression Resolution. |
spec/formatting.md
Outdated
> Attempting to format this message would result in an Reserved Syntax error | ||
> if done within a context that does not support the `^` private use sigil: |
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 think this might be better as:
> Attempting to format this message would result in an Reserved Syntax error | |
> if done within a context that does not support the `^` private use sigil: | |
> Attemping to format the following message would result in a _Reserved Syntax error_ | |
> if done in a context that does not support the private use sigils or where no private | |
> use function `^private` is available: |
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 don't think we want to refer to "private use functions" as a concept. We've been pretty good so far to not assign any meaning to reserved annotations, as they really could mean anything.
Your _underlines_
do remind me that we ought to apply a pass of **_definition_**
and _use_
styling to errors.
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.
Fair enough. Perhaps then:
... or where
^private
is undefined:
What I'm trying to get at is that when an implementation decides to support private use, the effect of it failing should be defined.
Part of me wants to go back (as commented before) and truly separate reserved from private-use (separate text sections and separate specification). Private-use is not the same as reserved usage.
I think maybe another important consideration is that we don't want interstitial processes to yak on messages that the ultimate consumer might be able to process (or might rely on), e.g. translation tools should not reject ^private
. Only the act of formatting should produce an error for reserved or private sequences (the "I don't know what this is" error)
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.
@eemeli ... and yeah, we need to do a doc sweep for formatting and MUSTard consistency.
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.
The language that I'm proposing is attempting to define this as a well-defined error in an implementation that does not attach any meaning to ^
, and it's explicitly not defining what might happen in an implementation that does attach some significance to the sigil. It could, for instance, be a custom comment format.
I think maybe another important consideration is that we don't want interstitial processes to yak on messages that the ultimate consumer might be able to process (or might rely on), e.g. translation tools should not reject
^private
. Only the act of formatting should produce an error for reserved or private sequences (the "I don't know what this is" error)
Agreed. That's why this is classified as a Resolution error and not a Syntax error, despite the name.
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.
The language that I'm proposing is attempting to define this as a well-defined error in an implementation that does not attach any meaning to ^, and it's explicitly not defining what might happen in an implementation that does attach some significance to the sigil. It could, for instance, be a custom comment format.
Understood. I think you can't say "doesn't attach any meaning to ^
" because that sigil always means "private use gorp follows". "There is a private use expression here, but I don't know what it means" or "there is a private use expression here, but I don't support formatting these".
I think what I was describing (the implementation supports private use and the sigil in question, but something is wrong with the private use expression) should just be a garden variety Resolution error? If we weren't trying to portmanteau the reserved and private-use sigils in a single section, you'd have some repetitive text, but we can elucidate the private use requirements separately at our leisure.
To your point about the name, I would change it from "Reserved Syntax error" to avoid the misleading term "syntax". Perhaps "reserved expression error" or (more-correct in the ABNF) "reserved annotation error"? Or, given that we want the same errors for reserved and private-use, perhaps "Unsupported expression error"?
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.
Renamed the error as "Unsupported Expression", as that seems suitably inclusive of both future-spec-reserved and private-reserved annotations.
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.
All comments are editorial, otherwise I would approve.
@@ -721,6 +725,26 @@ These are divided into the following categories: | |||
> when * {The value is not one.} | |||
> ``` | |||
|
|||
- **Unsupported Expression errors** occur when an expression uses |
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.
Use singular instead of plural:
- **Unsupported Expression errors** occur when an expression uses | |
An **Unsupported Expression error** occurs when an expression uses |
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.
The current plural language matches that used by all the other errors. Could fixes to the style used for the error descriptions be handled as a separate PR, rather than further increasing the scope of this PR?
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 tend to favor "not digging the hole deeper" when committing text, but since we'll probably merge this in an hour, I can come back and make a more holistic set of edits later.
With regard to this comment (and noting that it rhymes with other comments in other PRs), the style I prefer is something like:
Term is (definition here).
(discussion of term's attributes, usage, triggers, etc., with term referencing the definition)
I tend to prefer singular, because automatic linking of references to the definition is easier when the term is originally singular. Unlike W3C, we don't have markup to use for alternate forms of the term and will have to write the link generation eventually as a script (if we decide we want linking). 🤷
@@ -721,6 +725,26 @@ These are divided into the following categories: | |||
> when * {The value is not one.} | |||
> ``` | |||
|
|||
- **Unsupported Expression errors** occur when an expression uses | |||
syntax reserved for future standardization, | |||
or for private implementation use that is not supported by the current implementation. |
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 think we should try to be consistent when talking about private use. Also, remember your formatting. I'd suggest:
or for private implementation use that is not supported by the current implementation. | |
or for _annotations_ that use a _private-use_ sequence unsupported by the current implementation. |
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.
Your proposed change does not match our current description of reserved, which does not so differentiate annotations reserved for future spec versions compared to those reserved for private implementation use. Furthermore, we do not have a definition of private-use to refer to.
It's clear that the language around reserved could be updated, but that really ought to happen holistically rather than as a part of this PR, which so far is only touching the formatting parts of the spec.
or for private implementation use that is not supported by the current implementation. | ||
|
||
> For example, attempting to format this message | ||
> would always result in an Unsupported Expression error: |
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.
> would always result in an Unsupported Expression error: | |
would always result in an _Unsupported Expression error_: |
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.
Could this fix also be included in a separate PR of all our error definitions and mentions? The current proposal matches the style used by all our current error references.
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.
See ⬆️
The current error references should be edited to italicize terms like our spec's front matter says we do, so this change really should be committed and other, inconsistent, text fixed. I will file an issue to do formatting and take that as an action after I click "Comment" here 😉
> Attempting to format this message would result in an Unsupported Expression error | ||
> if done within a context that does not support the `^` private use sigil: |
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.
Formatting and a small wording change:
> Attempting to format this message would result in an Unsupported Expression error | |
> if done within a context that does not support the `^` private use sigil: | |
> Formatting this message would result in an _Unsupported Expression error_ | |
> if done within a context that does not support the `^` _private-use_ sigil: |
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.
This too seems like it should be a part of a separate subsequent PR updating our error definitions, as the current language matches that used by all our other error examples.
This cleans up a few things that were not included in PR #396. Namely: