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

Select "Match on variables instead of expressions" for selection-declarations #824

Merged
merged 3 commits into from
Aug 28, 2024
Merged
Changes from all commits
Commits
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
78 changes: 71 additions & 7 deletions exploration/selection-declaration.md
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
# Effect of Selectors on Subsequent Placeholders

Status: **Proposed**
Status: **Proposed, Ballot Requested**

<details>
<summary>Metadata</summary>
Expand All @@ -10,7 +10,8 @@ Status: **Proposed**
<dt>First proposed</dt>
<dd>2024-03-27</dd>
<dt>Pull Requests</dt>
<dd>#000</dd>
<dd><a href="https://github.com/unicode-org/message-format-wg/pull/755">#755</a></dd>
<dd><a href="https://github.com/unicode-org/message-format-wg/pull/824">#824</a></dd>
</dl>
</details>

Expand Down Expand Up @@ -149,14 +150,11 @@ _What prior decisions and existing conditions limit the possible design?_

## Proposed Design

_Describe the proposed solution. Consider syntax, formatting, errors, registry, tooling, interchange._
The design alternative [Match on variables instead of expressions](#match-on-variables-instead-of-expressions)
Copy link
Collaborator

Choose a reason for hiding this comment

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

The suggested edit to change the selected alternative is based on my & others' previous comments and meeting discussion. See other comment threads in the PR for specifics.

Suggested change
The design alternative [Match on variables instead of expressions](#match-on-variables-instead-of-expressions)
The design alternative [Do nothing](#do-nothing)

Copy link
Collaborator

Choose a reason for hiding this comment

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

There is also PR #867

I suggested an option there:
#867 (comment)

But we discuss the same thing in so many places that I don't know where to comment anymore...
Let me know if you want me to copy it here.

Copy link
Collaborator

Choose a reason for hiding this comment

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

@mihnita I created #867 to create a quick update to the design doc to reflect the discussion that came up in Monday's 2024-08-19 meeting, as @aphillips's suggested then, and because I couldn't make them as suggested changes in this PR since it's outside the bounds of Github's diff view. I think a separate PR is the way to do it.

(Btw, regarding the idea itself from your comment, I agree with @bearfriend's comment in response. I have more explanation to offer to that position. I can share that with you in a 1:1 chat in case that would affect whether you want to pursue a small PR that adds the option to the design doc so that it gets equal consideration.)

Copy link
Member

Choose a reason for hiding this comment

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

That discussion is not very clear. Can you boil it down to a reasonable use case where the selection should differ from the formatting semantics (eg disregarding different number systems).

Copy link
Collaborator

Choose a reason for hiding this comment

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

Isn't that covered by the added example?

.match {$person :gender}
male {{Bienvenido {$person :personName}}}
female {{Bienvenida {$person :personName}}}
other {{Le damos la bienvenida {$person :personName}}}

One can also imagine similar patterns for non-complex inputs such as {$time :partOfDay} (morning/afternoon/evening/night), {$adjective :affect}, or really any kind of "binning" classification into finite categories for distinct treatment that still needs the original value.

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 find the "Do Nothing" alternative problematic because it does not signal anything wrong with a message like

.match {$count :integer}
one {{You have {$count} whole apple.}}
* {{You have {$count} whole apples.}}

If that message is formatted with { count: 1.2 }, the result is "You have 1.2 whole apple." which is clearly not what was intended by its author.

This problem is the main reason that I filed #736 originally, and which I think we need some solution for. As we've discussed on many occasions, MF2 will always be an auxiliary language to its users, and we should not expect users to always remember its details. Therefore, I think it's important that we find some way for the above message to be considered invalid, so that this mistake is caught as early as possible, or we add a side effect to .match so that the desired behaviour is realised.

My own first preference is the alternative proposed in this PR, but I've also filed #860 to provide a next-best choice that retains our current syntax, but signals an error in this particular case, where a selector variable is used in a placeholder.

Copy link
Member

Choose a reason for hiding this comment

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

Thinking about this some more. My proposal is:

  • in a match statement, options effectively mutate variables (see below)
  • it is illegal for a variable to occur twice in a match statement.

This keeps most message more concise, producing the expected results: eg without the problems that occur above with input of count : 1.2

Example1.

.match {$count :integer}
one {{You have {$count} whole apple.}}
* {{You have {$count} whole apples.}}

is precisely equivalent to:

.local $count2 {$count :integer}
.match {$count2}
one {{You have {$count2} whole apple.}}
* {{You have {$count2} whole apples.}}

Example 2.
And the following is illegal.

.match {$count <anything>}{$count <anything>}

The message author is required to rewrite it explicitly, eg to:

.local $count3 {$count}
.match {$count <anything1>}{$count3 <anything2>}

I really don't think this is a burden, since the number of times the same variable is used twice in a match (or the older Select) is vanishingly small. Since it is an error — and the advice to fix is easy — that will prevent misbehavior.

Copy link
Member

Choose a reason for hiding this comment

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

@macchiati

I really don't think this is a burden, since the number of times the same variable is used twice in a match (or the older Select) is vanishingly small.

This is true of numbers. Dates offer an alternative... or person names in which one might want multiple selection from a complex value.

Note: I'm not arguing against the proposal.

described below is selected.
Comment on lines +153 to +154
Copy link
Member

Choose a reason for hiding this comment

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

I don't think I agree with this design alternative, since it requires the use of declarations.

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 way I see it, that's the least worst option that we have available. :/

Copy link
Member

Choose a reason for hiding this comment

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

Okay, but it's a breaking change. The other alternatives have quirks for sure (and I think the mutable/immutable labels might be backwards??), but we need to seek consensus.

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 agree. On this topic, it seems like we've been avoiding expressing any preferences one way or another for a while, so I didn't know of a better way to start seeking that consensus than proposing one solution above the others. The PR is structured in a way that it should be relatively easy for us to change the selection from my initial proposition, should our consensus land elsewhere.

Copy link
Collaborator

Choose a reason for hiding this comment

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

I prefer a different alternative, "Do nothing", that others also share, as pointed out in this comment. We also discussed it in Monday's 2024-08-19 meeting.

The concern with the following set of alternatives is that they propose doubling up the functionality of .match selectors to also behave like a declaration for the input operand to define implicit function option values.

  • "Allow both local and input declarative selectors with immutability"
  • "Allow mutable input declarative selectors"
  • "Allow immutable input declarative selectors"

They are also implicitly creating a new "lexical scope", if you will, that is needed for this implicit binding (declaration).

The concern with the alternative "Match on variables instead of expressions"
is that it is designed for and assumes the scenario that formatting needed for selection and formatting needed for placeholders in the pattern are the same. As Use Case 6 points out, not only can the formatting options differ -- because selection and formatting are distinct operations -- but we might have different formatting functions altogether. The strictness of this option is not appropriate since it's based on an assumption that is not universally true in practice (although I appreciate the good intent behind the idea).

The concerns with "Provide a #-like Feature" are the obvious strong undesirability of positional arguments and the complexity & downgrade to readability that brings. The last time someone discussed positional arguments in earnest here in MFWG was perhaps early 2020...around 4+ years ago, because there was quick unanimous alignment around named arguments.

Copy link
Collaborator

Choose a reason for hiding this comment

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

On the point of formatting for selection != formatting for display, I dug up this "virtual whiteboard" doc from older times of MFWG when we couldn't meet in person (July 2020, to be exact), in which @mihnita and I had a kind of Socratic dialogue where Mihai explained to me through examples why selectors and placeholders are distinct operations and thus cannot be collapsed into a single construct type in the data model.


## Alternatives Considered

_What other solutions are available?_
_How do they compare against the requirements?_
_What other properties they have?_

### Do nothing

In this alternative, selectors are independent of declarations.
Expand Down Expand Up @@ -358,3 +356,69 @@ and a data model error otherwise.
Removes some self-documentation from the pattern.
- Requires the pattern to change if the selectors are modified.
- Limits number of referenceable selectors to 10 (in the current form)

### Hybrid approach: Match may mutate, no duplicates

In this alternative, in a `.match` statement:

1. variables are mutated by their annotation
2. no variable can be the operand in two selectors

This keeps most messages more concise, producing the expected results in Example 1.

#### Example 1

```
.match {$count :integer}
one {{You have {$count} whole apple.}}
* {{You have {$count} whole apples.}}
```
is precisely equivalent to:

#### Example 2
```
.local $count2 = {$count :integer}
.match {$count2}
one {{You have {$count2} whole apple.}}
* {{You have {$count2} whole apples.}}
```

This avoids the serious problems with mismatched selection and formats
as in Example 1 under "Do Nothing", whereby the input of `count = 1.2`,
results the malformed "You have 1.2 whole apple."

Due to clause 2, this requires users to declare any selector using a `.input` or `.local` declaration
before writing the `.match`. That is, the following is illegal.

#### Example 3
```
.match {$count <anything>}{$count <anything>}
```
It would need to be rewritten as something along the lines of:

#### Example 4
```
.local $count3 = {$count}
.match {$count <anything1>}{$count3 <anything2>}
```
Notes:
- The number of times the same variable is used twice in a match (or the older Select) is vanishingly small. Since it is an error — and the advice to fix is easy — that will prevent misbehavior.
- There would be no change to the ABNF; but there would be an additional constraint in the spec, and relaxation of immutability within the .match statement.

**Pros**
- No new syntax is required
- Preserves immutability before and after the .match statement
- Avoids the serious problem of mismatch of selector and format of option "Do Nothing"
- Avoids the extra syntax of option "Allow both local and input declarative selectors with immutability"
- Avoids the problem of multiple variables in "Allow immutable input declarative selectors"
- Is much more consise than "Match on variables instead of expressions", since it doesn't require a .local or .input for every variable with options
- Avoids the readability issues with "Provide a #-like Feature"

**Cons**
- Complexity: `.match` means more than one thing
- Complexity: `.match` implicitly creates a new lexical scope
- Violates immutability that we've established everywhere else
- Requires additional `.local` declarations in cases where a variable would occur twice
such as `.match {$date :date option=monthOnly} {$date :date option=full}`