From 7e6919010f9e59590d5000b816b5cefc19df7fe7 Mon Sep 17 00:00:00 2001 From: itning Date: Thu, 29 Oct 2026 00:36:34 +0800 Subject: [PATCH] fix(editor): Draw semantic highlighting above the syntax highlighter (#154) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Semantic token decorations were registered at Prec.high, the same precedence as CodeMirror's tree highlighter, so the syntax span ended up innermost and its colour won over the semantic one (fields, constants and other LSP-coloured identifiers looked like plain text). They now register at Prec.highest. The ⌘-hover link colour is kept over semantic colours, and semantic modifier lookup ignores Object.prototype names, which turned jdtls' `constructor` modifier into a bogus class. Claude-Session: https://claude.ai/code/session_01Dq6jQQn8ZhRfkkyj3PRutA Co-authored-by: itning <19759891+itning@users.noreply.github.com> --- src/editor/lsp/features.test.ts | 14 ++++++++ src/editor/lsp/semantic.test.ts | 63 +++++++++++++++++++++++++++++++++ src/editor/lsp/semantic.ts | 17 ++++++--- src/editor/lsp/theme.ts | 3 +- 4 files changed, 91 insertions(+), 6 deletions(-) create mode 100644 src/editor/lsp/semantic.test.ts diff --git a/src/editor/lsp/features.test.ts b/src/editor/lsp/features.test.ts index 4f754239..357d35ec 100644 --- a/src/editor/lsp/features.test.ts +++ b/src/editor/lsp/features.test.ts @@ -34,6 +34,20 @@ describe("semanticRanges", () => { ); expect(semanticRanges(doc, input, false, "java")[0]?.className).toBe("cm-sem-property cm-sem-static"); }); + + it("ignores type and modifier names that only exist on Object.prototype", () => { + const input = { + tokenTypes: ["method", "toString"], + tokenModifiers: ["valueOf"], + data: [0, 0, 3, 0, 1, 0, 4, 3, 1, 1], + }; + expect(semanticRanges(doc, input)).toEqual([{ from: 0, to: 3, className: "cm-sem-function" }]); + }); + + it("colours a constructor (jdtls: class + `constructor`) as a function", () => { + const input = { tokenTypes: ["class"], tokenModifiers: ["constructor", "declaration"], data: [0, 0, 3, 0, 0b11] }; + expect(semanticRanges(doc, input)[0]?.className).toBe("cm-sem-function cm-sem-declaration"); + }); }); describe("semantic highlighting (rainbow identifiers)", () => { diff --git a/src/editor/lsp/semantic.test.ts b/src/editor/lsp/semantic.test.ts new file mode 100644 index 00000000..8f2f89c6 --- /dev/null +++ b/src/editor/lsp/semantic.test.ts @@ -0,0 +1,63 @@ +import { afterEach, describe, expect, it, vi } from "vitest"; +import { EditorState, Prec, type Extension } from "@codemirror/state"; +import { Decoration, EditorView } from "@codemirror/view"; + +// One `property` token over the whole word, as jdtls reports an instance field. +const lspSemanticTokens = vi.fn(async () => ({ + status: "ok" as const, + data: { data: [0, 0, 13, 0, 0], tokenTypes: ["property"], tokenModifiers: [] }, +})); + +vi.mock("@/platform/ipc", async (orig) => { + const actual = await orig(); + return { ...actual, isTauri: () => true, commands: { ...actual.commands, lspSemanticTokens } }; +}); + +const { semanticExtension } = await import("./semantic"); +const { lspDoc } = await import("./lsp"); + +const views: EditorView[] = []; +afterEach(() => { + views.splice(0).forEach((v) => v.destroy()); + vi.clearAllMocks(); + vi.useRealTimers(); +}); + +/** A syntax-highlighter mark at the same precedence CodeMirror's `treeHighlighter` uses. */ +const syntaxMark = (cls: string, from = 0, to = 13): Extension => + Prec.high(EditorView.decorations.of(Decoration.set([Decoration.mark({ class: cls }).range(from, to)]))); + +async function editor(extensions: Extension[]) { + const view = new EditorView({ + state: EditorState.create({ + doc: "nasProperties", + extensions: [ + lspDoc.of({ doc: 1, path: "/p/A.java", language: "java", uiMode: "Java", readOnly: false }), + ...extensions, + ], + }), + }); + document.body.append(view.dom); + views.push(view); + await vi.advanceTimersByTimeAsync(1000); + return view; +} + +describe("semantic decorations vs the syntax highlighter", () => { + it("puts the semantic mark inside the syntax span, whichever is added last", async () => { + vi.useFakeTimers(); + for (const extensions of [ + [semanticExtension(), syntaxMark("tok-variableName")], + [syntaxMark("tok-variableName"), semanticExtension()], + ]) { + const view = await editor(extensions); + const sem = view.contentDOM.querySelector(".cm-sem-property"); + expect(sem).not.toBeNull(); + // Inner nodes win their own colour: the field must not be wrapped by a + // syntax span that re-sets `color`. + expect(sem!.children.length).toBe(0); + expect(lspSemanticTokens).toHaveBeenCalled(); + view.destroy(); + } + }); +}); diff --git a/src/editor/lsp/semantic.ts b/src/editor/lsp/semantic.ts index 9d09656e..2e4451b3 100644 --- a/src/editor/lsp/semantic.ts +++ b/src/editor/lsp/semantic.ts @@ -96,12 +96,15 @@ export function semanticRanges(doc: Text, tokens: SemanticTokens, rainbow = fals const names: string[] = []; for (let b = 0; b < tokenModifiers.length; b++) if (mods & (1 << b)) names.push(tokenModifiers[b]!); const classes: string[] = []; - let base = TYPE_CLASS[type]; + let base = Object.hasOwn(TYPE_CLASS, type) ? TYPE_CLASS[type] : undefined; + // jdtls marks constructors as a class with a `constructor` modifier; IDEA colours them as methods. + if (names.includes("constructor")) base = "function"; const constant = names.includes("constant") || (type === "variable" && names.includes("readonly") && names.includes("static")); if (constant && (type === "variable" || type === "property")) base = "constant"; if (base && base !== "variable") classes.push(`cm-sem-${base}`); - for (const n of names) if (MODIFIER_CLASS[n]) classes.push(`cm-sem-${MODIFIER_CLASS[n]}`); + // `hasOwn`: servers send names like `constructor` (jdtls) that `Object.prototype` also has. + for (const n of names) if (Object.hasOwn(MODIFIER_CLASS, n)) classes.push(`cm-sem-${MODIFIER_CLASS[n]}`); if (language === "python" && names.includes("static")) classes.push("cm-sem-python-static"); const local = rainbow && @@ -197,7 +200,11 @@ const semanticPlugin = ViewPlugin.fromClass( export function semanticExtension(): Extension { // Higher-precedence decoration sources create the *inner* DOM nodes, so this - // must outrank Lezer's highlighter (default precedence) for semantic colours - // to win; they only set colour / style, never backgrounds. - return [semanticField, semanticPlugin, Prec.high(EditorView.decorations.from(semanticField))]; + // must outrank Lezer's highlighter for semantic colours to win; they only set + // colour / style, never backgrounds. `Prec.highest`, not `Prec.high`: the + // highlighter (`treeHighlighter` in @codemirror/language) is itself + // `Prec.high`, and at equal precedence the later source ends up outside — + // its inner span then re-set the colour (a field's `variableName` grey hid + // `cm-sem-property`). + return [semanticField, semanticPlugin, Prec.highest(EditorView.decorations.from(semanticField))]; } diff --git a/src/editor/lsp/theme.ts b/src/editor/lsp/theme.ts index 98b64de1..51b019d5 100644 --- a/src/editor/lsp/theme.ts +++ b/src/editor/lsp/theme.ts @@ -274,7 +274,6 @@ export const lspTheme = EditorView.theme({ textUnderlineOffset: "2px", cursor: "pointer", }, - ".cm-lsp-link *": { color: "var(--link)" }, ".cm-lsp-occurrence": { backgroundColor: "var(--bg-identifier-under-caret, color-mix(in srgb, var(--accent) 14%, transparent))", }, @@ -383,6 +382,8 @@ export const lspTheme = EditorView.theme({ }, ]), ), + // After the semantic classes: the link wraps their (innermost) spans, and its colour must win. + ".cm-lsp-link *": { color: "var(--link)" }, ...markdown, });