Indicate inactive compilation condition code branches

[award999]

Is your feature request related to a problem? Please describe.
Add a setting to show some kind of indication for inactive compilation condition branches.
Like Xcode has had for a while now.

Xcode simply grays out the inactive branch:

Describe alternatives you've considered
Eyeballing inactive branches.

Additional context
Add any other context or screenshots about the feature request here.

[award999]

Asked by user of swift extension swiftlang/vscode-swift#1733

[ahoppen]

I just learned that clangd gained support for this in llvm/llvm-project@3f6a904 using the textDocument/inactiveRegions notification that is sent from the server to the client with the following parameters. The request is not document in clangd’s extensions (llvm/clangd-www#104).

interface InactiveRegionsParams {
  /**
   * The textdocument these inactive regions belong to.
   */
  textDocument: TextDocumentIdentifier;
  /** 
   * The inactive regions that should be sent.
   */
  regions: Range[];
}

This design is problematic for several reasons:

1. The notification from the server to the client does not specify the document version that the inactive regions are for. This means that you can get into race conditions where a textDocument/inactiveRegions notification is sent to the client just after it sent an edit, so that the inactive regions no longer match the contents shown in the editor
2. My impression is that LSP is favoring client-side pull of information opposed to server-side push (eg. the introduction of textDocument/diagnostic to complement textDocument/publishDiagnostics (which incidentally had the same race condition problem until the introduction of the version field in LSP 3.15.0). I agree with this direction, because client-side pull allows the client to cancel in-flight requests, gives the client control about de-bouncing when to request this kind of information and is generally more explicit.

So, when we implement this, I would propose to deviate from the clangd design:

textDocument/inactiveRanges

New requests that is sent from the client to the server to get the ranges of the source file that are inside conditional compilation clauses that are not being compiled in the context of the conditional compilation parameters with which the source file is currently being viewed.

• params: InactiveRangesParams
• result: InactiveRange[]

export InactiveRangesParams {
  /**
   * The document to compute the inactive regions for.
   */
  textDocument: TextDocumentIdentifier;

  /**
   * The visible document range for which the inactive regions should be computed.
   */
  range: Range;
}

export InactiveRange {
  /** 
   * The range that is inactive within the context in which the file is currently viewed.
   */
  var range: Range;
}

A couple thoughts on the design:

• The request name shouldn’t be textDocument/inactiveRegions because having a request and a notification with the same name would confuse our decoding logic in SourceKit-LSP. While it might be possible to work around this, it would also be confusing during debugging because you can’t easily tell if textDocument/inactiveRegions refers to clangd’s server-side notification or our request
• I chose to make the result InactiveRange[] instead of Range[] because this might allow us to add additional metadata to the inactive regions in the future. Not planning to implement this but I could think about things like a description why the range is inactive or jump-to-definition to the condition of the range being reasonable requests in the future.

On the clangd side, we would need to capture the textDocument/inactiveRegions notifications sent by clangd and then expose them in our pull-based diagnostics.

@award999, I’d like to hear your thoughts on this request design and if you can think of ways that it could be improved for the implementation in the Swift VS Code Extension.

[award999]

@ahoppen so if I'm understanding clangd's textDocument/inactiveRegions notification is not part of the LSP spec, and looking at the spec quickly seems to confirm that. I agree we'd want to name the request something different to avoid confusion. On the vscode side we can do a decoration collection that will set the opacity of these inactive lines so no concern there. Since the is not in the spec, we cannot rely on the client to do the synchronization for us for free. Now maybe we do something similar to document test discovery where each time we get a new list of symbols we poll for inactive regions too?

onDocumentSymbols(
    document: vscode.TextDocument,
    symbols: vscode.DocumentSymbol[] | null | undefined
) {
    const uri = document?.uri;
    if (
        this.testExplorer &&
        symbols &&
        uri &&
        uri.scheme === "file" &&
        isPathInsidePath(uri.fsPath, this.folder.fsPath)
    ) {
        void this.testExplorer.getDocumentTests(this, uri, symbols);
    }
}

No issue owning our own InactiveRange type. Do we have any information already that we could provide from day 1? The decoration API already allows hover messages https://code.visualstudio.com/api/references/vscode-api#DecorationOptions

[ahoppen]

Now maybe we do something similar to document test discovery where each time we get a new list of symbols we poll for inactive regions too?

Inactive regions can change without the list of symbols changing, eg. when you switch #if true to #if false, so I would say that this needs to be re-requested after edits with a debounce duration (eg. 2 seconds).

Do we have any information already that we could provide from day 1?

I can’t think of anything right now that would provide value and I don’t think we should add a message just because we can.

[award999]

I can’t think of anything right now that would provide value and I don’t think we should add a message just because we can.

Sounds good, no need to make stuff up

Inactive regions can change without the list of symbols changing, eg. when you switch #if true to #if false, so I would say that this needs to be re-requested after edits with a debounce duration (eg. 2 seconds).

My guess is that the document symbols request would still get triggered for this change? At least on the vscode side, it doesn't look like we check if the list actually changed.

[ahoppen]

My guess is that the document symbols request would still get triggered for this change? At least on the vscode side, it doesn't look like we check if the list actually changed.

Oh, I misunderstood you. I thought you said that you wanted to run this request whenever the symbols changed but you meant that you’ll run it whenever you get a document symbols response from SK-LSP. That makes sense to me.

[award999]

Cool, then I think the design and proposed plan works