This repository was 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
codemirror file view: Use new /scip highlighting endpoint #40166
Merged
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
21 tasks
fkling
commented
Aug 9, 2022
Comment on lines
-67
to
-69
// LSIF is the base64 encoded byte array of an LSIF Typed payload containing highlighting data. | ||
LSIF string | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This seems to be unused.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
yeah, it is unused.
umpox
approved these changes
Aug 10, 2022
tjdevries
approved these changes
Aug 10, 2022
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM, assuming you rebase off of main from my changes now and then test the different combos of:
(syntect vs treesitter) * (old blob view vs codemirror)
This commit makes minimal and additive changes to the frontend and Go backend to make the new /scip highlighting endpoint available through GraphQL. There are currently three possible scenarios how a file might get highlighted: - HTML blob view, default: Highlighted HTML is generated by syntect - HTML blob view, tree sitter: SCIP data is generated by tree sitter, HTML is generated on the client side - CodeMirror blob view, default: SCIP is generated by either syntect or tree sitter So far SCIP data was only generated for specific languages, determined by the server. With CodeMirror, SCIP also needs to be generated when the client requests it. My preferred solution would have been to let the server determine this based on the requested fields in the GraphQL request, but the library we are using [does not support that yet](graph-gophers/graphql-go#17). Making the highlighting requests in the field resolvers (i.e. `HTML()` and `LSIF()`) is also not possible without additional changes because the `Aborted()` field depends on the result of the request. This led me change the `formatOnly` field from a boolean to an enum, with which the client can now request: - `HTML_PLAINTEXT` - `HTML_HIGHLIGHT` - `JSON_SCIP` It's not ideal because the server can still return SCIP data depending on whether tree sitter is configured for the language (see second bullet point at the beginning) but I think this is still better than introducing a new field for highlighting. So, if CodeMirror is *disabled*, everything works as before. When CodeMirror is *enabled* it will explicitly request `JSON_SCIP` data and only include the `lsif` field in the GraphQL request. The server will route that request to the new `/scip` endpoint.
4bc0986
to
11e55b2
Compare
Sign up for free
to subscribe to this conversation on GitHub.
Already have an account?
Sign in.
Labels
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.
Based on #39850
This commit makes minimal and additive changes to the frontend and Go
backend to make the new /scip highlighting endpoint available through GraphQL.
There are currently three possible scenarios how a file might get
highlighted:
HTML is generated on the client side
tree sitter
So far SCIP data was only generated for specific languages, determined
by the server. With CodeMirror, SCIP also needs to be generated when the
client requests it.
My preferred solution would have been to let the server determine this
based on the requested fields in the GraphQL request, but the library we
are using does not support that yet.
Making the highlighting requests in the field resolvers (i.e.
HTML()
and
LSIF()
) is also not possible without additional changes becausethe
Aborted()
field depends on the result of the request.This led me change the
formatOnly
field from a boolean to an enum,with which the client can now request:
HTML_PLAINTEXT
HTML_HIGHLIGHT
JSON_SCIP
It's not ideal because the server can still return SCIP data depending
on whether tree sitter is configured for the language (see second bullet
point at the beginning) but I think this is still better than
introducing a new field for highlighting.
So, if CodeMirror is disabled, everything works as before. When
CodeMirror is enabled it will explicitly request
JSON_SCIP
data andonly include the
lsif
field in the GraphQL request. The server willroute that request to the new
/scip
endpoint.Demo:
sg-cm-syntax-highlight.mp4
Test plan
GraphQL response inspection (can be done via devtools and filtering for
graphql?Blob
requests:lsif
field and nohtml
field.lsif
field and nohtml
field.lsif
field and a non-emptyhtml
field.lsif
field and a non-emptyhtml
field.