Skip to content
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

Clarify that calling MatchSelectorKeys always returns a list #828

Merged
merged 1 commit into from
Jul 22, 2024

Conversation

eemeli
Copy link
Collaborator

@eemeli eemeli commented Jul 15, 2024

Follow-up to #816 (comment), as it was not previously clear that MatchSelectorKeys never fails.

@aphillips
Copy link
Member

The description of this PR and the text seem different? MatchSelectorKeys "never fails" by emitting an error and returning the empty list?

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.

Suggestion for the fallback handling.

@@ -549,6 +549,10 @@ The returned list MAY be empty.
The most-preferred key is first,
with each successive key appearing in order by decreasing preference.

If calling MatchSelectorKeys encounters any error,
a _Bad Selector_ error is emitted
and an empty list is returned.
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
and an empty list is returned.
and an empty list is returned.
The fallback representation of such a _message_ is
the _fallback value_ for the _message_.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

That correction does not match what the algorithm results in. If we get an empty list here, we end up selecting the * variant.

@aphillips aphillips merged commit 5e46aaa into main Jul 22, 2024
1 check passed
@aphillips aphillips deleted the resolve-on-error branch July 22, 2024 17:26
eemeli added a commit to messageformat/messageformat that referenced this pull request Aug 21, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants