-
-
Notifications
You must be signed in to change notification settings - Fork 36
Add consensus decisions #154
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
Conversation
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.
Could you add more background so that someone coming in and reading this doc for the first time can understand what this means? Issue number references would also help.
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.
Looking at this again with fresh eyes there were a few parts of this that I found difficult to follow. I have some suggestions that might help with readability without changing the substance of the consensus.
Co-authored-by: Dan Minor <[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.
Apart from the one suggested change, everything looks good to me.
And thanks for finding links to decisions, that is very handy.
Co-authored-by: Elango <[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.
I'm not going to block this PR, but before I approve it, I'd like to ask @zbraniecki to reconsider the wording of consensus 6.
I'd be much more comfortable with something along the lines of @dminor's suggestion, and I think the reason for that is that it's easier for me to agree with a statement about the present (…does not intend to block the addition…) than it is to agree with a statement about the future (…will attempt to avoid blocking addition…). In the latter, what am I really signing up for? How can we verify in the future that we have attempted hard enough?
I believe the value I'm seeking out of that statement is far outweighed by the amount of energy and brainpower my excellent colleagues devoted to it. I'm comfortable with @dminor's rewording and it seems to help @stasm. Let's do this and advance :) |
@zbraniecki I agree with this statement but would stress that, while rough consensus and running code form the core of good standards (I'm borrowing IETF's koan here on purpose), technical considerations should also inform our choices. One right outlying opinion ought to outweigh 1000 badly formed consensus opinions. (judging "rightness" being, of course, the problem :-)) |
This is a prerequisite for top-level selectors to be able to represent complex messages, without requiring those messages to be split up in an unergonomic manner. | ||
This is an extension or relaxation of what's allowed in MessageFormat 1. | ||
|
||
While message references make it technically possible for the data model to represent multi-argument selectors otherwise, this requires the use of n²-1 artificial "messages", where n is the number of arguments. This is not desirable. |
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 artificiality/undesirability of the n^2-1 messages is in the eye of the beholder. While it is the case that nested select/plural structures beyond a couple of levels produces a lot of rather repetitious (nearly identical but often subtly different--particularly in a forgiving language like English) messages, this is not necessarily undesirable. When the localization system can participate actively, the burden on translators can be reduced.
I am not arguing for folks to have 10-level nested structures, please note. The size of the data is a problem and consistency management becomes a chore. But I often see teams writing 2-3 levels of nest. The best part is, we can make tools that understand this stuff and make it easier to author and it is something I can teach to developers. If we didn't provide this, developers would go back to their bad old ways of writing switches in code or doing substring replacement (grammatical consistency forgotten).
I don't have an alternate suggestion for wording just now, but this is something I have my eye on.
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.
It also occurs to me that I may not understand the term "nested" in this context. For me, this is a "nested" structure (in this case a select in a plural--I copied this from a code review and I'd have written it with the select on the outside, but still...):
{
"value": {
"param": "unreadCount",
"pluralItems": {
"=0": {
"param": "msgType",
"selectItems": {
"email": "Hello {name}, you have no emails in your inbox.",
"notification": "Hello {name}, you have no notifications in your inbox.",
"other": "Hello {name}, you have no new items in your inbox."
}
},
"one": {
"param": "msgType",
"selectItems": {
"email": "Hello {name}, you have {unreadCount} email in your inbox.",
"notification": "Hello {name}, you have {unreadCount} notification in your inbox.",
"other": "Hello {name}, you have {unreadCount} item in your inbox."
}
},
"other": {
"param": "msgType",
"selectItems": {
"email": "Hello {name}, you have {unreadCount} emails in your inbox.",
"notification": "Hello {name}, you have {unreadCount} notifications in your inbox.",
"other": "Hello {name}, you have {unreadCount} item in your inbox."
}
}
}
}
}
Is this what we mean by nested? Or would nested mean the more classical MessageFormat 1 like:
Hello {name}, you have {unreadCount,plural, =0 {{msgType,select, email {{unreadCount,number,integer} emails}} one ...// etc... won't bother to type the whole thing
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 current aim is not to allow for one selector to be inside another selector in the same message, instead enabling the representation of messages such as your example by having that top-level selector be able to use more than one variable (e.g. both unreadCount
and msgType
) to select one of the cases. Here's what your message would look like with one of the data model candidates we're developing:
{
id: 'unread-somethings',
value: {
select: [
{ func: 'plural', args: [{ var_path: ['unreadCount'] }] },
{ var_path: ['msgType'] }
],
cases: [
{
key: [0, 'email'],
value: ['Hello ', { var_path: ['name'] }, ' you have no emails in your inbox.']
},
{
key: [0, 'notification'],
value: ['Hello ', { var_path: ['name'] }, ' you have no notifications in your inbox.']
},
{
key: [0, 'other'],
value: ['Hello ', { var_path: ['name'] }, ' you have no new items in your inbox.']
},
{
key: ['one', 'email'],
value: ['Hello ', { var_path: ['name'] }, ' you have ', { var_path: ['unreadCount'] }, ' email in your inbox.']
},
{
key: ['one', 'notification'],
value: ['Hello ', { var_path: ['name'] }, ' you have ', { var_path: ['unreadCount'] }, ' notification in your inbox.']
},
{
key: ['one', 'other'],
value: ['Hello ', { var_path: ['name'] }, ' you have ', { var_path: ['unreadCount'] }, ' item in your inbox.']
},
{
key: ['other', 'email'],
value: ['Hello ', { var_path: ['name'] }, ' you have ', { var_path: ['unreadCount'] }, ' emails in your inbox.']
},
{
key: ['other', 'notification'],
value: ['Hello ', { var_path: ['name'] }, ' you have ', { var_path: ['unreadCount'] }, ' notifications in your inbox.']
},
{
key: ['other', 'other'],
value: ['Hello ', { var_path: ['name'] }, ' you have ', { var_path: ['unreadCount'] }, ' items in your inbox.']
}
]
}
}
The n^2 -1 reference in the text is for one of the alternative representations of this message, where we avoid using more than one argument for a selector by extracting variants into separate messages, and one of those is then picked by a parent message. Kind of like this:
{
id: 'unread-emails',
value: {
select: [{ func: 'plural', args: [{ var_path: ['unreadCount'] }] }],
cases: [
{
key: [0],
value: ['Hello ', { var_path: ['name'] }, ' you have no emails in your inbox.']
},
{
key: ['one'],
value: ['Hello ', { var_path: ['name'] }, ' you have ', { var_path: ['unreadCount'] }, ' email in your inbox.']
},
{
key: ['other'],
value: ['Hello ', { var_path: ['name'] }, ' you have ', { var_path: ['unreadCount'] }, ' emails in your inbox.']
}
]
}
},
{
id: 'unread-notifications',
value: {
select: [{ func: 'plural', args: [{ var_path: ['unreadCount'] }] }],
cases: [
{
key: [0],
value: ['Hello ', { var_path: ['name'] }, ' you have no notifications in your inbox.']
},
{
key: ['one'],
value: ['Hello ', { var_path: ['name'] }, ' you have ', { var_path: ['unreadCount'] }, ' notification in your inbox.']
},
{
key: ['other'],
value: ['Hello ', { var_path: ['name'] }, ' you have ', { var_path: ['unreadCount'] }, ' notifications in your inbox.']
}
]
}
},
{
id: 'unread-items',
value: {
select: [{ func: 'plural', args: [{ var_path: ['unreadCount'] }] }],
cases: [
{
key: [0],
value: ['Hello ', { var_path: ['name'] }, ' you have no new items in your inbox.']
},
{
key: ['one'],
value: ['Hello ', { var_path: ['name'] }, ' you have ', { var_path: ['unreadCount'] }, ' item in your inbox.']
},
{
key: ['other'],
value: ['Hello ', { var_path: ['name'] }, ' you have ', { var_path: ['unreadCount'] }, ' items in your inbox.']
}
]
}
},
{
id: 'unread-somethings',
value: {
select: [{ var_path: ['msgType'] }],
cases: [
{ key: ['email'], value: [{msg_path: ['unread-emails']}] },
{ key: ['notification'], value: [{msg_path: ['unread-notifications']}] },
{ key: ['other'], value: [{msg_path: ['unread-items']}] }
]
}
}
The "actual" message is still the last one, but with only one argument being used for the top-level selectors, the three others are also required.
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.
@eemeli thanks for this. I had followed the discussion of matrixed selectors elsewhere, which is why I hadn't come out of the woodwork with comments earlier. Your top example and mine are just "misspellings" of each other--functionally equivalent while being syntactically different. As long as I was mentally able to map the two functionally, I wasn't concerned. When I saw and paused to read the proposed consensus, though, I went "huh? must have missed something here..."
This is a prerequisite for top-level selectors to be able to represent complex messages, without requiring those messages to be split up in an unergonomic manner.
I think the more relevant detail here is: "This allows selectors to represent complex messages while avoiding linguistically problematic constructs that can occur when selectors operate only part of a message" (... such as found in MF1)
This is an extension or relaxation of what's allowed in MessageFormat 1.
I don't find this sentence helpful. This doesn't relax any constraints of MF1 (since it's a separate syntax altogether). I suppose it could be "an extension". But really it's just different from the philosophy of MF1.
While message references make it technically possible for the data model to represent multi-argument selectors otherwise, this requires the use of n²-1 artificial "messages", where n is the number of arguments. This is not desirable.
The latter example is a logical outgrowth of message references. But I don't see how the proposed text leads here: this should be reserved for a discussion of references, as it doesn't have anything to do with number 3?
See PR discussion for details: https://github.com/unicode-org/message-format-wg/pull/154/files#r580985173 Co-authored-by: Dan Minor <[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.
(setting review bit)
Proper fix for #154 (comment)
Co-authored-by: Stanisław Małolepszy <[email protected]>
@@ -66,4 +66,4 @@ The group believes that the known value of this feature can be sufficiently cove | |||
**Discussion:** | |||
The cost analysis of the nested selectors feature was performed in the absence of sufficient in-field experience of use in production systems. | |||
In result, the group's decision to not currently incorporate the feature is based on the lack of sufficient known value that would require them, which the group recognizes may change in the future. | |||
In result, it is the intent of the group to attempt to design MessageFormat 2 in such a way, that wouldn't block future revisions of the standard to be extended with nested selectors feature. | |||
In result, it is the intent of the group to design MessageFormat 2 in a way that wouldn't prevent future revisions of the standard to be extended with nested selectors feature. |
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 reads oddly. I would suggest saying this in a positive way:
As a result, the design of MessageFormat 2 is being created in such a way as to potentially allow as to allow future revisions of the standard to be extended with the nested selectors feature.
Or, if we prefer to talk about group intentions:
As a result, it is the intention of the group to design MessageFormat 2 in a way that might allow future versions of the standard to provide a nested selectors feature.
@@ -66,4 +66,4 @@ The group believes that the known value of this feature can be sufficiently cove | |||
**Discussion:** | |||
The cost analysis of the nested selectors feature was performed in the absence of sufficient in-field experience of use in production systems. | |||
In result, the group's decision to not currently incorporate the feature is based on the lack of sufficient known value that would require them, which the group recognizes may change in the future. |
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.
Both this paragraph and the following one say "in result". This is also oddly worded. I'd suggest:
Because the inclusion of nested selectors is not a blocker for the initial release and because the need for nested selectors remains subject to debate, ultimately the group decided not to incorporate the feature into this version of the standard.
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.
@aphillips Would you be open to having the current text be accepted provisionally in this PR, and then improving the language of Consensus 6 in a separate PR? I'm getting the sense that this one is a bit more divisive than the rest, and that it might be good to have a more focused discussion on its particulars.
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.
Go for it, although these suggestions are more about English prosody than content and I think we've about wrung any additional value out of this thread.
As promised, merging this before next Monday's meeting, now that it looks like we've reached consensus on this consensus. 🎉 |
Proper fix for #154 (comment)
Resolves #103, resolves #127, resolves #137
As discussed in recent meetings, we have reached consensus on a number of issues, and this ought to be well documented and made more explicit. This PR adds a new document,
docs/consensus_decisions.md
, which enumerates said decisions, along with some clarifying discussion.The first two were originally documented in the minutes of the third meeting of the #103 taskforce and were approved at the November meeting of the workgroup, while the last four are from the discussion on issue #137 (where "3" and "4" are referred to as "1" and "2"), and were last approved at the February meeting of the workgroup.
Even though technically we've already agreed on all of this, it would be useful to communicate your approval about all this on this issue. (Switch to the "Files changed" tab, click on "Review changes", select "Approve", and then "Submit review")
The intent is to get this merged sometime before the next workgroup meeting, which is on 15 March.