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

Commit 4ce4241

Browse files
committed
codemirror file view: Use new /scip highlighting endpoint
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.
1 parent bd796dc commit 4ce4241

File tree

8 files changed

+126
-42
lines changed

8 files changed

+126
-42
lines changed

client/web/src/lsif/html.ts

+4
Original file line numberDiff line numberDiff line change
@@ -70,6 +70,10 @@ function highlightSlice(html: HtmlBuilder, kind: SyntaxKind | undefined, slice:
7070

7171
// Currently assumes that no ranges overlap in the occurrences.
7272
export function render(lsif_json: string, content: string): string {
73+
if (!lsif_json.trim()) {
74+
return ''
75+
}
76+
7377
const occurrences = (JSON.parse(lsif_json) as JsonDocument).occurrences?.map(occ => new Occurrence(occ))
7478
if (!occurrences) {
7579
return ''

client/web/src/repo/blob/BlobPage.tsx

+8-5
Original file line numberDiff line numberDiff line change
@@ -13,7 +13,7 @@ import { ErrorLike, isErrorLike, asError } from '@sourcegraph/common'
1313
import { SearchContextProps } from '@sourcegraph/search'
1414
import { StreamingSearchResultsListProps } from '@sourcegraph/search-ui'
1515
import { ExtensionsControllerProps } from '@sourcegraph/shared/src/extensions/controller'
16-
import { Scalars } from '@sourcegraph/shared/src/graphql-operations'
16+
import { HighlightResponseFormat, Scalars } from '@sourcegraph/shared/src/graphql-operations'
1717
import { PlatformContextProps } from '@sourcegraph/shared/src/platform/context'
1818
import { SettingsCascadeProps } from '@sourcegraph/shared/src/settings/settings'
1919
import { TelemetryProps } from '@sourcegraph/shared/src/telemetry/telemetryService'
@@ -158,15 +158,15 @@ export const BlobPage: React.FunctionComponent<React.PropsWithChildren<Props>> =
158158
return of(undefined)
159159
}
160160

161-
return fetchBlob({ repoName, commitID, filePath, formatOnly: true }).pipe(
161+
return fetchBlob({ repoName, commitID, filePath, format: HighlightResponseFormat.HTML_PLAINTEXT }).pipe(
162162
map(blob => {
163163
if (blob === null) {
164164
return blob
165165
}
166166

167167
const blobInfo: BlobPageInfo = {
168168
content: blob.content,
169-
html: blob.highlight.html,
169+
html: blob.highlight.html ?? '',
170170
repoName,
171171
revision,
172172
commitID,
@@ -202,6 +202,9 @@ export const BlobPage: React.FunctionComponent<React.PropsWithChildren<Props>> =
202202
commitID,
203203
filePath,
204204
disableTimeout,
205+
format: enableCodeMirror
206+
? HighlightResponseFormat.JSON_SCIP
207+
: HighlightResponseFormat.HTML_HIGHLIGHT,
205208
})
206209
),
207210
map(blob => {
@@ -219,8 +222,8 @@ export const BlobPage: React.FunctionComponent<React.PropsWithChildren<Props>> =
219222

220223
const blobInfo: BlobPageInfo = {
221224
content: blob.content,
222-
html: blob.highlight.html,
223-
lsif: blob.highlight.lsif,
225+
html: blob.highlight.html ?? '',
226+
lsif: blob.highlight.lsif ?? '',
224227
repoName,
225228
revision,
226229
commitID,

client/web/src/repo/blob/backend.ts

+21-13
Original file line numberDiff line numberDiff line change
@@ -6,18 +6,17 @@ import { dataOrThrowErrors, gql } from '@sourcegraph/http-client'
66
import { ParsedRepoURI, makeRepoURI } from '@sourcegraph/shared/src/util/url'
77

88
import { requestGraphQL } from '../../backend/graphql'
9-
import { BlobFileFields, BlobResult, BlobVariables } from '../../graphql-operations'
9+
import { BlobFileFields, BlobResult, BlobVariables, HighlightResponseFormat } from '../../graphql-operations'
1010

11-
function fetchBlobCacheKey(parsed: ParsedRepoURI & { disableTimeout?: boolean; formatOnly?: boolean }): string {
12-
return `${makeRepoURI(parsed)}?disableTimeout=${parsed.disableTimeout}&formatOnly=${parsed.formatOnly}`
11+
function fetchBlobCacheKey(parsed: ParsedRepoURI & { disableTimeout?: boolean; format?: string }): string {
12+
return `${makeRepoURI(parsed)}?disableTimeout=${parsed.disableTimeout}&=${parsed.format}`
1313
}
14-
1514
interface FetchBlobArguments {
1615
repoName: string
1716
commitID: string
1817
filePath: string
1918
disableTimeout?: boolean
20-
formatOnly?: boolean
19+
format?: HighlightResponseFormat
2120
}
2221

2322
export const fetchBlob = memoizeObservable(
@@ -26,16 +25,24 @@ export const fetchBlob = memoizeObservable(
2625
commitID,
2726
filePath,
2827
disableTimeout = false,
29-
formatOnly = false,
30-
}: FetchBlobArguments): Observable<BlobFileFields | null> =>
31-
requestGraphQL<BlobResult, BlobVariables>(
28+
format = HighlightResponseFormat.HTML_HIGHLIGHT,
29+
}: FetchBlobArguments): Observable<BlobFileFields | null> => {
30+
// We only want to include HTML data if explicitly requested. We always
31+
// include LSIF because this is used for languages that are configured
32+
// to be processed with tree sitter (and is used when explicitly
33+
// requested via JSON_SCIP).
34+
const html =
35+
format === HighlightResponseFormat.HTML_PLAINTEXT || format === HighlightResponseFormat.HTML_HIGHLIGHT
36+
37+
return requestGraphQL<BlobResult, BlobVariables>(
3238
gql`
3339
query Blob(
3440
$repoName: String!
3541
$commitID: String!
3642
$filePath: String!
3743
$disableTimeout: Boolean!
38-
$formatOnly: Boolean!
44+
$format: HighlightResponseFormat!
45+
$html: Boolean!
3946
) {
4047
repository(name: $repoName) {
4148
commit(rev: $commitID) {
@@ -49,14 +56,14 @@ export const fetchBlob = memoizeObservable(
4956
fragment BlobFileFields on File2 {
5057
content
5158
richHTML
52-
highlight(disableTimeout: $disableTimeout, formatOnly: $formatOnly) {
59+
highlight(disableTimeout: $disableTimeout, format: $format) {
5360
aborted
54-
html
61+
html @include(if: $html)
5562
lsif
5663
}
5764
}
5865
`,
59-
{ repoName, commitID, filePath, disableTimeout, formatOnly }
66+
{ repoName, commitID, filePath, disableTimeout, format, html }
6067
).pipe(
6168
map(dataOrThrowErrors),
6269
map(data => {
@@ -65,6 +72,7 @@ export const fetchBlob = memoizeObservable(
6572
}
6673
return data.repository.commit.file
6774
})
68-
),
75+
)
76+
},
6977
fetchBlobCacheKey
7078
)

cmd/frontend/graphqlbackend/highlight.go

+3-2
Original file line numberDiff line numberDiff line change
@@ -7,6 +7,7 @@ import (
77
"github.com/gogo/protobuf/jsonpb"
88

99
"github.com/sourcegraph/sourcegraph/cmd/frontend/internal/highlight"
10+
"github.com/sourcegraph/sourcegraph/internal/gosyntect"
1011
"github.com/sourcegraph/sourcegraph/internal/search/result"
1112
)
1213

@@ -35,7 +36,7 @@ type HighlightArgs struct {
3536
DisableTimeout bool
3637
IsLightTheme *bool
3738
HighlightLongLines bool
38-
FormatOnly bool
39+
Format string
3940
}
4041

4142
type highlightedFileResolver struct {
@@ -92,7 +93,7 @@ func highlightContent(ctx context.Context, args *HighlightArgs, content, path st
9293
HighlightLongLines: args.HighlightLongLines,
9394
SimulateTimeout: simulateTimeout,
9495
Metadata: metadata,
95-
FormatOnly: args.FormatOnly,
96+
Format: gosyntect.GetResponseFormat(args.Format),
9697
})
9798

9899
result.aborted = aborted

cmd/frontend/graphqlbackend/schema.graphql

+24-6
Original file line numberDiff line numberDiff line change
@@ -4326,6 +4326,24 @@ type GitTree implements TreeEntry {
43264326
): Boolean!
43274327
}
43284328

4329+
"""
4330+
The format and highlighting to use when requesting highlighting information for a file.
4331+
"""
4332+
enum HighlightResponseFormat {
4333+
"""
4334+
HTML formatted file content without syntax highlighting.
4335+
"""
4336+
HTML_PLAINTEXT
4337+
"""
4338+
HTML formatted file content with syntax highlighting.
4339+
"""
4340+
HTML_HIGHLIGHT
4341+
"""
4342+
SCIP highlighting information as JSON.
4343+
"""
4344+
JSON_SCIP
4345+
}
4346+
43294347
"""
43304348
A file.
43314349
In a future version of Sourcegraph, a repository's files may be distinct from a repository's blobs
@@ -4393,9 +4411,9 @@ interface File2 {
43934411
"""
43944412
highlightLongLines: Boolean = false
43954413
"""
4396-
Return un-highlighted blob contents that are only formatted as a table for use in our blob views.
4414+
Specifies which format/highlighting technique to use.
43974415
"""
4398-
formatOnly: Boolean = false
4416+
format: HighlightResponseFormat = HTML_HIGHLIGHT
43994417
): HighlightedFile!
44004418
}
44014419

@@ -4460,9 +4478,9 @@ type VirtualFile implements File2 {
44604478
"""
44614479
highlightLongLines: Boolean = false
44624480
"""
4463-
Return un-highlighted blob contents that are only formatted as a table for use in our blob views.
4481+
Specifies which format/highlighting technique to use.
44644482
"""
4465-
formatOnly: Boolean = false
4483+
format: HighlightResponseFormat = HTML_HIGHLIGHT
44664484
): HighlightedFile!
44674485
}
44684486

@@ -4565,9 +4583,9 @@ type GitBlob implements TreeEntry & File2 {
45654583
"""
45664584
highlightLongLines: Boolean = false
45674585
"""
4568-
Return un-highlighted blob contents that are only formatted as a table for use in our blob views.
4586+
Specifies which format/highlighting technique to use.
45694587
"""
4570-
formatOnly: Boolean = false
4588+
format: HighlightResponseFormat = HTML_HIGHLIGHT
45714589
): HighlightedFile!
45724590
"""
45734591
Submodule metadata if this tree points to a submodule

cmd/frontend/internal/highlight/highlight.go

+8-7
Original file line numberDiff line numberDiff line change
@@ -106,10 +106,8 @@ type Params struct {
106106
// Metadata provides optional metadata about the code we're highlighting.
107107
Metadata Metadata
108108

109-
// FormatOnly, if true, will skip highlighting and only return the code
110-
// in a formatted table view. This is useful if we want to display code
111-
// as quickly as possible, without waiting for highlighting.
112-
FormatOnly bool
109+
// Format defines the response format of the syntax highlighting request.
110+
Format gosyntect.HighlightResponseType
113111
}
114112

115113
// Metadata contains metadata about a request to highlight code. It is used to
@@ -405,7 +403,7 @@ func Code(ctx context.Context, p Params) (response *HighlightedCode, aborted boo
405403
return plainResponse, true, nil
406404
}
407405

408-
if p.FormatOnly {
406+
if p.Format == gosyntect.FormatHTMLPlaintext {
409407
return unhighlightedCode(err, code)
410408
}
411409

@@ -431,6 +429,7 @@ func Code(ctx context.Context, p Params) (response *HighlightedCode, aborted boo
431429
Tracer: ot.GetTracer(ctx),
432430
LineLengthLimit: maxLineLength,
433431
CSS: true,
432+
Engine: getEngineParameter(filetypeQuery.Engine),
434433
}
435434

436435
// Set the Filetype part of the command if:
@@ -443,7 +442,7 @@ func Code(ctx context.Context, p Params) (response *HighlightedCode, aborted boo
443442
query.Filetype = filetypeQuery.Language
444443
}
445444

446-
resp, err := client.Highlight(ctx, query, filetypeQuery.Engine == EngineTreeSitter)
445+
resp, err := client.Highlight(ctx, query, p.Format)
447446

448447
if ctx.Err() == context.DeadlineExceeded {
449448
log15.Warn(
@@ -487,7 +486,9 @@ func Code(ctx context.Context, p Params) (response *HighlightedCode, aborted boo
487486
return unhighlightedCode(err, code)
488487
}
489488

490-
if filetypeQuery.Engine == EngineTreeSitter {
489+
// We need to return SCIP data if explicitly requested or if the selected
490+
// engine is tree sitter.
491+
if p.Format == gosyntect.FormatJSONSCIP || filetypeQuery.Engine == EngineTreeSitter {
491492
document := new(scip.Document)
492493
data, err := base64.StdEncoding.DecodeString(resp.Data)
493494

cmd/frontend/internal/highlight/language.go

+10
Original file line numberDiff line numberDiff line change
@@ -10,6 +10,7 @@ import (
1010

1111
"github.com/sourcegraph/sourcegraph/internal/conf"
1212
"github.com/sourcegraph/sourcegraph/internal/conf/conftypes"
13+
"github.com/sourcegraph/sourcegraph/internal/gosyntect"
1314
)
1415

1516
type EngineType int
@@ -20,6 +21,15 @@ const (
2021
EngineSyntect
2122
)
2223

24+
// Converts an engine type to the corresponding parameter value for the syntax
25+
// highlighting request. Defaults to "syntec".
26+
func getEngineParameter(engine EngineType) string {
27+
if engine == EngineTreeSitter {
28+
return gosyntect.SyntaxEngineTreesitter
29+
}
30+
return gosyntect.SyntaxEngineSyntect
31+
}
32+
2333
type SyntaxEngineQuery struct {
2434
Engine EngineType
2535
Language string

0 commit comments

Comments
 (0)