From 607b3e3494daba4a357e99a02b580b51adb287fe Mon Sep 17 00:00:00 2001 From: Alf Eaton Date: Tue, 3 Sep 2024 11:23:36 +0100 Subject: [PATCH] feat: making highlighting of errors more specific (#19963) GitOrigin-RevId: 63bc147e18e80c1e070722bc70114f8fca8509ae --- .../features/pdf-preview/util/output-files.js | 1 + .../source-editor/extensions/annotations.ts | 68 ++++++++++++++++--- .../hooks/use-codemirror-scope.ts | 2 +- .../human-readable-logs/HumanReadableLogs.js | 4 ++ .../HumanReadableLogsRules.tsx | 4 ++ services/web/types/annotation.ts | 1 + 6 files changed, 68 insertions(+), 12 deletions(-) diff --git a/services/web/frontend/js/features/pdf-preview/util/output-files.js b/services/web/frontend/js/features/pdf-preview/util/output-files.js index 0e261b272f..e273cf037d 100644 --- a/services/web/frontend/js/features/pdf-preview/util/output-files.js +++ b/services/web/frontend/js/features/pdf-preview/util/output-files.js @@ -183,6 +183,7 @@ export function buildLogEntryAnnotations(entries, fileTreeData, rootDocId) { text: entry.message, source: 'compile', // NOTE: this is used in Ace for filtering the annotations ruleId: entry.ruleId, + command: entry.command, } // set firstOnLine for the first non-typesetting annotation on a line diff --git a/services/web/frontend/js/features/source-editor/extensions/annotations.ts b/services/web/frontend/js/features/source-editor/extensions/annotations.ts index 1f8521e9a7..07bed7f1ec 100644 --- a/services/web/frontend/js/features/source-editor/extensions/annotations.ts +++ b/services/web/frontend/js/features/source-editor/extensions/annotations.ts @@ -2,17 +2,19 @@ import { EditorView, ViewUpdate } from '@codemirror/view' import { Diagnostic, linter, lintGutter } from '@codemirror/lint' import { Compartment, + EditorState, Extension, + Line, RangeSet, RangeValue, StateEffect, StateField, - Text, } from '@codemirror/state' import { Annotation } from '../../../../../types/annotation' import { debugConsole } from '@/utils/debugging' import { sendMB } from '@/infrastructure/event-tracking' import importOverleafModules from '../../../../macros/import-overleaf-module.macro' +import { syntaxTree } from '@codemirror/language' interface CompileLogDiagnostic extends Diagnostic { compile?: true @@ -142,14 +144,17 @@ export const compileDiagnosticsState = StateField.define< }, }) -export const setAnnotations = (doc: Text, annotations: Annotation[]) => { +export const setAnnotations = ( + state: EditorState, + annotations: Annotation[] +) => { const diagnostics: CompileLogDiagnostic[] = [] for (const annotation of annotations) { // ignore "whole document" (row: -1) annotations if (annotation.row !== -1) { try { - diagnostics.push(convertAnnotationToDiagnostic(doc, annotation)) + diagnostics.push(...convertAnnotationToDiagnostic(state, annotation)) } catch (error) { // ignore invalid annotations debugConsole.debug('invalid annotation position', error) @@ -171,19 +176,60 @@ export const showCompileLogDiagnostics = (show: boolean) => { } } -const convertAnnotationToDiagnostic = ( - doc: Text, +const commandRanges = (state: EditorState, line: Line, command: string) => { + const ranges: { from: number; to: number }[] = [] + + syntaxTree(state).iterate({ + enter(nodeRef) { + if (nodeRef.type.is('CtrlSeq')) { + const { from, to } = nodeRef + if (command === state.sliceDoc(from, to)) { + ranges.push({ from, to }) + } + } + }, + from: line.from, + to: line.to, + }) + + return ranges.slice(0, 1) // NOTE: only highlighting the first match on a line, to avoid duplicate messages +} + +const chooseHighlightRanges = ( + state: EditorState, + line: Line, annotation: Annotation -): CompileLogDiagnostic => { +) => { + const ranges: { from: number; to: number }[] = [] + + if (annotation.command) { + ranges.push(...commandRanges(state, line, annotation.command)) + } + + // default to highlighting the whole line + if (ranges.length === 0) { + ranges.push(line) + } + + return ranges +} + +const convertAnnotationToDiagnostic = ( + state: EditorState, + annotation: Annotation +): CompileLogDiagnostic[] => { if (annotation.row < 0) { throw new Error(`Invalid annotation row ${annotation.row}`) } - const line = doc.line(annotation.row + 1) + // NOTE: highlight whole line by default, as synctex doesn't output column number + const line = state.doc.line(annotation.row + 1) - return { - from: line.from, - to: line.to, // NOTE: highlight whole line as synctex doesn't output column number + const highlightRanges = chooseHighlightRanges(state, line, annotation) + + return highlightRanges.map(location => ({ + from: location.from, + to: location.to, severity: annotation.type, message: annotation.text, ruleId: annotation.ruleId, @@ -192,7 +238,7 @@ const convertAnnotationToDiagnostic = ( entryIndex: annotation.entryIndex, source: annotation.source, firstOnLine: annotation.firstOnLine, - } + })) } export const renderMessage = (diagnostic: RenderedDiagnostic) => { diff --git a/services/web/frontend/js/features/source-editor/hooks/use-codemirror-scope.ts b/services/web/frontend/js/features/source-editor/hooks/use-codemirror-scope.ts index 2eb082a2c0..a6d54bdc09 100644 --- a/services/web/frontend/js/features/source-editor/hooks/use-codemirror-scope.ts +++ b/services/web/frontend/js/features/source-editor/hooks/use-codemirror-scope.ts @@ -497,7 +497,7 @@ function useCodeMirrorScope(view: EditorView) { // dispatch in a timeout, so the dispatch isn't in the same cycle as the edit which caused it window.setTimeout(() => { view.dispatch( - setAnnotations(view.state.doc, annotations || []), + setAnnotations(view.state, annotations || []), // reconfigure the compile log lint source, so it runs once with the new data showCompileLogDiagnostics(enableCompileLogLinterRef.current) ) diff --git a/services/web/frontend/js/ide/human-readable-logs/HumanReadableLogs.js b/services/web/frontend/js/ide/human-readable-logs/HumanReadableLogs.js index e3ea21ada0..57a35adfdc 100644 --- a/services/web/frontend/js/ide/human-readable-logs/HumanReadableLogs.js +++ b/services/web/frontend/js/ide/human-readable-logs/HumanReadableLogs.js @@ -51,6 +51,10 @@ export default { } } + if (entry.contentDetails && ruleDetails.highlightCommand) { + entry.command = ruleDetails.highlightCommand(entry.contentDetails) + } + // suppress any entries that are known to cascade from previous error types if (ruleDetails.cascadesFrom) { for (const type of ruleDetails.cascadesFrom) { diff --git a/services/web/frontend/js/ide/human-readable-logs/HumanReadableLogsRules.tsx b/services/web/frontend/js/ide/human-readable-logs/HumanReadableLogsRules.tsx index bbfa9af7de..5e51ab8308 100644 --- a/services/web/frontend/js/ide/human-readable-logs/HumanReadableLogsRules.tsx +++ b/services/web/frontend/js/ide/human-readable-logs/HumanReadableLogsRules.tsx @@ -16,6 +16,7 @@ interface Rule { details?: [string] ) => string | [string, JSX.Element] package?: string + highlightCommand?: (contentDetails: string[]) => string | undefined } const rules: Rule[] = [ @@ -78,6 +79,9 @@ const rules: Rule[] = [ } return currentTitle }, + highlightCommand(contentDetails) { + return contentDetails[0] + }, }, { ruleId: 'hint_undefined_environment', diff --git a/services/web/types/annotation.ts b/services/web/types/annotation.ts index 1eacda21c5..e8727476bf 100644 --- a/services/web/types/annotation.ts +++ b/services/web/types/annotation.ts @@ -7,4 +7,5 @@ export type Annotation = { id: string entryIndex: number firstOnLine: boolean + command?: string }