Implement the simplified pattern selection algorithm in the spec (#898) #1080
Conversation
64da247 to
a166fef
Compare
…code-org#898) Implement Mark Davis's suggested pattern selection algorithm in the spec. Update the "resolved values" example to use the `match()` and `compare()` methods instead of `selectKeys()`. Rewrite the numeric and `:string` selector specs accordingly.
a166fef to
eff3bac
Compare
| The selection method is defined in terms of two auxiliary operations, | ||
| `selectorsMatch` and `selectorsCompare`. These operations in turn assume |
There was a problem hiding this comment.
Let's not change multiple things at the same time, so please keep abstract method references in the same style as MatchSelectorKeys previously, and follow semantic line breaks.
| and `rv.compare(k1, k2)` returns (for any keys `k1` and `k2`) | ||
| a value from the set `{WORSE, SAME, BETTER}`. | ||
|
|
||
| Other than the `match()` and `compare()` operations on resolved values, |
There was a problem hiding this comment.
Are rv.match(k) and match() referring to the same thing, or to different things? I presume they're meant to be the same, and if so, the name should not vary.
| 1. Let `nomatch` be a _resolved value_ for which `match(k)` is false | ||
| for any key `k`. |
There was a problem hiding this comment.
This is the third time the match method is referenced in the text, and it's the third different way of referring to it.
| in their first element have the same relative order in `sorted`). | ||
| Next, using `res`: | ||
|
|
||
| 1. Set `bestVariant` to `UNSET`. |
There was a problem hiding this comment.
What is bestVariant? Please include a "Let" statement when first introducing a variable.
| 1. Set `bestVariant` to `var`. | ||
| 1. Else: | ||
| 1. Let `bestVariantKeys` be the keys of `bestVariant`. | ||
| 1. If `selectorsCompare(res, keys, bestVariantKeys)` is `BETTER` |
There was a problem hiding this comment.
If we only care whether the result of this comparison is BETTER or not, why do we differentiate SAME and WORSE?
I previously asked about this in #898 (comment).
| 1. Else: | ||
| 1. Return `result`. | ||
|
|
||
| The result of `selectorsCompare(selectors, keys1, keys2)` is `result`. |
There was a problem hiding this comment.
Not true, given the early returns around catch-all keys.
| 1. Else: | ||
| 1. Return `result`. | ||
|
|
||
| The result of `selectorsCompare(selectors, keys1, keys2)` is `result`. | ||
|
|
||
| #### Pattern Selection Examples |
There was a problem hiding this comment.
These non-normative examples are getting verbose enough that we ought to move them to an annex rather than having them interrupt the prose here.
| 1. ASSERT: `resolvedSelector.match(key1)`. | ||
| 1. ASSERT: `resolvedSelector.match(key2)`. |
There was a problem hiding this comment.
| 1. ASSERT: `resolvedSelector.match(key1)`. | |
| 1. ASSERT: `resolvedSelector.match(key2)`. | |
| 1. Assert that `resolvedSelector.match(key1)` is true. | |
| 1. Assert that `resolvedSelector.match(key2)` is true. |
| 1. Let `k1` be the _resolved value_ of `key1` in Unicode Normalization Form C [\[UAX#15\]](https://www.unicode.org/reports/tr15). | ||
| 1. Let `k2` be the _resolved value_ of `key2` in Unicode Normalization Form C [\[UAX#15\]](https://www.unicode.org/reports/tr15). |
There was a problem hiding this comment.
Defining these as resolved values does not match how they're presumed to be strings later on.
| 1. ASSERT: `resolvedSelector.match(key1)`. | ||
| 1. ASSERT: `resolvedSelector.match(key2)`. |
There was a problem hiding this comment.
What is the role of these assertions when key1 and key2 are unused later on?
Implement Mark Davis's suggested pattern selection algorithm in the spec. Update the "resolved values" example to use the
match()andcompare()methods instead ofselectKeys().Rewrite the numeric and
:stringselector specs accordingly.