From 5571be9bd0b264c50bd8c9bbddc12530e0e104fc Mon Sep 17 00:00:00 2001 From: Nathan Ridge Date: Tue, 13 Aug 2019 23:24:11 -0400 Subject: [PATCH] [WIP] Attempted fix for issue #2906 (semantic highlighting styles overruled by TM scopes) The idea for the fix comes from the observation that there are VSCode plugins that implement semantic highlighting using TextEditor.setDecorations(), and do not suffer from this problem. We don't want to use setDecorations() itself because it's not really suited for incremental updates to the highlighting (which the LSP protocol extension that Theia implements allows for). However, setDecorations() is implemented in terms of deltaDecorations() (which is what Theia uses), and changing Theia's use of deltaDecorations() to be more like the implementation of setDecorations() seems to fix this bug. Specifically, instead of getting an inlineClassName directly from the TokenMetadata (which seems to produce the problematic inlineClassName that coflicts with TM), we: * Get the token color from the TokenMetadata * Construct an IDecorationRenderOptions from the token color (as if we were going to call setDecorations()) * Use ICodeEditorService.registerDecorationType() and ICodeEditorService.resolveDecorationOptions() to massage the IDecorationRenderOptions into an IModelDecorationOptions. This appears to cause monaco to allocate a new inlineClassName for the color which doesn't conflict with TM. * Call deltaDecorations() with IModelDecorationOptions obtained in this way. --- .../monaco-semantic-highlighting-service.ts | 78 ++++++++++++++++--- 1 file changed, 68 insertions(+), 10 deletions(-) diff --git a/packages/monaco/src/browser/monaco-semantic-highlighting-service.ts b/packages/monaco/src/browser/monaco-semantic-highlighting-service.ts index db162f29dc13f..ae0534c937ab8 100644 --- a/packages/monaco/src/browser/monaco-semantic-highlighting-service.ts +++ b/packages/monaco/src/browser/monaco-semantic-highlighting-service.ts @@ -22,6 +22,7 @@ import { Disposable, DisposableCollection } from '@theia/core/lib/common/disposa import { EditorDecoration, EditorDecorationOptions } from '@theia/editor/lib/browser/decorations'; import { SemanticHighlightingService, SemanticHighlightingRange, Range } from '@theia/editor/lib/browser/semantic-highlight/semantic-highlighting-service'; import { MonacoEditor } from './monaco-editor'; +import { MonacoEditorService } from './monaco-editor-service'; @injectable() export class MonacoSemanticHighlightingService extends SemanticHighlightingService { @@ -32,9 +33,51 @@ export class MonacoSemanticHighlightingService extends SemanticHighlightingServi @inject(EditorManager) protected readonly editorManager: EditorManager; + @inject(MonacoEditorService) + protected readonly monacoEditorService: MonacoEditorService; + protected readonly decorations = new Map>(); protected readonly toDisposeOnEditorClose = new Map(); + // laguage id -> (scope index -> model decoration) + protected readonly modelDecorations = new Map>(); + protected readonly decorationTypeKeys = new IdGenerator('DecorationType'); + + register(languageId: string, scopes: string[][] | undefined): Disposable { + const result = super.register(languageId, scopes); + if (scopes) { + const map = new Map(); + for (let index = 0; index < scopes.length; index++) { + const modelDecoration = this.toModelDecoration(scopes[index]); + if (modelDecoration) { + map.set(index, modelDecoration); + } + } + this.modelDecorations.set(languageId, map); + } + // TODO(nridge): + // 1. Remove the decoration types. + // 2. Update the decoration types if the theme changes. + return result; + } + + protected toModelDecoration(scopes: string[]): monaco.editor.IModelDecorationOptions | undefined { + // TODO: why for-of? How to pick the right scope? Is it fine to get the first element (with the narrowest scope)? + const key = this.decorationTypeKeys.nextId(); + for (const scope of scopes) { + const tokenTheme = this.tokenTheme(); + const metadata = tokenTheme.match(undefined, scope); + const colorIndex = monaco.modes.TokenMetadata.getForeground(metadata); + const color = tokenTheme.getColorMap()[colorIndex]; + const options: monaco.editor.IDecorationRenderOptions = { + color: color.toString(), + }; + this.monacoEditorService.registerDecorationType(key, options); + return this.monacoEditorService.resolveDecorationOptions(key, false); + } + return undefined; + } + async decorate(languageId: string, uri: URI, ranges: SemanticHighlightingRange[]): Promise { const editor = await this.editor(uri); if (!editor) { @@ -116,22 +159,25 @@ export class MonacoSemanticHighlightingService extends SemanticHighlightingServi protected toDecoration(languageId: string, range: SemanticHighlightingRange): EditorDecoration { const { start, end } = range; - const scopes = range.scope !== undefined ? this.scopesFor(languageId, range.scope) : []; - const options = this.toOptions(scopes); + const options = this.toOptions(languageId, range.scope); return { range: Range.create(start, end), options }; } - protected toOptions(scopes: string[]): EditorDecorationOptions { - // TODO: why for-of? How to pick the right scope? Is it fine to get the first element (with the narrowest scope)? - for (const scope of scopes) { - const metadata = this.tokenTheme().match(undefined, scope); - const inlineClassName = monaco.modes.TokenMetadata.getClassNameFromMetadata(metadata); - return { - inlineClassName - }; + protected toOptions(languageId: string, scope: number | undefined): EditorDecorationOptions { + if (scope) { + const decorationsForLanguage = this.modelDecorations.get(languageId); + if (decorationsForLanguage) { + const decoration = decorationsForLanguage.get(scope); + if (decoration) { + return { + className: decoration.className, + inlineClassName: decoration.inlineClassName + }; + } + } } return {}; } @@ -141,3 +187,15 @@ export class MonacoSemanticHighlightingService extends SemanticHighlightingServi } } + +// TODO(nridge): Can we use this class from @theia/plugin, instead of duplicating it? +class IdGenerator { + private lastId: number; + constructor(private prefix: string) { + this.lastId = 0; + } + + nextId(): string { + return this.prefix + (++this.lastId); + } +}