Skip to content

Clarifications to pattern selection #385

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.

Sign up for GitHub

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 9 commits into from
Jun 5, 2023
Merged

Clarifications to pattern selection #385

Changes from all commits
Commits
Show all changes
9 commits
Select commit Hold shift + click to select a range
5aa988b
Clarifications to pattern selection
catamorphism May 22, 2023
e2c61e2
Remove reference to "the formatter" in description of catch-all matching
catamorphism May 23, 2023
919e1d4
Change "decoded" to "resolved" for keys
catamorphism May 23, 2023
81efa3e
Clarify the relationship between key resolution and literal resolution
catamorphism May 24, 2023
ade76a3
Clarify the relationship between key resolution and literal resolutio…
catamorphism May 24, 2023
3cd0914
Normative language for required number of keys in variants
catamorphism May 31, 2023
f08b9b6
Rewrite the call to `SortVariants`
catamorphism May 31, 2023
04bc82d
Follow style for describing method return values
catamorphism May 31, 2023
8668af2
Add newline
catamorphism May 31, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
40 changes: 25 additions & 15 deletions spec/formatting.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -27,11 +27,13 @@ the _pattern_ of one of the _variants_ must be selected for formatting.
When a message has a single _selector_,
an implementation-defined method compares each key to the _selector_
and determines which of the keys match, and in what order of preference.
A catch-all key will always match, but is always the least preferred choice.
During selection, the _variant_ with the best-matching key is selected.
The list of keys passed to this implementation-defined method
does not include the catch-all key `*`.
The catch-all key `*` is treated as a match for any _selector_,
but with the lowest possible preference.
Comment on lines +30 to +33
Copy link
Member

Choose a reason for hiding this comment

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

Wait... why are we doing this? What is wrong with letting the "implementation-defined method" decide that the * key is the best match?

What you're requiring here is that the selector method return "no match" in order to reach * and that the higher-level caller of the "implementation-defined method" then choose the fallback vs. allowing the selector method to make that determination. We haven't discussed this previously, so I don't understand why we are changing this text.

Copy link
Collaborator

Choose a reason for hiding this comment

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

I don't think this is intended as a change to the selection method, but a clarification of what's already in the Resolve Preferences section below.

If you think that should be changed, that might be best discussed in a separate issue or PR.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

@aphillips Yes, as Eemeli said, it's meant to be a clarification of what's already there. The existing definitions of "Resolve Preferences", "Filter Variants", and "Sort Variants" only pass literals to "the implementation-defined method". So the method can do whatever it wants if it's passed anything that's not a literal, such as *. Saying "The list of keys passed to this implementation-defined method does not include the catch-all key *" is another way of saying that these definitions don't rely on undefined behavior.

Copy link
Member

Choose a reason for hiding this comment

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

We're being overly normative. In fact, the "implementation-defined method" is probably overly normative. I don't particularly like, on re-reading, the text on lines 27-29 (since my old implementation considered the selector to be a function doing the comparison and this is different from a method that "compares each to the selector")

I also don't think that the "when one selector" followed by the "when more than one selector" structure is that helpful. I would probably write this section as:

When a message contains a match statement, it determines which expression matches by ordering the available keys.
In a message with more than one selector, the number of keys in each variant must equal the number of selectors.
Each key corresponds to the selectors in the match statement by position.
To determine the match, each selector is used in turn to order and filter the list of keys.
In order to select the single best variant for a given set of inputs, the full list of variants is filtered and sorted. Each variant with a key that does not match its selector is omitted from the list of variants. The remaining variants are sorted by key preference, with earlier selectors in the list of selectors having a higher priority than later ones.
The highest-sorted variant is selected.

Notice that my text is equivalent to the text here, but doesn't say how a selector is implemented or how sorting is implemented. It just says what must happen when processing a message.

Copy link
Collaborator

Choose a reason for hiding this comment

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

@aphillips Would you prefer the current proposed change to be included or dropped from this PR? Getting this merged would make it easier for you to file a subsequent PR with your proposed rewrite.

Copy link
Member

Choose a reason for hiding this comment

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

On the one hand, I would prefer not to merge non-consensus text wherever possible. But I agree that the lack of other reviewers leaves little evidence about whether this should or should not go.

Let's merge this. I will file my comments as a separate issue and create a PR with them once this is merged. Sound like a plan?


