-
-
Notifications
You must be signed in to change notification settings - Fork 35
In bidi default strategy, make steps consistent with each other #969
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
In bidi default strategy, make steps consistent with each other #969
Conversation
|
Adding |
spec/formatting.md
Outdated
| 1. If `msgdir` is `'LTR'` | ||
| and `isolate` is False, | ||
| let `fmt` be itself | ||
| append `fmt` to the formatted output. |
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 other steps don't do append. They talk about what fmt contains (e.g. "prefix fmt with...")
The change we should be making should actually be earlier. Something like:
- For each expression
expin pattern:
i. Letfmtbe the formatted string representation of the resolved value ofexpto be appended to the formatted output.
|
As discussed in today's meeting, I rewrote the text to specify a function from expressions to formatted strings. |
spec/formatting.md
Outdated
| Implementations MAY supply a _bidirectional isolation strategy_ that performs no processing. | ||
| The _Default Bidi Strategy_ is defined as follows: | ||
| The _Default Bidi Strategy_ is defined as a function `B` from expressions |
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.
Linkify expressions.
This is weird? Where does B come from? Why is it a "function"? The text above this talks about "bidirectional isolation strategies". I think this needs to be different.
Perhaps:
The Default Bidi Strategy is defined as follows:
- Let
outbe an empty string to contain the formatted output of the message- Let
msgdir...- For each expression
expin the pattern:
- If
expis a plain literal (text), appendexptoout.- If
expis an placeholder, letfmtbe the formatted string representation of
the resolved value ofexp.- Let
dirbe the directionality offmt...
// etc.- Append
fmttoout.- Emit
outas the formatted output of the message.
Note that we don't have a name for the non-placeholder bits of text in a pattern. I call them expressions here, but they aren't really expressions.
It would be even better if the section on formatting just above here defined how to make the string output. The "bidi strategy" then could be reference in that set of instructions ("apply the appropriate bidirectional isolation strategy to the contents of expression")
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 rewrote this imperatively instead of as a function. I also did a rewrite to call the components of a pattern a "part", and distinguish between text parts and placeholder parts.
Co-authored-by: Addison Phillips <[email protected]>
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 is an improvement. See inline for a few further nitpicky suggestions.
I'm not sure what "If
msgdirisLTRin the formatted output" meant, but this is my best guess; "letfmtbe itself" was inconsistent with the other steps, which refer to "In the formatted output..." rather than settingfmt.I think this is a normative change, since the spec was basically saying
there is no formatted outputthat no change is made to the formatted output if step 2(iv)(a) applies.