Skip to content
This repository was archived by the owner on Sep 30, 2024. It is now read-only.

codemirror file view: Use new /scip highlighting endpoint #40166

Merged
merged 3 commits into from
Aug 11, 2022

Conversation

fkling
Copy link
Contributor

@fkling fkling commented Aug 9, 2022

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 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.
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.

Demo:

sg-cm-syntax-highlight.mp4

Test plan

  • With CodeMirror file view disabled, inspect search results and file views. Highlighting should work as before.
  • With CodeMirror file view enabled, inspect search results and file views. Both should be highlighted.

GraphQL response inspection (can be done via devtools and filtering for graphql?Blob requests:

@fkling fkling added team/code-navigation codemirror PRs and issues related to migrating to CodeMirror labels Aug 9, 2022
@fkling fkling requested review from tjdevries and umpox August 9, 2022 19:14
@cla-bot cla-bot bot added the cla-signed label 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

Copy link
Contributor Author

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.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

yeah, it is unused.

Base automatically changed from tjdevries/syntax-scip-rust-only to main August 10, 2022 16:33
Copy link
Contributor

@tjdevries tjdevries left a 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)

fkling added 3 commits August 11, 2022 13:03
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.
@fkling fkling force-pushed the fkling/scip-graphql branch from 4bc0986 to 11e55b2 Compare August 11, 2022 11:27
@fkling fkling merged commit c790f04 into main Aug 11, 2022
@fkling fkling deleted the fkling/scip-graphql branch August 11, 2022 11:50
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
cla-signed codemirror PRs and issues related to migrating to CodeMirror team/code-navigation
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants