Skip to content
This repository has been archived by the owner on Mar 3, 2023. It is now read-only.

GrammarRegistry.loadGrammar does not catch errors from addGrammar #23041

Open
1 task done
50Wliu opened this issue Sep 28, 2021 · 3 comments
Open
1 task done

GrammarRegistry.loadGrammar does not catch errors from addGrammar #23041

50Wliu opened this issue Sep 28, 2021 · 3 comments

Comments

@50Wliu
Copy link
Contributor

50Wliu commented Sep 28, 2021

Prerequisites

Description

While investigating 50Wliu/language-hclrs#4, I root-caused the issue to using a newer tree-sitter version than Atom supported. However, this was non-obvious, and the only place it was reported was in the console.

When loading grammars, loadGrammar checks to make sure that readGrammar succeeds:

this.readGrammar(grammarPath, (error, grammar) => {
if (error) return callback(error);

However, there are no checks for the subsequent addGrammar call:

this.addGrammar(grammar);
callback(null, grammar);

Even though addGrammar can fail when (eventually) calling TreeSitterLanguageMode.parse, which itself calls parser.setLanguage which can throw.

parser.setLanguage(language);

Resulting in e.g.
Incompatible tree-sitter version

Steps to Reproduce

  1. Install language-hclrs@0.0.2
  2. Set language to HCLRS

Expected behavior:

A helpful error notificaton.

Actual behavior:

Nothing! Why isn't my syntax highlighting working?

Reproduces how often:

100%

Versions

1.58.0, Windows 11 Insider.

Additional Information

N/A

@icecream17
Copy link
Contributor

icecream17 commented Sep 28, 2021

Wow, this information here is more helpful than the information I gave at #22129... going to update that

@50Wliu
Copy link
Contributor Author

50Wliu commented Sep 28, 2021

Whoops, didn't search for the right keywords 😊. Figures that this has already been reported.

I'll keep this one open to track the fact that loadGrammar is unreliable at reporting errors, and #22129 seems better suited to tracking how Atom can support languages that use tree-sitter 0.19.0+ :).

@claytonrcarter
Copy link
Contributor

This also seems to be happening outside of GrammarRegistry.loadGrammar. In particular, if a "working" tree-sitter grammar is loaded but a "not working" grammar is injected into some part of it. Also, because injections can happen in multiple places, this could trigger multiple errors.

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

No branches or pull requests

3 participants