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.

Sign up for GitHub

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

Jump to bottom

Unify input and local declarations in data model #799

Closed
wants to merge 3 commits into from
Closed

Unify input and local declarations in data model #799

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
ac4d83a
Unify input and local declarations in data model
eemeli May 22, 2024
44f3767
Merge branch 'main' into simpler-data
eemeli Jun 4, 2024
5310b04
Merge branch 'main' into simpler-data
eemeli Oct 7, 2024
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
51 changes: 13 additions & 38 deletions spec/data-model/README.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -78,13 +78,13 @@ type Message = PatternMessage | SelectMessage;

interface PatternMessage {
type: "message";
declarations: Declaration[];
declarations: (Declaration | UnsupportedStatement)[];
catamorphism marked this conversation as resolved.
Show resolved Hide resolved
pattern: Pattern;
}

interface SelectMessage {
type: "select";
declarations: Declaration[];
declarations: (Declaration | UnsupportedStatement)[];
selectors: Expression[];
variants: Variant[];
}
Expand All @@ -94,9 +94,10 @@ Each message _declaration_ is represented by a `Declaration`,
which connects the `name` of a _variable_
with its _expression_ `value`.
The `name` does not include the initial `$` of the _variable_.

The `name` of an `InputDeclaration` MUST be the same
as the `name` in the `VariableRef` of its `VariableExpression` `value`.
If the `value` has a `VariableRef` `arg` with the same `name`
as the `Declaration`,
it represents an _input-declaration_.
Otherwise, it represents a _local-declaration_.
Copy link
Collaborator

Choose a reason for hiding this comment

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

I'm not fond of the blanket value: Expression, which requires this wording here. I'd much prefer if the data model didn't need these additional validity constraints. I think the current data model achieves this to a larger extent.

I wouldn't want anyone to think that it's OK to have .input {:func} or .input {|1|} -- which I think the proposed change does suggest.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

This change is in fact removing a MUST validity constraint on InputDeclaration, in addition to removing a similar implicit constraint on the LocalDeclaration that it must not contain in its value a VariableExpression which uses the same name as used at the top level of the declaration.

Could you explain how this proposed language suggests that something like .input {:func} is ok? As it explicitly requires the expression to have a VariableRef argument with a matching name, isn't that the same set of requirements that are expressed in the syntax?


An `UnsupportedStatement` represents a statement not supported by the implementation.
Its `keyword` is a non-empty string name (i.e. not including the initial `.`).
Expand All @@ -112,16 +113,8 @@ The non-empty `expressions` correspond to the trailing _expressions_ of the _res
> this data model.

```ts
type Declaration = InputDeclaration | LocalDeclaration | UnsupportedStatement;

interface InputDeclaration {
type: "input";
name: string;
value: VariableExpression;
}

interface LocalDeclaration {
type: "local";
interface Declaration {
type: "declaration";
name: string;
value: Expression;
}
Expand Down Expand Up @@ -170,37 +163,19 @@ expressions or markup.
```ts
type Pattern = Array<string | Expression | Markup>;

type Expression =
| LiteralExpression
| VariableExpression
| FunctionExpression
| UnsupportedExpression;
type Expression = OperandExpression | AnnotationExpression;
Copy link
Collaborator

Choose a reason for hiding this comment

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

This is an interesting suggestion worth pursuing in a separate PR.


interface LiteralExpression {
interface OperandExpression {
type: "expression";
arg: Literal;
arg: Literal | VariableRef;
annotation?: FunctionAnnotation | UnsupportedAnnotation;
attributes: Attribute[];
}

interface VariableExpression {
type: "expression";
arg: VariableRef;
annotation?: FunctionAnnotation | UnsupportedAnnotation;
attributes: Attribute[];
}

interface FunctionExpression {
type: "expression";
arg?: never;
annotation: FunctionAnnotation;
attributes: Attribute[];
}

interface UnsupportedExpression {
interface AnnotationExpression {
type: "expression";
arg?: never;
annotation: UnsupportedAnnotation;
annotation: FunctionAnnotation | UnsupportedAnnotation;
attributes: Attribute[];
}

Expand Down
6 changes: 1 addition & 5 deletions spec/data-model/message.dtd
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3,12 +3,8 @@
(pattern | (selectors,variant+))
)>

<!-- In a <declaration type="input">, the <expression> MUST contain a <variable> -->
<!ELEMENT declaration (expression)>
<!ATTLIST declaration
type (input | local) #REQUIRED
name NMTOKEN #REQUIRED
>
<!ATTLIST declaration name NMTOKEN #REQUIRED>

<!ELEMENT unsupportedStatement (expression)+>
<!ATTLIST unsupportedStatement
Expand Down
77 changes: 14 additions & 63 deletions spec/data-model/message.json
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -21,18 +21,17 @@
},
"required": ["type", "name"]
},
"literal-or-variable": {
"oneOf": [{ "$ref": "#/$defs/literal" }, { "$ref": "#/$defs/variable" }]
},

