Remove TextMate grammar, add tree-sitter grammar

rixo commented 2 years ago

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 (#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's language-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 community language-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:

We can move forward on the core benefits of this plugin, that is the language-server, unhindered by tricky grammar decisions.
Different approaches to the grammar problem can be matured in community packages.
People can pick the right extension for their own needs and, crucially, we don't come with a risk of forcing people into a bad trade off if they want to benefit from the language server.

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).

jasonlyu123 commented 2 years ago

Wondering if the classic Textmate syntax highlight in language-tools would work. i.e. the original one created by James Birtles. Maybe the reason why the new one doesn't work it's because of the injection. I also have some trouble porting the new syntax highlight to another editor. I eventually decide to fork the classic one. If you're interested in the forked one, you can find it here

rixo commented 2 years ago

Since an adapted version of the cutting edge VSCode TextMate grammar has been added to this plugin, I'll keep working on the Tree-sitter grammar in a dedicated plugin. In effect, due to the current state of the Tree-sitter Atom ecosystem, having the Tree-sitter grammar bundle with the plugin might be breaking to some users / setups. We might reconsider this decision when the Tree-sitter grammar matures, and things settles in the ecosystem.

sveltejs / svelte-atom