Review data model #651

mihnita · 2024-02-15T05:16:12Z

Addresses issue #639

I will also update the .json and .dtd, but after we get an agreement on this.

Addresses issue #639 I will also update the .json and .dtd, but after we get an agreement on this.

mihnita · 2024-02-15T05:18:22Z

I don't consider it 100% done, but it is the result of a first (slow and careful) pass.
My plan is to take a break, and take another look.
But I thought it might be good to give some extra time to our colleagues in Europe.

aphillips

Thanks for this.

The top part looks like it needs more work. Other parts looks like they might be in better harmony with the syntax. I didn't comment fully.

aphillips · 2024-02-15T05:26:03Z

spec/data-model/README.md

+A `SimpleMessage` corresponds to a syntax message that includes _selectors_.
+A message without _selectors_ and with a single _pattern_ is represented by a `ComplexMessage`.


This one is backwards.

Suggested change

A `SimpleMessage` corresponds to a syntax message that includes _selectors_.

A message without _selectors_ and with a single _pattern_ is represented by a `ComplexMessage`.

A `SimpleMessage` is a _message_ without _selectors_ and with a single _pattern_.

A `ComplexMessage` is any _message_ that includes one or more _declarations_, a _selector_, or both.

eemeli

While the changes proposed here bring the data model much closer to the MF2 syntax, they also make it less useful for any other purpose. None of the proposed changes are presented with any discussion about why they are being made, and they are sufficiently extensive that I would expect a design document discussing them, should we wish to consider their incorporation in the spec.

In short, this does not appear to be a review of the data model, but a rewriting of it. I do not think that can be in scope for our consideration this week.

eemeli · 2024-02-15T08:49:24Z