"options": {
"type": "array",
"items": {
"type": "object",
"properties": {
"name": { "type": "string" },
"value": {
"oneOf": [
{ "$ref": "#/$defs/literal" },
{ "$ref": "#/$defs/variable" }
]
}
"value": { "$ref": "#/$defs/literal-or-variable" }
},
"required": ["name", "value"]
}
Expand All @@ -43,12 +42,7 @@
"type": "object",
"properties": {
"name": { "type": "string" },
"value": {
"oneOf": [
{ "$ref": "#/$defs/literal" },
{ "$ref": "#/$defs/variable" }
]
}
"value": { "$ref": "#/$defs/literal-or-variable" }
},
"required": ["name"]
}
Expand Down Expand Up @@ -78,58 +72,25 @@
]
},

"literal-expression": {
"type": "object",
"properties": {
"type": { "const": "expression" },
"arg": { "$ref": "#/$defs/literal" },
"annotation": { "$ref": "#/$defs/annotation" },
"attributes": { "$ref": "#/$defs/attributes" }
},
"required": ["type", "arg"]
},
"variable-expression": {
"expression": {
"type": "object",
"properties": {
"type": { "const": "expression" },
"arg": { "$ref": "#/$defs/variable" },
"arg": { "$ref": "#/$defs/literal-or-variable" },
"annotation": { "$ref": "#/$defs/annotation" },
"attributes": { "$ref": "#/$defs/attributes" }
},
"required": ["type", "arg"]
},
"function-expression": {
"type": "object",
"properties": {
"type": { "const": "expression" },
"annotation": { "$ref": "#/$defs/function-annotation" },
"attributes": { "$ref": "#/$defs/attributes" }
},
"required": ["type", "annotation"]
},
"unsupported-expression": {
"type": "object",
"properties": {
"type": { "const": "expression" },
"annotation": { "$ref": "#/$defs/unsupported-annotation" },
"attributes": { "$ref": "#/$defs/attributes" }
},
"required": ["type", "annotation"]
},
"expression": {
"oneOf": [
{ "$ref": "#/$defs/literal-expression" },
{ "$ref": "#/$defs/variable-expression" },
{ "$ref": "#/$defs/function-expression" },
{ "$ref": "#/$defs/unsupported-expression" }
{ "required": ["type", "arg"] },
{ "required": ["type", "annotation"] }
]
},

"markup": {
"type": "object",
"properties": {
"type": { "const": "markup" },
"kind": { "oneOf": [ "open", "standalone", "close" ] },
"kind": { "oneOf": ["open", "standalone", "close"] },
"name": { "type": "string" },
"options": { "$ref": "#/$defs/options" },
"attributes": { "$ref": "#/$defs/attributes" }
Expand All @@ -148,19 +109,10 @@
}
},

"input-declaration": {
"type": "object",
"properties": {
"type": { "const": "input" },
"name": { "type": "string" },
"value": { "$ref": "#/$defs/variable-expression" }
},
"required": ["type", "name", "value"]
},
"local-declaration": {
"declaration": {
"type": "object",
"properties": {
"type": { "const": "local" },
"type": { "const": "declaration" },
"name": { "type": "string" },
"value": { "$ref": "#/$defs/expression" }
},
Expand All @@ -183,8 +135,7 @@
"type": "array",
"items": {
"oneOf": [
{ "$ref": "#/$defs/input-declaration" },
{ "$ref": "#/$defs/local-declaration" },
{ "$ref": "#/$defs/declaration" },
{ "$ref": "#/$defs/unsupported-statement" }
]
}
Expand Down