-
-
Notifications
You must be signed in to change notification settings - Fork 34
Commit
This commit does not belong to any branch on this repository, and may belong to a fork outside of the repository.
- Loading branch information
Showing
1 changed file
with
215 additions
and
0 deletions.
There are no files selected for viewing
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,215 @@ | ||
# 22 July 2024 | MessageFormat Working Group Teleconference | ||
|
||
### Attendees | ||
- Addison Phillips - Unicode (APP) - chair | ||
- Elango Cheran - Google (ECH) | ||
- Mihai Niță - Google (MIH) | ||
- Luca Casonato - Deno (LCA) | ||
- Harmit Goswami - Mozilla (HGO) | ||
- Eemeli Aro - Mozilla (EAO) | ||
- Staś Małolepszy - Google (STA) | ||
- Tim Chevalier - Igalia (TIM) | ||
- Matt Radbourne - Bloomberg (MRR) | ||
|
||
Scribe: HGO | ||
## Topic: Info Share | ||
|
||
(none) | ||
|
||
## Topic: Tech Preview | ||
Let’s review the Task List: | ||
|
||
https://github.com/unicode-org/message-format-wg/wiki/Things-That-Need-Doing | ||
|
||
## Topic: Result of balloting for ‘error handling’ (#830) | ||
|
||
[APP] : Issue 830 has the votes, 7 had #1 ranked first so that won. #3 was second, and #4 was third. Any objections? | ||
|
||
(No objections, wg consensus) | ||
|
||
[APP] : I’ll mark up the PRs, two of which are of particular interest from this vote. Elango can you change #804? | ||
|
||
[ECH] : I can update it | ||
|
||
[APP] : 816 is the next one. Eemeli can you make changes and the rest of us can review? | ||
|
||
[EAO]: we can mark it as ready-for-review right now, and others can look at it later. | ||
|
||
[EAO]: A lot of discussion was about formalizing error codes. Currently, error codes are (and should be) kebab-case and camelCase of the error type. Should the error code be included in the spec? | ||
|
||
[APP] : I think consensus is you must signal and implement fallback for errors. So far, I haven't heard us being normative about emitting errors. I think we should make the error codes more formal | ||
|
||
[EAO]: This affects the spec since some versions of the PR say you must provide a way to get the ‘name’ of the error, but we say you must ‘identify’ the error. We should specify if we want ‘error code’ | ||
|
||
[APP] : Throwing an error should be enough. We don’t need people to implement an error as a type, since it adds too much complexity for its value. | ||
|
||
[ECH] : Agree with APP. In ICU4C we have enu for different error types, but to convert to an identifier, I don’t see that being useful | ||
|
||
[EAO]: I want to clarify that in the current test suite, we say you must end up with an error that ends up with a `bad-operand` code. Should this be connected to the badOperand error or is this implied? | ||
|
||
[MIH]: Case is framework/language dependent, so error codes are probably more helpful for [...] | ||
|
||
[LCA]: Another use case can be logging/diagnostic errors, but I don’t think it’s super important either | ||
|
||
[APP] : I think it makes sense for us to say in a non-normative way, this is how we annotate our text, so they know how it links to the spec. I agree with EAO that we should have kebab-case next to the error. Matching is a good idea, but not required | ||
|
||
[EAO]: The test README can explain which errors correspond to which codes | ||
|
||
[STA]: as a user, can I specify [...] | ||
|
||
[EAO]: Users will know that there is a bad operand, but they won’t get a specific error code for ‘bad operand’. We don’t specify if it's a string, or an instance of error ‘BadOperand’, etc. | ||
|
||
[EAO]: Should we go through the spec and ensure we don’t have any errors where an implementation ‘SHOULD’ emit | ||
|
||
[APP] : We should go through that. I can’t think of anywhere where it may be, but there could be instances in the spec still. | ||
|
||
[EAO]: Let’s take this to a future possible PR that I or someone else can file | ||
|
||
[APP] : I think you already started a PR, but every failure should be clear about fallback representation because of our latest vote. I think we already have that | ||
|
||
## Topic: PR Review | ||
|
||
Timeboxed review of items ready for merge. | ||
|
||
Key discussions today should focus on: | ||
|
||
- Selection declarations (#824) | ||
- Function composition (#823, #814, #728, #646) | ||
|
||
If time permits: | ||
|
||
- Whitespace/bidi handling (#811, #673) | ||
- Contextual options or expression attributes (#780) | ||
|
||
|
||
# 824 | ||
|
||
[APP] : EAO has made this PR to force us to talk about which selection declaration we should choose. He suggests a change in syntax to match on variables instead of expressions, but we haven’t discussed this yet. | ||
|
||
[EAO]: I’m happy to leave this until the next call as long as people look at it. No one is sharing their opinions on this, so I proposed my preferred solution, but we can return to this later once people have reviewed it. | ||
|
||
[APP] : Sounds good. I’m suspicious that some instances of ‘mutable’ and ‘immutable’ may be backwards in the doc though. | ||
|
||
# 813 | ||
|
||
[APP] : We agreed that medium should be the default date-style in last week's call. EAO has updated the PR. Any objections to merging? | ||
|
||
[None, merged] | ||
|
||
# 799 | ||
|
||
[APP] : Are we waiting for any further discussions? STA and ECH had comments open. | ||
|
||
[EAO]: We got stuck last week talking about this. I can’t remember our conclusion but I can review the comments for next week’s meeting. | ||
|
||
# 828 | ||
|
||
[EAO]: Can this be approved and merged? | ||
|
||
[APP] : I’m willing to merge if someone else approves | ||
|
||
[EAO]: tldr; currently, the match-selector-keys method doesn’t handle if user-defined, custom-code throws an error or unexpected exception. This spec selects the * variant. | ||
|
||
[TIM]: I just approved it. | ||
|
||
[APP] : Merged! | ||
|
||
# 780 | ||
|
||
[APP] : EAO proposes to add built-in contextual options that every implementation must provide for any expression. I prefer having this in @ expression-attributes, since this wouldn’t interfere with anyone’s implementations. Can I merge this as proposed and we discuss more next week? | ||
|
||
[All agreed, added to agenda+] | ||
|
||
# 829 | ||
|
||
[APP] : This is a change to our spec, in the PR comments I suggest changing the stability promise in a much different way. I don’t think we can promise what we currently have. We should only promise not removing options/option values, and that previously valid messages won’t become invalid. | ||
|
||
[APP] : Should I create a new PR with just my change? Since this makes EAO’s spec moot | ||
|
||
[EAO]: I’d rather we kept them separate. My change alters the promise for custom option values. This should be a valid small change before talking about your bigger proposed change | ||
|
||
[APP] : If you use a defined option, that shouldn’t become an error in the future | ||
|
||
[EAO]: Think about currency. We don’t require it to be defined in a certain way, so no error is thrown for a made-up currency. In the future, we should, for example, have preset currency options, but then we can’t make the promise that there will be no error in case users make a custom currency | ||
|
||
[MIH]: I understand what you are saying but it’s not clear from the PR. We can also force this in a linter, and avoid burdening the runtime having to check for this | ||
|
||
[EAO]: Are you saying that options without namespace should be ignored? | ||
|
||
[MIH]: Yes. This would be better for cross-device functionality as well. | ||
|
||
[EAO]: Can you comment on the PR so we can discuss and debate more on this issue? [MIH]: sure | ||
|
||
[APP] : It’s important to consider unrecognized options right now. I can see currency being a predefined option, and see the arguments in that case. I can merge the PR because I’ll rip the stability promise sentences out anyways. | ||
|
||
[EAO]: Any objections to merging? | ||
|
||
[ECH] : Yes, if you’re going to remove this anyways APP then this discussion feels moot, so we can just close it | ||
|
||
# 827 | ||
|
||
[EAO]: If we get an approval from MRR, then can we merge this after the meeting? | ||
|
||
[APP] : Sure thing | ||
|
||
# 825 | ||
|
||
[APP] : EAO wants to rename quoted/unquoted to quoted literal/unquoted literal, to make it more similar to numbers. Any objections? | ||
|
||
[All approved, merged] | ||
|
||
|
||
## Topic: Issue review | ||
Let’s close all of the issues | ||
https://github.com/unicode-org/message-format-wg/issues?q=is%3Aissue+is%3Aopen+label%3ALDML45 | ||
|
||
https://github.com/unicode-org/message-format-wg/issues | ||
|
||
Currently we have 61 open (was 66 last time). | ||
- 18 are Preview-Feedback | ||
- 2 are resolve-candidate and proposed for close. | ||
- 1 are Agenda+ and proposed for discussion. | ||
- 1 is a ballot (now closed) | ||
|
||
# 831 | ||
|
||
[APP] : Any objections to closing? | ||
|
||
[None, closed] | ||
|
||
# 821 | ||
|
||
[APP] : EAO and I both responded to this, this is a request to allow syntactic sugar for things like arrays. Our feedback was our syntax doesn’t support this. Any comments? | ||
|
||
[None, closed] | ||
|
||
# 809 | ||
|
||
[APP] : LCA, this is your issue. We partially addressed this with the trailing whitespace PR. Any discussion comments? | ||
|
||
[LCA]: When you have a string literal with a MF2 message, this message is complex. I think it sucks that whitespaces aren’t allowed. This inevitably results in a new line if you want to preserve indents. The look-ahead for simple v.s. Complex messages means that asking for a look-ahead for leading whitespace shouldn’t be too bad to implement | ||
|
||
[ECH] : We talked about this a few months ago. As a developer, you should be able to handle precision about spaces in the code you write. Different languages handle multi-line strings differently and have varying levels of support. Since JavaScript doesn’t do this well, then that’s a JS problem. JSON multi-line strings aren’t nice but one or two languages shouldn’t dictate our design | ||
|
||
[APP] : As a reminder, our wg consensus is: for whitespaces, you need to look ahead until you see an unambiguous non-simple message start, and you might still need escapes. Therefore, it becomes a much farther look-ahead than the one we currently have. | ||
|
||
[MIH]: I think the position in other discussions was that this debate belongs in each respective templating language. It’s the job of the templating engine to ensure the text can properly compile before the next level. | ||
|
||
[EAO]: I agree with LCA, we should have whitespace at the start. Because of BiDi, there will be a look-ahead of up to 3 characters (at least). If we decide against this, that’s okay, but as long as we don’t care about the particularity of string handling when the string is embedded in another language. | ||
|
||
[LCA]: I wanted to reply to the comments. I don’t think that we should change this just because of JS, but because whitespace is not meaningful outside of quotes. A complex message is closer to a function body in a programming language, where whitespaces between declarations after the final declaration are not significant. This would be more familiar and intuitive to programmers. | ||
|
||
[ECH] : One of the issues with allowing whitespace is we don’t know if it's simple or complex. The look-ahead, along with potential escaping, will be much too complex. Adding something like this could affect another part of the spec, since our spec is kind of sensitive. I don’t think we should treat MF2 as a programming language. | ||
|
||
[APP] : I differ with ECH slightly, we want to permit unquoted plain strings to be messages, but then they cannot be trimmed. Complex messages can be trimmed and normalized. Simple messages currently have nothing around them in our spec, but there could be changes made to allow whitespaces. Although, this can be tricky so we need to be careful. | ||
|
||
[LCA]: I implemented this in my Rust implementation, in such a way that this doesn’t affect simple messages. I didn’t find it too difficult to implement, since the unbounded look-ahead only happens at the start of the message. Additionally, this is a mistake I made when making some tests early on, so I wanted to make sure others don’t make the mistake I do. | ||
|
||
[STA]: I don’t really remember all the nuance between our messages. For simple messages do we trim or preserve white space? [APP] : We preserve. | ||
|
||
[STA]: The other comment I have is that, not taking sides, but our Syntax.md says [...] | ||
|
||
|
||
## Topic: AOB? | ||
|
||
|