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

wit feature request: add explicit item delimiters #142

Closed
morigs opened this issue Dec 14, 2022 · 7 comments · Fixed by #249
Closed

wit feature request: add explicit item delimiters #142

morigs opened this issue Dec 14, 2022 · 7 comments · Fixed by #249

Comments

@morigs
Copy link

morigs commented Dec 14, 2022

I'd like to suggest adding explicit delimiters between interface/world items. For example, this could be semicolons:

interface example {
  a: func();
  b: func() -> ();
  c: func() -> t;
  type t = u32;
}

Reasoning:
I was experimenting with textmate grammars for WIT (tmLanguage is used by vscode) and discovered that it's difficult to add full-featured syntax highlighting because of tm limitations. In order to add "scope" to some part of the document, tm has to know exactly where it starts and ends. This works for interfaces (they start with { and end with }), however it causes problems with interface children. For example, functions can span multiple lines (so we can't use \n as end), they can have return type missing or specified without parenthesis (so we can't use ')').
Proto format uses semicolons for this.

There are workarounds: for instance it's possible to ignore meta and other conventional scopes and highlight keywords only instead. However, this will make writing wit files uncomfortable :(

VS Code supports "semantic tokens" which provides better highlighting for lsp-enabled languages. However, it's difficult to say if this is enough without actually implementing both tm and lsp

@alexcrichton
Copy link
Collaborator

This seems reasonable to me to add. I think the original inspiration for lack of semicolons was TypeScript buat making it easier on tools seems ok to me at least. That being said I don't have a ton of strong opinions about specific syntax and others may have more thoughts here too.

@lukewagner
Copy link
Member

I've also been quietly worried that our lack of end-of-statement-delimiter may one day come back to bite us in terms of grammatical ambiguities. See also JS Automatic Semicolon Insertion which is a real nightmare.

alexcrichton added a commit to alexcrichton/component-model that referenced this issue Sep 19, 2023
This commit commit is an implementation of WebAssembly#142 where semicolons are now
required as delimiters between items in the WIT text format. All items
in the WIT format are now delimited with either curly braces (`{}`) or
semicolons except for the `package` statement where it subjectively felt
a bit weird to require a semicolon. I've updated the various examples in
`WIT.md` as an example of the new syntax.

My plan on implementing this would be along the lines of:

* Implement the semicolon syntax in `wit-parser`
* Add a parser mode which requires semicolons. This means that the same
  `wit-parser` crate can either or either not require semicolons.
* Update all tests in the `wasm-tools` repository to require semicolons.
* Publish `wit-parser` and `wasm-tools`, integrating the
  semicolon-supporting-mode into all existing tools.
* Wait for Wasmtime to get published with this support. At this point
  everything in the ecosystem should have a point where semicolons are
  optionally supported.
* Remove the parser mode which doesn't require semicolons, meaning
  semicolons are now required.
* Push this update through the tooling, fixing any issues that arise.

The hope is to create a period of time where both syntax forms are
accepted. This provides a transitionary means from one syntax to the
other while proposals are updated. This transitionary period is finite
in length, however.

Closes WebAssembly#142
@alexcrichton
Copy link
Collaborator

I've posted #249 as an example of implementing this along with a rollout plan which hopefully won't cause too much disruption from this.

alexcrichton added a commit to alexcrichton/wasm-tools that referenced this issue Sep 20, 2023
This commit is an implementation of WebAssembly/component-model#142 to
add semicolon delimiters to the WIT text format. The intention of this
change is to be rolled out gradually to minimize ecosystem disruption
(although some is inevitable). The strategy implemented in this commit
is to add support to parse semicolons but optionally do so. Requiring
semicolons is configured via:

* A new `WIT_REQUIRE_SEMICOLONS` environment variable is now read. If
  set to `1` then semicolons are required during parsing.
* A new `SourceMap::set_require_semicolons` is provided to
  programmatically indicate what to do.

When semicolons are not required they are still parsed, but a failure is
not generated if they're not present. This should allow semicolon-using
WIT to coexist with non-semicolon-using WIT for a transitionary period.
After a release or two the default for requiring semicolons will
switch to `true` from the current `false`. That means that
`WIT_REQUIRE_SEMICOLONS=0` can be used to keep old WITs compiling. The
after a few releases of that support for optionally parsing semicolons
will be removed and they'll be unconditionally required.
lukewagner pushed a commit that referenced this issue Sep 25, 2023
This commit commit is an implementation of #142 where semicolons are now
required as delimiters between items in the WIT text format. All items
in the WIT format are now delimited with either curly braces (`{}`) or
semicolons except for the `package` statement where it subjectively felt
a bit weird to require a semicolon. I've updated the various examples in
`WIT.md` as an example of the new syntax.

My plan on implementing this would be along the lines of:

* Implement the semicolon syntax in `wit-parser`
* Add a parser mode which requires semicolons. This means that the same
  `wit-parser` crate can either or either not require semicolons.
* Update all tests in the `wasm-tools` repository to require semicolons.
* Publish `wit-parser` and `wasm-tools`, integrating the
  semicolon-supporting-mode into all existing tools.
* Wait for Wasmtime to get published with this support. At this point
  everything in the ecosystem should have a point where semicolons are
  optionally supported.
* Remove the parser mode which doesn't require semicolons, meaning
  semicolons are now required.
* Push this update through the tooling, fixing any issues that arise.

The hope is to create a period of time where both syntax forms are
accepted. This provides a transitionary means from one syntax to the
other while proposals are updated. This transitionary period is finite
in length, however.

Closes #142
alexcrichton added a commit to bytecodealliance/wasm-tools that referenced this issue Sep 26, 2023
* Add support for semicolon delimiters to `wit-parser`

This commit is an implementation of WebAssembly/component-model#142 to
add semicolon delimiters to the WIT text format. The intention of this
change is to be rolled out gradually to minimize ecosystem disruption
(although some is inevitable). The strategy implemented in this commit
is to add support to parse semicolons but optionally do so. Requiring
semicolons is configured via:

* A new `WIT_REQUIRE_SEMICOLONS` environment variable is now read. If
  set to `1` then semicolons are required during parsing.
* A new `SourceMap::set_require_semicolons` is provided to
  programmatically indicate what to do.

When semicolons are not required they are still parsed, but a failure is
not generated if they're not present. This should allow semicolon-using
WIT to coexist with non-semicolon-using WIT for a transitionary period.
After a release or two the default for requiring semicolons will
switch to `true` from the current `false`. That means that
`WIT_REQUIRE_SEMICOLONS=0` can be used to keep old WITs compiling. The
after a few releases of that support for optionally parsing semicolons
will be removed and they'll be unconditionally required.

* Update all in-repo tests to use semicolons

Additionally add environment-variable support to WIT printing to
optionally print semicolons, disabled for now.
@bashor
Copy link

bashor commented Oct 5, 2023

Couldn't we avoid introducing a delimiter if every declaration inside the interface started with a keyword?

@bashor
Copy link

bashor commented Oct 5, 2023

Isn't it better to use the same delimiter everywhere (inside interface, record, variant, enum, and flags)?

@alexcrichton
Copy link
Collaborator

From a grammatical perspective I don't think semicolons are required today to avoid ambiguities or anything like that, so in that sense I don't believe that semicolons are required. From a "it's easier to parse" and an "it's more extensible" perspective, however, having delimiters like semicolons I believe makes it easier to do syntax highlighting and quick-and-dirty parsing in addition to providing more flexibility in terms of grammar to add in the future.

For different delimiters, that's a good point! It indeed could be possible to use ; instead of , within the constructs you mention (apart from interface which already uses ;)

@leon-thomm
Copy link

leon-thomm commented Oct 8, 2023

If the component model achieves wide adoption, and interfaces will be read many times, this decrease in readability not technically necessary (and this is a lot of redundancy) feels like a step backwards.

If delimiters in the grammar are likely to be necessary at some point anyway then I'm with you, but I would not think so since clearly enough languages don't need them, some of which have excellent vscode integration.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

5 participants