Skip to content

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

Merged
merged 4 commits into from
Jul 10, 2023

Conversation

eemeli
Copy link
Collaborator

@eemeli eemeli commented Jul 3, 2023

This cleans up a few things that were not included in PR #396. Namely:

  • A formatting context suggestion from @aphillips.
  • Avoid using non-normative "may".
  • Add resolution rules for private reserved annotations.

@eemeli eemeli requested a review from aphillips July 3, 2023 19:49
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.

Good start. Some comments about normative language.

Co-authored-by: Addison Phillips <addisonI18N@gmail.com>
@eemeli eemeli requested a review from aphillips July 4, 2023 07:26
@eemeli
Copy link
Collaborator Author

eemeli commented Jul 6, 2023

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.

Comment on lines 739 to 740
> 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:
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 this might be better as:

Suggested change
> 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:

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 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.

Copy link
Member

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)

Copy link
Member

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.

Copy link
Collaborator Author

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.

Copy link
Member

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"?

Copy link
Collaborator Author

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.

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.

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
Copy link
Member

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:

Suggested change
- **Unsupported Expression errors** occur when an expression uses
An **Unsupported Expression error** occurs when an expression uses

Copy link
Collaborator Author

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?

Copy link
Member

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.
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 we should try to be consistent when talking about private use. Also, remember your formatting. I'd suggest:

Suggested change
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.

Copy link
Collaborator Author

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:
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
> would always result in an Unsupported Expression error:
would always result in an _Unsupported Expression error_:

Copy link
Collaborator Author

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.

Copy link
Member

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 😉

Comment on lines +739 to +740
> 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:
Copy link
Member

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:

Suggested 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:

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 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.

@eemeli eemeli requested a review from aphillips July 10, 2023 09:14
@aphillips aphillips changed the title Minor formatting fixes Fix normative vs. non-normative keywords in formatting.md Jul 10, 2023
@aphillips aphillips merged commit 46fa20f into unicode-org:main Jul 10, 2023
@eemeli eemeli deleted the formatting-polish branch July 11, 2023 08:22
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants