search

atom / language-javascript

JavaScript language package for Atom
Other
194 stars 235 forks source link

Regexp #661

Closed wizard04wsu closed 5 years ago

wizard04wsu commented 5 years ago

Description of the Change

This is a replacement of the RegExp portions of the TextMate grammar. This adds more detailed regexes and scopes to allow for syntax highlighting within regular expressions, making them easier to write and interpret. It also updates the grammar to support the Unicode (u) flag and its rule variants, including the addition of Unicode code point escapes and Unicode property escapes.

Tests using the One Dark theme.zip

Benefits

Regular expressions can have detailed syntax highlighting of their own.

Possible Drawbacks

Syntax highlighting might be slower in files with a large number of regular expressions.

wizard04wsu commented 5 years ago

The AppVeyor build is giving call stack errors. Why?

rsese commented 5 years ago

Thanks so much for your contribution :bow: For any changes we consider, it's best to have a specific open issue that's being addressed, and that issue should be reviewed and acknowledged by the maintainers (basically meaning that we would accept a pull request to resolve a particular issue). Unless I'm missing it, is this pull request addressing a particular open issue?

But additionally, we've been migrating from our old first-mate grammar engine to the new Tree-sitter engine. This will enable a number of new features, more consistent syntax highlighting, and better performance, among other benefits. In order to free up our limited resources, we have decided to stop maintaining the first-mate grammar when there is a built-in Tree-sitter grammar available and it looks like these changes are targeting the first-mate grammar (let me know if I'm misunderstanding).

After chatting with the other maintainers, with these 2 points in mind we can't review this pull request as is - if you think Tree-sitter would benefit from the same changes, are you interested in making a Tree-sitter specific pull request? If so however, we would first need an issue opened to explain in more detail what the proposal is so the maintainers can consider if it's something we would accept and maintain.

wizard04wsu commented 5 years ago

Okay. This isn't addressing an issue; I'll make sure to create an issue first next time. And yes, this was just targeting the first-mate grammar.

I am interested in updating the Tree-sitter grammar (I'm still learning how). That would be at tree-sitter/tree-sitter-javascript, correct? Or should I open an issue here?

Arcanemagus commented 5 years ago

tree-sitter-javascript is the correct place for issues in how the language is parsed, this repository is the correct place for how the parsed tree is converted into scopes within Atom.

Note that RegEx parsing in the Tree-sitter grammar is handled by tree-sitter-regex, with the JavaScript language defining the start/end points and the rest being handled in there.