Remove TextMate grammar, add tree-sitter grammar #12
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
This PR aims at upgrading Svelte grammar support and fix issues like #8 by adding a tree-sitter based grammar.
However, after writing down the description of the PR and thinking more about it, I now believe this package shouldn't include any grammar at all, and should instead focus only on language server features, because:
we don't have any perfect solution in our reach yet
generally speaking, addressing grammars in Atom right now proves to be too much of a headache and would be a burden for this package
bad decisions or imperfect grammar solutions (including our current best shot with tree-sitter) could break some users' setup, forcing them to chose between better syntax highlighting and language server
I'm leaving my previous text below for the details of this reasoning, and because I think it still contains useful information about the state of Svelte grammar in Atom. (And I can't think of a better place to put this info right now.)
Atom supports 2 kinds of grammars: TextMate, and tree-sitter. tree-sitter is now the officially recommended format, unfortunately it doesn't feel fully mature yet...
In Atom, you've got to chose whether you enable or disable tree-sitter grammars with a global option. If you opt in, then tree-sitter grammars will be used in priority for every language, if both are available; in particular, you can't selectively enable or disable tree-sitter for a given grammar. This is unfortunate because, in my experience, tree-sitter grammars seems to be less well polished and/or maintained than their TextMate equivalent. Notably, I've found that (officially supported by Atom org) JS and TS grammars are not as well integrated with other Atom packages, and not so comfortable in my own usage...
TextMate
The current TextMate grammar in this plugin is broken (Syntax highlighting broken for Svelte 3 #8), and the motivation to fix and maintain it is very low, since a very good TextMate grammar for VSCode already exists in the language tool repo : https://github.com/sveltejs/language-tools/blob/master/packages/svelte-vscode/syntaxes/svelte.tmLanguage.src.yaml.
Unfortunately, I wasn't able to port the VSCode grammar into Atom. I think the cause is lacking support of newest regex features in current Atom. Hopefully, one day Atom will close the gap, and we'll be able to ship the VSCode grammar for Atom.
Community grammar
A community supported grammar for Svelte does exists : https://atom.io/packages/language-svelte
As far as I can tell, it feels at least as good as the one currently provided by our plugin, but it also somewhat outdated (last updated on oct. 2020).
tree-sitter
A tree-sitter parser for Svelte does exist, and it feels pretty solid. Once the parser exists, it is very straightforward to build a syntax highlighting grammar upon it, since there's no regex magic involved. I have been using the grammar in PR #12 for some time, and the Svelte grammar part, if not perfect (due in part to some current limitations of the parser), has been very satisfying.
However, I have been facing a number of pain points in my own use of tree-sitter in Atom (with other grammars) that eventually made me revert to TextMate grammars for now, so I must conclude this alone won't be a definitive solution for every Svelte user.
Blockers
Atom currently doesn't support the last version of tree-sitter, that the parser uses
References:
I have contacted the authors of the parser about the problem. They're understandably not very interested in downgrading the lib in their package or publishing an Atom specific version.
Suggested solution: Fork and publish our own Atom specific version of the parser for the specific use of this plugin / grammar.
Atom's language-typescript doesn't has an injectionRegex
In order to be able to defer to another grammar, for embedded languages, the target grammar needs to have an
injectionRegex
, that the official Atom'slanguage-typescript
is currently lacking. The PR that would fix it has not been addressed since January 2020, so I'm not hopeful we can it fixed upstream in the immediate future.In the current version of this PR, I have tackled the problem by adding a custom TS grammar for exclusive use by this plugin. However, I'm not convinced this is the wisest thing to do... In particular, because this might cause discrepancy between the grammar the user is using for
.ts
(in particular if they're not using the official one that I have copied over), and what they get in.svelte
files. As a consequence, I believe it would be better to not launch in such a bold move for now, and remove this part from the PR.Conclusions
My initial conclusion leaned toward removing the currently outdated TextMate grammar, and shipping only a tree-sitter grammar with this package on the basis that users that don't want to use tree-sitter grammars can disable it in Atom, and use the community TextMate grammar instead.
Unfortunately, as long as the
injectionRegex
problem in the TS grammar is not fixed (and we haven't addressed it somehow), this would completely break syntax highlighting of<script lang="ts">
for users of tree-sitter grammars, even if they install the communitylanguage-svelte
extension... As alluded earlier, switching grammar format in Atom has significant effects, and so I don't believe this is something we can ask from users, just to fix a problem we newly introduced.Given all the issues that still need to be resolved, either by us or in other projects, to offer a proper Svelte grammar solution that don't come with a risk of being completely breaking to some users' setups, and given the somewhat tricky state of Atom's current grammar ecosystem, I now believe it is better to remove any grammar from
ide-svelte
(this plugin), and defer the responsibility down to other Atom packages.This way:
I am personally interested in pushing with the tree-sitter grammar (in a dedicated plugin), because it does seem the most promising for Atom currently (with some adjustments to other grammars...).
Eventually, when Atom catches up with its JS and regex engine, I am hopeful we can include the TextMate grammar of the VSCode plugin (or as its own package).