This repository has been archived by the owner on Sep 30, 2024. It is now read-only.
-
Notifications
You must be signed in to change notification settings - Fork 1.3k
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
75ed2c1
to
17daf8f
Compare
ec0b016
to
a99521b
Compare
Diffs in snapshot files are primarily caused by a large rewrite of the upstream grammar: tree-sitter/tree-sitter-c-sharp#333
c4d9f96
to
ccbc7fe
Compare
The version bump PR was merged
ccbc7fe
to
75559d1
Compare
Sign up for free
to subscribe to this conversation on GitHub.
Already have an account?
Sign in.
Labels
cla-signed
team/graph
Graph Team (previously Code Intel/Language Tools/Language Platform)
team/product-platform
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Common changes
Most of the highlighting queries needed to be adjusted, because in 0.21
tree-sitter-highlight
switched to a "last-match-wins" strategy for dealing with overlapping captures. This mostly means that "catch all" rules like(identifier) @variable
now need to move to the top of the query, but there are a few other cases with overlaps which I flipped around until all the snapshots passed again.In some places the upstream grammar renamed/removed/wrapped nodes, which meant I needed to make small adjustments to the queries. Where appropriate I tried to follow the upstream queries, and otherwise just made sure to keep the snapshots correct.
Language specific changes
JS/TS snapshot diffs are caused by a refactoring of both JSX tag handling and strings. The queries now cause a few more separate overlapping matches for strings which is noisy in the snapshot output but shouldn't change the highlighting as the same ranges are still marked as "string".
I added a file to test jsonnnet highlighting as we didn't have one in the repo yet.
The C# grammar was completely rewritten in tree-sitter/tree-sitter-c-sharp#333, and it produces error nodes on a file we checked in to guard against a syntect loop at some point.
I don't know enough about C# to really have a good grasp on what's the best thing to do here. The highlighting diff looks reasonable for the file we checked in for highlight testing.I discussed this with Matt and we agreed that I'd fork pre-grammar-rewrite and bump its tree-sitter version. I've also gone ahead and reported the errors we found upstream.
The Nickel snapshot changes are caused by updating the highlights.scm file. From what I can tell we were previously using a copy of the upstream queries, and so I also updated our copy. This highlighting ranges are very different, but they seem more detailed and correct at a glance.
The Python snapshot changes are a bug-fix.
The Rust snapshot changes are partially bugfix/partially improved parsing for attribute macros from upstream.
I swapped our SQL grammar as the one we were using wasn't maintained for a while. From what I can tell the new grammar is maintained nicely and well structured.
I pointed at
tree-sitter-pkl
for my personal fork, but didn't upstream a PR because there is a much more comprehensive PR there already. My fork just contains the minimal change to bump the TS version. We could either fork to sourcegraph/tree-sitter-pkl or maybe we know someone to ping to take a look at the upstream PR?Deleted any reference to xlsg, as apparently it was a made up joke language?
Bazel/Build specific
Unfortunately the released 0.22.6 version of tree-sitter (from May) doesn't build under Bazel. We need the fix from tree-sitter/tree-sitter#3352, so I've used a Cargo feature to make sure we're using that patched tree-sitter for our build. I'm not sure why it has been such a long time since TS put out a release but we can get rid of that override with the next release.
TODO:
main
that we no longer need). Maybe we should rebasemain
on the fork to get closer to upstream again @mmanela?Test plan
Updated snapshots, things continue to compile