[clangd] Client-side support for inactive regions

[nridge]

This is not for review yet, I'm just posting it to illustrate the client side of D67536.

[nridge]

Update to reflect changes to server side in D67536

[nridge]

Updated to reflect changes to server side in D67536

[nridge]

Clean up patch a bit and update tests as well

[nridge]

clang-tools-extra/clangd/clients/clangd-vscode/src/semantic-highlighting.ts

220

I'm open to suggestions for how we could avoid harcoding these colors. VSCode themes do not seem to include colors for "meta.disabled" or any similar scope that would fit this role.

Perhaps we can allow the user to define the colors in clangd-specifier setting names, e.g. clangd.inactiveHighlightColor.light?

[nridge]

Review ping :)

[nridge]

Review ping again :)

[hokein]

sorry for the looong delay, I was OOO for a few weeks before.

The patch looks fine, my concern is that how do we support this line extension when LSP provides a standard implementation -- there is some significant progress about semantic highlighting on VSCode, VSCode now provides an experimental semantic token API, but no LSP binding yet (is not far away I think). When it is ready, we'd like to switch to that and get rid of our own implementation. Looking at the API there, it seems like we could make the line-style scope name meta.disable clangd specific, and define our own style.

clang-tools-extra/clangd/clients/clangd-vscode/src/semantic-highlighting.ts

288

another solution that requires minimal changes (but more hacky) is that, since clangd emits a meta.disabled token with line-set-only range, we could inject the inactiveCodeDecorationType to the meta.disabled slot of Highlighter::decorationTypes, and everything should work.

[nridge]

In D67537#1833987, @hokein wrote:

there is some significant progress about semantic highlighting on VSCode, VSCode now provides an experimental semantic token API, but no LSP binding yet (is not far away I think). When it is ready, we'd like to switch to that and get rid of our own implementation.

Agreed.

(One potential issue is that vscode themes don't support background styling. However, that issue has been getting a lot of attention, hopefully it too will be fixed soon.)

I do think there's value in landing this patch in the interim.

Looking at the API there, it seems like we could make the line-style scope name meta.disable clangd specific, and define our own style.

Do you mean naming the scope something like clangd.meta.disable to make it clear it's a clangd extension?

(In that case, we might as well make it something more descriptive, like clangd.preprocessor.inactive. meta doesn't convey much information.)

[nridge]

Inject the inactive code decoration type into Highligher.decorationTypes

[merge_guards_bot]

Unit tests: fail. 62144 tests passed, 5 failed and 811 were skipped.

failed: libc++.std/language_support/cmp/cmp_partialord/partialord.pass.cpp
failed: libc++.std/language_support/cmp/cmp_strongeq/cmp.strongeq.pass.cpp
failed: libc++.std/language_support/cmp/cmp_strongord/strongord.pass.cpp
failed: libc++.std/language_support/cmp/cmp_weakeq/cmp.weakeq.pass.cpp
failed: libc++.std/language_support/cmp/cmp_weakord/weakord.pass.cpp

clang-tidy: fail. clang-tidy found 2 errors and 0 warnings. 0 of them are added as review comments below (why?).

clang-format: fail. Please format your changes with clang-format by running git-clang-format HEAD^ or applying this patch.

Build artifacts: diff.json, clang-tidy.txt, clang-format.patch, CMakeCache.txt, console-log.txt, test-results.xml

Pre-merge checks is in beta. Report issue. Please join beta or enable it for your project.

[hokein]

Do you mean naming the scope something like clangd.meta.disable to make it clear it's a clangd extension?

(In that case, we might as well make it something more descriptive, like clangd.preprocessor.inactive. meta doesn't convey much information.)

yes, this was my suggestion.

clang-tools-extra/clangd/clients/clangd-vscode/src/semantic-highlighting.ts

193

hmm, looks like clangd omits the InactiveCode token.

I'd suggest we send it like other tokens, that will simplify the patch, we don't need the inactiveDecorationIndex and isInactive, I think it also helps when we migrate to the official implementation. WDYT?

[nridge]

Update to use 'clangd.preprocessor.inactive' as the scope name for inactive lines

[nridge]

clang-tools-extra/clangd/clients/clangd-vscode/src/semantic-highlighting.ts

193

This was discussed fairly extensively in the server-side review (https://reviews.llvm.org/D67536).

In the end, we decided to go with this approach of having a separate field in the protocol for inactive lines, for a variety of reasons including the one discussed in https://reviews.llvm.org/D67536#inline-616346

clang-tools-extra/clangd/clients/clangd-vscode/src/semantic-highlighting.ts

 // structure in the semantic highlighting proposal for LSP.
 interface SemanticHighlightingInformation {
   // The zero-based line position in the text document.
   line: number;
   // A base64 encoded string representing every single highlighted characters
   // with its start position, length and the "lookup table" index of of the
   // semantic highlighting Text Mate scopes.
   tokens?: string;
+  // Whether the line is part of an inactive preprocessor branch.
+  isInactive?: boolean;
 }
 
 // A SemanticHighlightingToken decoded from the base64 data sent by clangd.
 interface SemanticHighlightingToken {
   // Start column for this token.
   character: number;
   // Length of the token.
   length: number;
   // The TextMate scope index to the clangd scope lookup table.
   scopeIndex: number;
 }
 // A line of decoded highlightings from the data clangd sent.
 export interface SemanticHighlightingLine {
   // The zero-based line position in the text document.
   line: number;
   // All SemanticHighlightingTokens on the line.
   tokens: SemanticHighlightingToken[];
+  // Whether the line is part of an inactive preprocessor branch.
+  isInactive: boolean;
 }
 
 // Language server push notification providing the semantic highlighting
 // information for a text document.
 const NotificationType =
     new vscodelc.NotificationType<SemanticHighlightingParams, void>(
         'textDocument/semanticHighlighting');
 
initialize(capabilities: vscodelc.ServerCapabilities,
     this.subscriptions.push(vscode.window.onDidChangeVisibleTextEditors(
         (editors: vscode.TextEditor[]) => editors.forEach(
             (e) => this.highlighter.applyHighlights(e.document.uri))));
     this.subscriptions.push(vscode.workspace.onDidCloseTextDocument(
         (doc) => this.highlighter.removeFileHighlightings(doc.uri)));
   }
 
   handleNotification(params: SemanticHighlightingParams) {
     const lines: SemanticHighlightingLine[] = params.lines.map(
-        (line) => ({line : line.line, tokens : decodeTokens(line.tokens)}));
+      (line) => ({
+        line: line.line, tokens: decodeTokens(line.tokens),
+        isInactive: line.isInactive || false
+      }));
     this.highlighter.highlight(vscode.Uri.parse(params.textDocument.uri),
                                lines);
   }
   // Disposes of all disposable resources used by this object.
   public dispose() {
     this.subscriptions.forEach((d) => d.dispose());
     this.subscriptions = [];
   }

 // sends.
 export class Highlighter {
   // Maps uris with currently open TextDocuments to the current highlightings.
   private files: Map<string, Map<number, SemanticHighlightingLine>> = new Map();
   // DecorationTypes for the current theme that are used when highlighting. A
   // SemanticHighlightingToken with scopeIndex i should have the decoration at
   // index i in this list.
   private decorationTypes: vscode.TextEditorDecorationType[] = [];
+  private inactiveDecorationIndex: number;
   // The clangd TextMate scope lookup table.
   private scopeLookupTable: string[][];
   constructor(scopeLookupTable: string[][]) {
     this.scopeLookupTable = scopeLookupTable;
   }
   public dispose() {
     this.files.clear();
     this.decorationTypes.forEach((t) => t.dispose());
     // Dispose must not be not called multiple times if initialize is
     // called again.
     this.decorationTypes = [];
   }
   // This function must be called at least once or no highlightings will be
   // done. Sets the theme that is used when highlighting. Also triggers a
   // recolorization for all current highlighters. Should be called whenever the
   // theme changes and has been loaded. Should also be called when the first
   // theme is loaded.
   public initialize(themeRuleMatcher: ThemeRuleMatcher) {
     this.decorationTypes.forEach((t) => t.dispose());
-    this.decorationTypes = this.scopeLookupTable.map((scopes) => {
+    this.decorationTypes = this.scopeLookupTable.map((scopes, index) => {
+      if (scopes[0] == "meta.disabled") {
+        this.inactiveDecorationIndex = index;
+        return vscode.window.createTextEditorDecorationType({
+          isWholeLine: true,
+          // FIXME: Avoid hardcoding these colors.
+          light: {
+            color: "rgb(100, 100, 100)",
+            backgroundColor: "rgba(220, 220, 220, 0.3)"
+          },
+          dark: {
+            color: "rgb(100, 100, 100)",
+            backgroundColor: "rgba(18, 18, 18, 0.3)"
+          }
+        });
+      }
       const options: vscode.DecorationRenderOptions = {
         // If there exists no rule for this scope the matcher returns an empty
         // color. That's ok because vscode does not do anything when applying
         // empty decorations.
         color : themeRuleMatcher.getBestThemeRule(scopes[0]).foreground,
         // If the rangeBehavior is set to Open in any direction the
         // highlighting becomes weird in certain cases.
         rangeBehavior : vscode.DecorationRangeBehavior.ClosedClosed,
       };
       return vscode.window.createTextEditorDecorationType(options);
     });
     this.getVisibleTextEditorUris().forEach((fileUri) =>
                                                 this.applyHighlights(fileUri));
   }
 
   // Adds incremental highlightings to the current highlightings for the file
   // with fileUri. Also applies the highlightings to any associated
   // TextEditor(s).
   public highlight(fileUri: vscode.Uri,
                    highlightingLines: SemanticHighlightingLine[]) {
     const fileUriStr = fileUri.toString();
     if (!this.files.has(fileUriStr)) {
const lines: SemanticHighlightingLine[] =
         Array.from(this.files.get(fileUriStr).values());
     const decorations: vscode.Range[][] = this.decorationTypes.map(() => []);
     lines.forEach((line) => {
       line.tokens.forEach((token) => {
         decorations[token.scopeIndex].push(new vscode.Range(
             new vscode.Position(line.line, token.character),
             new vscode.Position(line.line, token.character + token.length)));
       });
+      if (line.isInactive) {
+        decorations[this.inactiveDecorationIndex].push(new vscode.Range(
+            new vscode.Position(line.line, 0),
+            new vscode.Position(line.line, 0)));
+      }
     });
     return decorations;
   }
 }
 
 // A rule for how to color TextMate scopes.
 interface TokenColorRule {
   // A TextMate scope that specifies the context of the token, e.g.

clang-tools-extra/clangd/clients/clangd-vscode/test/semantic-highlighting.test.ts

test('ScopeRules overrides for more specific themes', () => {
     assert.deepEqual(tm.getBestThemeRule('variable.other.parameter').scope,
                      'variable.other.parameter');
     assert.deepEqual(tm.getBestThemeRule('variable.other.parameter.cpp').scope,
                      'variable.other.parameter');
   });
   test('Colorizer groups decorations correctly', async () => {
     const scopeTable = [
       [ 'variable' ], [ 'entity.type.function' ],
-      [ 'entity.type.function.method' ]
+      [ 'entity.type.function.method' ],
+      [ 'meta.disabled' ]
     ];
     // Create the scope source ranges the highlightings should be highlighted
     // at. Assumes the scopes used are the ones in the "scopeTable" variable.
     const createHighlightingScopeRanges =
         (highlightingLines:
              semanticHighlighting.SemanticHighlightingLine[]) => {
           // Initialize the scope ranges list to the correct size. Otherwise
           // scopes that don't have any highlightings are missed.
           let scopeRanges: vscode.Range[][] = scopeTable.map(() => []);
           highlightingLines.forEach((line) => {
             line.tokens.forEach((token) => {
               scopeRanges[token.scopeIndex].push(new vscode.Range(
                   new vscode.Position(line.line, token.character),
                   new vscode.Position(line.line,
                                       token.character + token.length)));
             });
+            if (line.isInactive) {
+              scopeRanges[scopeRanges.length - 1].push(new vscode.Range(
+                new vscode.Position(line.line, 0),
+                new vscode.Position(line.line, 0)
+              ));
+            }
           });
           return scopeRanges;
         };
 
     const fileUri1 = vscode.Uri.parse('file:///file1');
     const fileUri2 = vscode.Uri.parse('file:///file2');
     const fileUri1Str = fileUri1.toString();
     const fileUri2Str = fileUri2.toString();
assert.deepEqual(highlighter.applicationUriHistory,
                      [ fileUri1Str, fileUri1Str, fileUri2Str ]);
     // Groups decorations into the scopes used.
     let highlightingsInLine: semanticHighlighting.SemanticHighlightingLine[] = [
       {
         line : 1,
         tokens : [
           {character : 1, length : 2, scopeIndex : 1},
           {character : 10, length : 2, scopeIndex : 2},
-        ]
+        ],
+        isInactive: false
       },
       {
         line : 2,
         tokens : [
           {character : 3, length : 2, scopeIndex : 1},
           {character : 6, length : 2, scopeIndex : 1},
           {character : 8, length : 2, scopeIndex : 2},
-        ]
+        ],
+        isInactive: true
       },
     ];
 
     highlighter.highlight(fileUri1, highlightingsInLine);
     assert.deepEqual(highlighter.applicationUriHistory,
                      [ fileUri1Str, fileUri1Str, fileUri2Str, fileUri1Str ]);
     assert.deepEqual(highlighter.getDecorationRanges(fileUri1),
                      createHighlightingScopeRanges(highlightingsInLine));
     // Keeps state separate between files.
     const highlightingsInLine1:
         semanticHighlighting.SemanticHighlightingLine = {
       line : 1,
       tokens : [
         {character : 2, length : 1, scopeIndex : 0},
-      ]
+      ],
+      isInactive: false
     };
     highlighter.highlight(fileUri2, [ highlightingsInLine1 ]);
     assert.deepEqual(
         highlighter.applicationUriHistory,
         [ fileUri1Str, fileUri1Str, fileUri2Str, fileUri1Str, fileUri2Str ]);
     assert.deepEqual(highlighter.getDecorationRanges(fileUri2),
                      createHighlightingScopeRanges([ highlightingsInLine1 ]));
     // Does full colorizations.