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

Commit

Permalink
codemirror file view: Use new /scip highlighting endpoint (#40166)
Browse files Browse the repository at this point in the history
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.
  • Loading branch information
fkling authored Aug 11, 2022
1 parent 6e0c4db commit c790f04
Show file tree
Hide file tree
Showing 9 changed files with 127 additions and 43 deletions.
4 changes: 4 additions & 0 deletions client/web/src/lsif/html.ts
Original file line number Diff line number Diff line change
Expand Up @@ -70,6 +70,10 @@ function highlightSlice(html: HtmlBuilder, kind: SyntaxKind | undefined, slice:

// Currently assumes that no ranges overlap in the occurrences.
export function render(lsif_json: string, content: string): string {
if (!lsif_json.trim()) {
return ''
}

const occurrences = (JSON.parse(lsif_json) as JsonDocument).occurrences?.map(occ => new Occurrence(occ))
if (!occurrences) {
return ''
Expand Down
13 changes: 8 additions & 5 deletions client/web/src/repo/blob/BlobPage.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,7 @@ import { ErrorLike, isErrorLike, asError } from '@sourcegraph/common'
import { SearchContextProps } from '@sourcegraph/search'
import { StreamingSearchResultsListProps } from '@sourcegraph/search-ui'
import { ExtensionsControllerProps } from '@sourcegraph/shared/src/extensions/controller'
import { Scalars } from '@sourcegraph/shared/src/graphql-operations'
import { HighlightResponseFormat, Scalars } from '@sourcegraph/shared/src/graphql-operations'
import { PlatformContextProps } from '@sourcegraph/shared/src/platform/context'
import { SettingsCascadeProps } from '@sourcegraph/shared/src/settings/settings'
import { TelemetryProps } from '@sourcegraph/shared/src/telemetry/telemetryService'
Expand Down Expand Up @@ -158,15 +158,15 @@ export const BlobPage: React.FunctionComponent<React.PropsWithChildren<Props>> =
return of(undefined)
}

return fetchBlob({ repoName, commitID, filePath, formatOnly: true }).pipe(
return fetchBlob({ repoName, commitID, filePath, format: HighlightResponseFormat.HTML_PLAINTEXT }).pipe(
map(blob => {
if (blob === null) {
return blob
}

const blobInfo: BlobPageInfo = {
content: blob.content,
html: blob.highlight.html,
html: blob.highlight.html ?? '',
repoName,
revision,
commitID,
Expand Down Expand Up @@ -202,6 +202,9 @@ export const BlobPage: React.FunctionComponent<React.PropsWithChildren<Props>> =
commitID,
filePath,
disableTimeout,
format: enableCodeMirror
? HighlightResponseFormat.JSON_SCIP
: HighlightResponseFormat.HTML_HIGHLIGHT,
})
),
map(blob => {
Expand All @@ -219,8 +222,8 @@ export const BlobPage: React.FunctionComponent<React.PropsWithChildren<Props>> =

const blobInfo: BlobPageInfo = {
content: blob.content,
html: blob.highlight.html,
lsif: blob.highlight.lsif,
html: blob.highlight.html ?? '',
lsif: blob.highlight.lsif ?? '',
repoName,
revision,
commitID,
Expand Down
34 changes: 21 additions & 13 deletions client/web/src/repo/blob/backend.ts
Original file line number Diff line number Diff line change
Expand Up @@ -6,18 +6,17 @@ import { dataOrThrowErrors, gql } from '@sourcegraph/http-client'
import { ParsedRepoURI, makeRepoURI } from '@sourcegraph/shared/src/util/url'

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

function fetchBlobCacheKey(parsed: ParsedRepoURI & { disableTimeout?: boolean; formatOnly?: boolean }): string {
return `${makeRepoURI(parsed)}?disableTimeout=${parsed.disableTimeout}&formatOnly=${parsed.formatOnly}`
function fetchBlobCacheKey(parsed: ParsedRepoURI & { disableTimeout?: boolean; format?: string }): string {
return `${makeRepoURI(parsed)}?disableTimeout=${parsed.disableTimeout}&=${parsed.format}`
}

interface FetchBlobArguments {
repoName: string
commitID: string
filePath: string
disableTimeout?: boolean
formatOnly?: boolean
format?: HighlightResponseFormat
}

export const fetchBlob = memoizeObservable(
Expand All @@ -26,16 +25,24 @@ export const fetchBlob = memoizeObservable(
commitID,
filePath,
disableTimeout = false,
formatOnly = false,
}: FetchBlobArguments): Observable<BlobFileFields | null> =>
requestGraphQL<BlobResult, BlobVariables>(
format = HighlightResponseFormat.HTML_HIGHLIGHT,
}: FetchBlobArguments): Observable<BlobFileFields | null> => {
// We only want to include HTML data if explicitly requested. We always
// include LSIF because this is used for languages that are configured
// to be processed with tree sitter (and is used when explicitly
// requested via JSON_SCIP).
const html =
format === HighlightResponseFormat.HTML_PLAINTEXT || format === HighlightResponseFormat.HTML_HIGHLIGHT

return requestGraphQL<BlobResult, BlobVariables>(
gql`
query Blob(
$repoName: String!
$commitID: String!
$filePath: String!
$disableTimeout: Boolean!
$formatOnly: Boolean!
$format: HighlightResponseFormat!
$html: Boolean!
) {
repository(name: $repoName) {
commit(rev: $commitID) {
Expand All @@ -49,14 +56,14 @@ export const fetchBlob = memoizeObservable(
fragment BlobFileFields on File2 {
content
richHTML
highlight(disableTimeout: $disableTimeout, formatOnly: $formatOnly) {
highlight(disableTimeout: $disableTimeout, format: $format) {
aborted
html
html @include(if: $html)
lsif
}
}
`,
{ repoName, commitID, filePath, disableTimeout, formatOnly }
{ repoName, commitID, filePath, disableTimeout, format, html }
).pipe(
map(dataOrThrowErrors),
map(data => {
Expand All @@ -65,6 +72,7 @@ export const fetchBlob = memoizeObservable(
}
return data.repository.commit.file
})
),
)
},
fetchBlobCacheKey
)
2 changes: 1 addition & 1 deletion client/web/src/repo/tree/HomeTab.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -106,7 +106,7 @@ export const HomeTab: React.FunctionComponent<React.PropsWithChildren<Props>> =

const blobInfo: BlobInfo & { richHTML: string; aborted: boolean } = {
content: blob.content,
html: blob.highlight.html,
html: blob.highlight.html ?? '',
repoName: repo.name,
revision,
commitID,
Expand Down
5 changes: 3 additions & 2 deletions cmd/frontend/graphqlbackend/highlight.go
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@ import (
"github.com/gogo/protobuf/jsonpb"

"github.com/sourcegraph/sourcegraph/cmd/frontend/internal/highlight"
"github.com/sourcegraph/sourcegraph/internal/gosyntect"
"github.com/sourcegraph/sourcegraph/internal/search/result"
)

Expand Down Expand Up @@ -35,7 +36,7 @@ type HighlightArgs struct {
DisableTimeout bool
IsLightTheme *bool
HighlightLongLines bool
FormatOnly bool
Format string
}

type highlightedFileResolver struct {
Expand Down Expand Up @@ -92,7 +93,7 @@ func highlightContent(ctx context.Context, args *HighlightArgs, content, path st
HighlightLongLines: args.HighlightLongLines,
SimulateTimeout: simulateTimeout,
Metadata: metadata,
FormatOnly: args.FormatOnly,
Format: gosyntect.GetResponseFormat(args.Format),
})

result.aborted = aborted
Expand Down
30 changes: 24 additions & 6 deletions cmd/frontend/graphqlbackend/schema.graphql
Original file line number Diff line number Diff line change
Expand Up @@ -4326,6 +4326,24 @@ type GitTree implements TreeEntry {
): Boolean!
}

"""
The format and highlighting to use when requesting highlighting information for a file.
"""
enum HighlightResponseFormat {
"""
HTML formatted file content without syntax highlighting.
"""
HTML_PLAINTEXT
"""
HTML formatted file content with syntax highlighting.
"""
HTML_HIGHLIGHT
"""
SCIP highlighting information as JSON.
"""
JSON_SCIP
}

"""
A file.
In a future version of Sourcegraph, a repository's files may be distinct from a repository's blobs
Expand Down Expand Up @@ -4393,9 +4411,9 @@ interface File2 {
"""
highlightLongLines: Boolean = false
"""
Return un-highlighted blob contents that are only formatted as a table for use in our blob views.
Specifies which format/highlighting technique to use.
"""
formatOnly: Boolean = false
format: HighlightResponseFormat = HTML_HIGHLIGHT
): HighlightedFile!
}

Expand Down Expand Up @@ -4460,9 +4478,9 @@ type VirtualFile implements File2 {
"""
highlightLongLines: Boolean = false
"""
Return un-highlighted blob contents that are only formatted as a table for use in our blob views.
Specifies which format/highlighting technique to use.
"""
formatOnly: Boolean = false
format: HighlightResponseFormat = HTML_HIGHLIGHT
): HighlightedFile!
}

Expand Down Expand Up @@ -4565,9 +4583,9 @@ type GitBlob implements TreeEntry & File2 {
"""
highlightLongLines: Boolean = false
"""
Return un-highlighted blob contents that are only formatted as a table for use in our blob views.
Specifies which format/highlighting technique to use.
"""
formatOnly: Boolean = false
format: HighlightResponseFormat = HTML_HIGHLIGHT
): HighlightedFile!
"""
Submodule metadata if this tree points to a submodule
Expand Down
15 changes: 8 additions & 7 deletions cmd/frontend/internal/highlight/highlight.go
Original file line number Diff line number Diff line change
Expand Up @@ -106,10 +106,8 @@ type Params struct {
// Metadata provides optional metadata about the code we're highlighting.
Metadata Metadata

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

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

if p.FormatOnly {
if p.Format == gosyntect.FormatHTMLPlaintext {
return unhighlightedCode(err, code)
}

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

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

resp, err := client.Highlight(ctx, query, filetypeQuery.Engine == EngineTreeSitter)
resp, err := client.Highlight(ctx, query, p.Format)

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

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

Expand Down
10 changes: 10 additions & 0 deletions cmd/frontend/internal/highlight/language.go
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@ import (

"github.com/sourcegraph/sourcegraph/internal/conf"
"github.com/sourcegraph/sourcegraph/internal/conf/conftypes"
"github.com/sourcegraph/sourcegraph/internal/gosyntect"
)

type EngineType int
Expand All @@ -20,6 +21,15 @@ const (
EngineSyntect
)

// Converts an engine type to the corresponding parameter value for the syntax
// highlighting request. Defaults to "syntec".
func getEngineParameter(engine EngineType) string {
if engine == EngineTreeSitter {
return gosyntect.SyntaxEngineTreesitter
}
return gosyntect.SyntaxEngineSyntect
}

type SyntaxEngineQuery struct {
Engine EngineType
Language string
Expand Down
Loading

0 comments on commit c790f04

Please sign in to comment.