In a message with more than one _selector_,
each _variant_ also has a corresponding number of keys.
the number of keys in each _variant_ **_must_** equal the number of _selectors_.
These correspond to _selectors_ by position.
The same implementation-defined method as above is used to compare
the corresponding key of each _variant_ to its _selector_,
Expand Down Expand Up @@ -74,7 +76,8 @@ Next, using `res`, resolve the preferential order for all message keys:
1. For each _variant_ `var` of the message:
1. Let `key` be the `var` key at position `i`.
1. If `key` is not the catch-all key `'*'`:
1. Let `ks` be the decoded value of `key`.
1. Assert that `key` is a _literal_.
1. Let `ks` be the resolved value of `key`.
1. Append `ks` as the last element of the list `keys`.
1. Let `rv` be the resolved value at index `i` of `res`.
1. Let `matches` be the result of calling the method MatchSelectorKeys(`rv`, `keys`)
Expand All @@ -98,7 +101,8 @@ filter the list of _variants_ to the ones that match with some preference:
1. Let `key` be the `var` key at position `i`.
1. If `key` is the catch-all key `'*'`:
1. Continue the inner loop on `pref`.
1. Let `ks` be the decoded value of `key`.
1. Assert that `key` is a _literal_.
1. Let `ks` be the resolved value of `key`.
1. Let `matches` be the list of strings at index `i` of `pref`.
1. If `matches` includes `ks`:
1. Continue the inner loop on `pref`.
Expand All @@ -123,21 +127,27 @@ Finally, sort the list of variants `vars` and select the _pattern_:
1. Let `matchpref` be an integer with the value `minpref`.
1. Let `key` be the `tuple` _variant_ key at position `i`.
1. If `key` is not the catch-all key `'*'`:
1. Let `ks` be the decoded value of `key`.
1. Assert that `key` is a _literal_.
1. Let `ks` be the resolved value of `key`.
1. Let `matchpref` be the integer position of `ks` in `matches`.
1. Set the `tuple` integer value as `matchpref`.
1. Call the method SortVariants(`sortable`).
1. Set `sortable` to be the result of calling the method `SortVariants(sortable)`.
1. Set `i` to be `i` - 1.
1. Let `var` be the _variant_ element of the first element of `sortable`.
1. Select the _pattern_ of `var`.

The method SortVariants is determined by the implementation.
It takes as an argument a `sortable` list of (integer, _variant_) tuples,
which it modifies in place using some stable sorting algorithm.
The method does not return anything.
The list is sorted according to the tuple's first integer element,
such that a lower number is sorted before a higher one,
and entries that have the same number retain their order.
`SortVariants` is a method whose single argument is
a list of (integer, _variant_) tuples.
It returns a list of (integer, _variant_) tuples.
Any implementation of `SortVariants` is acceptable
as long as it satisfies the following requirements:

1. Let `sortable` be an arbitrary list of (integer, _variant_) tuples.
1. Let `sorted` be `SortVariants(sortable)`.
1. `sorted` is the result of sorting `sortable` using the following comparator:
1. `(i1, v1)` <= `(i2, v2)` if and only if `i1 <= i2`.
1. The sort is stable (pairs of tuples from `sortable` that are equal
in their first element have the same relative order in `sorted`).

### Examples

Expand Down Expand Up @@ -221,7 +231,7 @@ when * * {Otherwise}
Presuming a more powerful implementation which supports selection on numerical values,
and provides a `:plural` function that matches keys by their exact value
as well as their plural category (preferring the former, if possible),
and an Enligh-language formatting context in which
and an English-language formatting context in which
the variable reference `$count` resolves to the number `1`,
pattern selection proceeds as follows for this message:
Comment on lines 231 to 236
Copy link
Member

Choose a reason for hiding this comment

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

Good typo fix.

I find this example nearly impossible to read?

I don't agree with the idea of an "English-language formatting context". It should be in terms of a locale.

I'll come back an suggest an edit when I have a few more minutes.

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 edit could maybe be a separate PR?

Copy link
Member

Choose a reason for hiding this comment

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

Yes.

Copy link
Member

Choose a reason for hiding this comment

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

Actually, on second thought... it's a typo correct. Fix the typo. We'll address the text separately.


Expand Down