This repository was archived by the owner on Sep 30, 2024. It is now read-only.
Highlighting: add tree-sitter support for the most common programming languages #47571
Conversation
This file contains hidden or 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
olafurpg
changed the title
|
❌ Problem: the label |
olafurpg
changed the title
|
❌ Problem: the label |
|
❌ Problem: the label |
tjdevries
reviewed
Feb 20, 2023
...ghlighter/crates/sg-syntax/src/snapshots/sg_syntax__sg_treesitter__test__.__c_example.c.snap
Show resolved
Hide resolved
tjdevries
reviewed
Feb 20, 2023
tjdevries
reviewed
Feb 20, 2023
docker-images/syntax-highlighter/crates/sg-syntax/queries/tsx/highlights.scm
Show resolved
Hide resolved
|
ok, after those comments I think it looks good to me! |
tjdevries
approved these changes
Feb 20, 2023
olafurpg
force-pushed
the
olafurpg/ts-symbols
branch
from
f6dc291 to
3405724
Compare
|
This PR is ready to merge once |
|
@tjdevries Any idea why CI is failing with this error? |
olafurpg
force-pushed
the
olafurpg/ts-symbols
branch
2 times, most recently
from
8c406ca to
fb414e3
Compare
auto-merge was automatically disabled
February 23, 2023 15:46
Merge could not be authorized
|
The CI is failing with a new error after I added I'm unable to reproduce locally on macOS, the command |
|
hmm, that's pretty weird -- maybe somehow old version of rust in the ci images? I can try and take a look tomorrow |
|
I bumped the rust image to the latest alpine3.14 tag that I could find, which is rust 1.61.0, while our .tool-versions is 1.62.0. Let's see if that helps. |
Previously, we only supported Syntect syntax highlighting for TypeScript and JavaScript. This PR adds tree-sitter support for those languages, which should both have more reliable performance and produce higher quality highlighting. Down the road, we can start emitting symbol data alongside highlighting to unblock other features like rendering stable symbol-based URLs for "Copy link".
olafurpg
force-pushed
the
olafurpg/ts-symbols
branch
from
16188be to
e386293
Compare
|
I'm able to reproduce the CI failures locally with I tried to upgrade to the latest rust image |
|
@olafurpg maybe passing this environment variable while building the C code may be helpful: rust-lang/git2-rs#706 (comment) |
auto-merge was automatically disabled
February 24, 2023 13:07
Merge could not be authorized
olafurpg
force-pushed
the
olafurpg/ts-symbols
branch
from
c460cc3 to
9cbef72
Compare
olafurpg
force-pushed
the
olafurpg/ts-symbols
branch
from
780711c to
cb67af1
Compare
|
Temporarily restoring the branch so I can repro the problem. |
olafurpg
referenced
this pull request
Feb 28, 2023
Highlighthing is timing out for Ruby, C++, and Python, on sourcegraph.com because we're trying to highlight these languages with tree-sitter when syntax-highlighter has no tree-sitter support for those languages. The grammars for these languages depend on C++ code that we weren't able to build in CI so we removed them last minute before merging the PR that added tree-sitter highligting for Java, TypeScript, JavaScript, and Rust. https://github.com/sourcegraph/sourcegraph/pull/47571
olafurpg
referenced
this pull request
Feb 28, 2023
Highlighthing is timing out for Ruby, C++, and Python, on sourcegraph.com because we're trying to highlight these languages with tree-sitter when syntax-highlighter has no tree-sitter support for those languages. The grammars for these languages depend on C++ code that we weren't able to build in CI so we removed them last minute before merging the PR that added tree-sitter highligting for Java, TypeScript, JavaScript, and Rust. https://github.com/sourcegraph/sourcegraph/pull/47571 ## Test plan Manually tested that the blob view no longer times out after these changes. We need to add regression testing for all the main languages to prevent this kind of error from happening again. <!-- All pull requests REQUIRE a test plan: https://docs.sourcegraph.com/dev/background-information/testing_principles -->
olafurpg
referenced
this pull request
Mar 6, 2023
Follow-up from https://github.com/sourcegraph/sourcegraph/pull/47571 now that we can use grammars with C++ dependencies. Example Ruby highlighting  Example Python highlighting  ## Test plan Updated snapshots. <!-- All pull requests REQUIRE a test plan: https://docs.sourcegraph.com/dev/background-information/testing_principles -->
Sign up for free
to subscribe to this conversation on GitHub.
Already have an account?
Sign in.
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.
Previously, we only supported Syntect syntax highlighting for C# and Scala. This PR adds tree-sitter support for more languages (JS, TS, C, C++, Java, Ruby, Rust, and Python), which should both improve performance and produce higher quality highlighting.
Down the road, we can start emitting symbol data alongside highlighting to unblock other features like rendering stable symbol-based URLs for "Copy link".
Follow-up items:
@variablewith@identifier.SyntaxKindto include:IdentifierField,IdentifierMacro. See added TODOs in highlights.scm for a few other cases where our current kinds may not be good enough.IdentifierLocalbased onlocals.scmqueriesIdentifierKeywordintoKeyword.Test plan
See updated snapshot tests.