Tree-sitter rolling fixes, 1.118 edition #1010
Conversation
…from getting a `constant.other` scope applied. Also ensure correct scoping of JSX tags even when the tag name is a member expression like `Foo.Bar`.
…so that the grammar-choosing logic picks the modern Tree-sitter grammar more consistently.
…to gain support for type annotations on class constants, among other things.
…in anticipation of the new auto-generated API docs. (These are some tweaks that I've applied locally because they make the docs better.)
|
I wonder how to find out what has changed in the binary files? Or are they automatically generated from the rest? I'm just starting to slowly understand the entire codebase, so I don't understand a lot. |
The WASM files are generated from their various Tree-sitter repos. This is partly why we introduced the Hence if you wanted to know what's changed in |
|
@savetheclocktower thanx, got it. |
We made this change in one place, but not in another. The real fix is to make both places use the same copy of `resolveNodeDescriptor`, but that'll have to wait another month.
…so that it doesn’t try to look up a child node that we know won’t exist. Most Ruby comments aren’t foldable because they start and end on the same line — by their very nature! But there is one obscure multi-line syntax that we can support, so this tells Pulsar to fold to the end of the comment itself.
* Ensure private methods like `#foo` are included in lists of symbols. * Scope `void` as a unary operator. * Properly scope rest object properties in function definitions.
|
I never put this PR into draft status, but if I had, I'd be taking it out of draft status right now. It's ready for review. |
| 'common/highlights.scm' | ||
| 'tree-sitter-tsx/highlights.scm' | ||
| 'common/highlights.scm' |
There was a problem hiding this comment.
Is this change meaningful?
Any insights into the purpose of it if so? Thanks.
There was a problem hiding this comment.
Order matters here. The contents of each file is concatenated into one query file, and earlier queries are matched before later queries.
I needed a few JSX-specific rules to act before the ordinary rules in highlights.scm — the “Block off identifiers within complex JSX tag names…” stuff matches some things so that it can prevent them from being matched by a later rule.
In this instance, it didn't otherwise matter which query file was included first. But if it had, I'd have needed to create a third file, put only those rules into it, and configure it to be included first.
There was a problem hiding this comment.
Thanks, that's a pretty useful and practical explanation, with the examples.
| # The TextMate grammar has an elaborate `firstLineMatch` pattern defined with | ||
| # Oniguruma; it catches hashbang syntax and complex vim/emacs modelines along | ||
| # with the typical `<?php` prefix. | ||
| # | ||
| # We can't compete with all that, but we can catch obvious instances of | ||
| # shebangs and `<?php`s. In fact, we must, or else the TextMate grammar will be | ||
| # ranked higher for certain files. | ||
| firstLineRegex: '^\\s*<\\?(?:php|PHP|=|\\s|$)|^\\#!.*(?:\\s|\\/)php\\d?' | ||
|
|
There was a problem hiding this comment.
You'd say users are better served by using the Tree-sitter grammar than the TextMate one in these cases, right? Despite the less-sophisticated first line regex?
TextMate won't have issued with newer Electron if I understand correctly, so we can let TextMate handle it if it'd be better? But if I also recall correctly, Tree-sitter is Nifty ™️ in Ways ™️ so this is probably desirable.
Just checking!
There was a problem hiding this comment.
In general, they're better served by using the (modern) Tree-sitter grammars; that's why we prefer Tree-sitter grammars over TextMate grammars when all else is equal.
The goal of the TextMate PHP grammar's firstLineRegex — I'm nearly certain — is to help it detect things that don't have .php extensions but are nonetheless PHP files. That would allow Pulsar to auto-assign files that it otherwise didn't know what to do with. I don't think the intent was to identify patterns that that grammar would be better at parsing than another PHP grammar, since no such grammar existed until quite recently.
It's good to have the two different PHP grammars’ firstLineRegexes match as much as possible so that they're not competing aginst one another in the grammar assignment process — only against non-PHP grammars. But in this case it's not possible.
(If it really mattered, we could use Oniguruma for this task even when the grammar is a Tree-sitter grammar, but I'd rather not do that if we can avoid it.)
There was a problem hiding this comment.
Makes enough sense to me! Again, appreciate the explanation.
| // * `filePath`: A {String} path to the file. | ||
| // * `contents`: The {String} contents of the file. | ||
| // | ||
| // Returns a {Number}. | ||
| getGrammarScore(grammar, filePath, contents) { | ||
| if (contents == null && fs.isFileSync(filePath)) { |
There was a problem hiding this comment.
Should there be contents === null?
I apologize for writing after the merge, my comment was not sent due to the idiotic functionality of Github, it was in pending status, and I didn’t even know about it
There was a problem hiding this comment.
I suppose it depends on if there is a meaningful difference between "either null or undefined" here (==) vs "strictly null, deliberately excluding undefined" (===).
More fixes for TypeScript/JavaScript, plus fixes in grammar-matching logic for PHP. More to come, no doubt.