spec/data-model/README.md


 ```ts
-type Message = PatternMessage | SelectMessage;
+type Message = SimpleMessage | ComplexMessage;


Why this change in representation? This seems wholly unnecessary, as it links the data model with syntactic details like the representation of a message with a single pattern depending on whether it starts with a . or not.

I would have expected a change like this to have at least some argumentation about why it's being made, and some consideration of how it's impacting data model users and operations.

eemeli · 2024-02-15T08:50:45Z

spec/data-model/README.md

-An `UnsupportedStatement` represents a statement not supported by the implementation.
+An `ReservedStatement` represents a statement not supported by the implementation.


Why this rename? Again, I would expect some argumentation here.

eemeli · 2024-02-15T08:51:55Z

spec/data-model/README.md

-  value?: string;
+  value: Pattern;


This seems like a misunderstanding of this data model entry.

Specifically: I think it should be string because the intention here is for it to be a different string representing the catchall key in some syntax other than MF2. And it should be optional.

catamorphism · 2024-02-19T19:04:25Z

spec/data-model/README.md


 In the syntax,
-a `PatternMessage` may be represented either as a _simple message_ or as a _complex message_,
+a `ComplexMessage` may be represented either as a _simple message_ or as a _complex message_,


I think it's confusing to use "complex" to mean two different things (in the data model, having only a single pattern, regardless of declarations; in the concrete syntax, having no declarations and no selectors.)

I don't love the terms PatternMessage and SelectMessage, but I agree with Eemeli that it's probably best to keep it that way.

catamorphism · 2024-02-19T19:05:05Z

spec/data-model/README.md

+
+interface ComplexMessage {
+  type: "complex-message";
+  declarations?: Declaration[];


This should be declarations: Declaration[] per #633

catamorphism · 2024-02-19T19:06:07Z

spec/data-model/README.md

 with its _expression_ `value`.
-The `name` does not include the initial `$` of the _variable_.
+The `name` does not include the `$` sigil of the _variable_,
+or the starting and ending `|` of the literals.


I'm confused about this -- since this part is describing what appears on the left-hand side of a declaration, where do literals come in?

catamorphism · 2024-02-19T19:06:50Z

spec/data-model/README.md

-the `keys` and `value` of each _variant_ are represented as an array of `Variant`.
-For the `CatchallKey`, a string `value` may be provided to retain an identifier.
-This is always `'*'` in MessageFormat 2 syntax, but may vary in other formats.
+In a `Matcher` the `keys` and `value` of each _variant_ are represented as an array of `Variant`. \


Suggested change

In a `Matcher` the `keys` and `value` of each _variant_ are represented as an array of `Variant`. \

In a `Matcher`,

the `keys` and `value` of each _variant_ are represented as an array of `Variant`.

(Use semantic line breaks; remove stray backslash)

catamorphism · 2024-02-19T19:09:32Z

spec/data-model/README.md

-  value?: string;
+  value: Pattern;


Specifically: I think it should be string because the intention here is for it to be a different string representing the catchall key in some syntax other than MF2. And it should be optional.

catamorphism · 2024-02-19T19:10:40Z

spec/data-model/README.md

-Each element of the `Pattern` MUST either be a non-empty string, an `Expression`, or a `Markup` object.
-String values represent literal _text_.
+Each element of the `Pattern` MUST either be a non-empty string, an `Expression`,
+or a `Markup` object. \


Suggested change

or a `Markup` object. \

or a `Markup`.

Delete "object" for consistency with the rest of the sentence (it doesn't say "an Expression object").

And, I don't think the backslashes are necessary? (Here and elsewhere.)

catamorphism · 2024-02-19T19:17:08Z

spec/data-model/README.md

+`Literal` represents all literal values, both numeric (_unquoted_) and string (_quoted_).
+The presence or absence of quotes (`'|'` in MessageFormat 2) is not preserved by the data model.
+The `value` of `StringLiteral` is the "cooked" value (i.e. escape sequences are processed).
+The `value` of `NumberLiteral` is the numeric value after parsing (for example the trailing zeros are not preserved).


Suggested change

The `value` of `NumberLiteral` is the numeric value after parsing (for example the trailing zeros are not preserved).

The `value` of `NumberLiteral` is the numeric value after parsing,

(for example the trailing zeros are not preserved).

Use semantic line breaks.

More substantively: making this the "parsed" numeric value makes it hard to test the roundtrip property (maybe I missed an issue where this was discussed?)

catamorphism · 2024-02-19T19:17:51Z

spec/data-model/README.md

+and are guaranteed to not be in conflict with future standard development.
+
+An `ReservedAnnotation` represents a _reserved annotation_.
+not supported by the implementations.


Suggested change

not supported by the implementations.

not supported by the implementation.

(Consistency with the previous para)

catamorphism · 2024-02-19T19:17:59Z

spec/data-model/README.md

+
+An `ReservedAnnotation` represents a _reserved annotation_.
+not supported by the implementations.
+These are annotations that might be defined in the future version of the standars.


Suggested change

These are annotations that might be defined in the future version of the standars.

These are annotations that might be defined in the future version of the standard.

catamorphism · 2024-02-19T19:18:49Z

spec/data-model/README.md

+An `Identifier` is used to identify functions, options, attributes, markup,
+and supporting namespaces to avoid conflict between various proprietary extensions. \


Suggested change

An `Identifier` is used to identify functions, options, attributes, markup,

and supporting namespaces to avoid conflict between various proprietary extensions. \

An `Identifier` is used to identify functions, options, attributes, and markup.

It also includes a namespace, to avoid conflict between various proprietary extensions.

aphillips · 2024-02-22T20:26:33Z

Propose we move this post-tech preview.

Review data model

6d8b66b

Addresses issue #639 I will also update the .json and .dtd, but after we get an agreement on this.

mihnita requested review from aphillips and eemeli February 15, 2024 05:16

aphillips reviewed Feb 15, 2024

View reviewed changes

eemeli requested changes Feb 15, 2024

View reviewed changes

aphillips added LDML45 data model Issues related to the Interchange Data Model labels Feb 15, 2024

catamorphism requested changes Feb 19, 2024

View reviewed changes

aphillips added Agenda+ Requested for upcoming teleconference and removed LDML45 labels Feb 22, 2024

aphillips closed this Feb 26, 2024

		A `SimpleMessage` corresponds to a syntax message that includes _selectors_.
		A message without _selectors_ and with a single _pattern_ is represented by a `ComplexMessage`.

		An `UnsupportedStatement` represents a statement not supported by the implementation.
		An `ReservedStatement` represents a statement not supported by the implementation.

	In a `Matcher` the `keys` and `value` of each _variant_ are represented as an array of `Variant`. \
	In a `Matcher`,
	the `keys` and `value` of each _variant_ are represented as an array of `Variant`.

	The `value` of `NumberLiteral` is the numeric value after parsing (for example the trailing zeros are not preserved).
	The `value` of `NumberLiteral` is the numeric value after parsing,
	(for example the trailing zeros are not preserved).

	not supported by the implementations.
	not supported by the implementation.

	These are annotations that might be defined in the future version of the standars.
	These are annotations that might be defined in the future version of the standard.

		An `Identifier` is used to identify functions, options, attributes, markup,
		and supporting namespaces to avoid conflict between various proprietary extensions. \

Uh oh!

Review data model #651

Review data model #651

Uh oh!

Conversation

mihnita commented Feb 15, 2024

Uh oh!

mihnita commented Feb 15, 2024

Uh oh!

aphillips left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

eemeli left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

aphillips commented Feb 22, 2024

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants