Skip to content
This repository was archived by the owner on Dec 15, 2022. It is now read-only.

Conversation

@caleb531
Copy link

@caleb531 caleb531 commented Jun 19, 2019

Requirements

  • Filling out the template is required. Any pull request that does not include enough information to be reviewed in a timely manner may be closed at the maintainers' discretion.
  • All new code requires tests to ensure against regressions

Description of the Change

This PR adds simple highlighting of invalid/illegal markup in tree-sitter-parsed JSON files. This is similar to how the first-mate JSON grammar highlighted syntax errors in JSON files.

Current first-mate:
json-invalid-current-first-mate

Current tree-sitter:
json-invalid-current

Proposed tree-sitter:
json-invalid-new

Alternate Designs

The first-mate grammar tokenizes different kinds of invalid syntax (e.g. missing comma vs. unterminated strings). I chose not to go this deep with my changes for the tree-sitter grammar here, since 'ERROR': 'invalid.illegal' covers the majority of cases pretty clearly.

I could've also used a scope other than invalid.illegal, though since I am not distinguishing between different types of errors per the above, I felt no need to qualify the scope any further.

Benefits

It would be much more obvious when your JSON file contains invalid/illegal syntax, such as a missing comma or extraneous text.

Possible Drawbacks

Some people may find it distracting or difficult to read, especially if you are writing a JSON file by hand. This may be fixable with some modifications to my new grammar rules (either to scope the selectors more specifically, or to use regex). But the principle remains the same: since JSON and its ecosystem of parsers demand such strict syntax, I think the tradeoff is worth it to more easily catch syntax errors on the spot.

Screen Shot 2019-06-18 at 6 35 52 PM

Applicable Issues

N/A

@Aerijo
Copy link
Contributor

Aerijo commented Jun 19, 2019

Personally, I think this is better left to a linter. The instant feedback arguments apply just as well, and a linter is going to be more configurable than a grammar package.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants