SourceKit-LSP file status UX

SourceKit-LSP file status indicators

Rough proposal/idea to add some indication of file status to SourceKit-LSP. Let me know what you think.

Motivation

  1. Give the user an idea of what they should expect - is the build system initializing? Does the
    build system recognize the file? Is (pre)building still required before full functionality can
    occur?

    • This will increased perceived reliability as long as these statuses are accurate and clear
    • SourceKit-LSP can also change behavior based on these states, e.g. potentially hold back
      diagnostics if the build system still needs to prebuild.
  2. Make the clangd/sourcekitd loading state clear. This helps isolate build system vs. compiler
    issues and lets the user know the compiler is working (even if we can't provide progress estimates)

Scope

Limit compiler file status information to AST-based indexing functionality (no index store status).
We can consider expanding this information to include the index store status as well in the future.

Two different status types: build system and compiler (sourcekitd/clangd). While it
might make sense to merge them, I'm leaning towards keeping them distinct to handle the case
where compiler and build system actions are being performed at the same time.

File Statuses

The statuses below are mostly unstructured and rough ideas, I'm not sure how structured we want to
make them just yet. We'll likely want to categorize them a bit so we can decide what icons to use,
see the UI section below.

Compiler

  • Unknown - No response from compiler yet
  • Loading/processing file
  • Ready/Idle
  • (maybe) potentially outdated (requires some structured build system integration) if dependencies need to be rebuilt

Build System

  • Unknown - No response from build system yet

  • Initializing/Analyzing

    • Build system setting up/processing, build system can provide messages here to note what it's doing
    • e.g. SPM package loading, Bazel loading + analysis phase
  • Unsupported/error

    • Build system does not support the file or an error occurred while processing (can provide message here)
    • e.g. Unknown file Foo.m or error loading package Foobar
  • Prebuilding/Resolving generated files

    • Build system building generated inputs required for compilation and indexing
    • This in particular we might want to make more structured - that will allow us to do things like
      holding off diagnostics if we think we will emit false errors due to missing inputs
    • e.g. Xcode prebuilding swiftmodules
  • Ready/Idle

UI

Since this is outside of the LSP API, we'll have to implement this into editors ourselves. For
VSCode, we can put these indicators in the status bar on a per-file basis. If we can categorize
the above types we can also put icons in here and then show the full message on hover.

3 Likes

Rust language server does that and it's great. As far as I can tell it only tells you "RLS" or "RLS⚙️", but even just knowing that the language server is working is extremely calming and reassuring when you struggled a lot with installing other language servers.

I like the general direction and particularly like this:

that will allow us to do things like holding off diagnostics if we think we will emit false errors due to missing inputs e.g. Xcode prebuilding swiftmodules

On the specifics, for the "Build System" section, would it be better to have the build system communicate to the editor directly, instead of funneling all its status through LSP?
Presumably the build system should already be communicating with the editor for building status & output etc., it could also use the same mechanism to communicate its activity status for work requested by SKLSP.

For example, I think it's better if displaying a build activity/log for "prebuilding" is something that goes through the same mechanism as a normal build.

Yeah, we could do something like that and then display the message on hover. This fits VSCode well because you don't want to take up too much status bar space. Do you know if the RLS uses the LSP's progress API for this?

Yeah, that is definitely one option, although it would need some integration into SourceKit-LSP to note the prebuilding so diagnostics can be withheld.

I guess it depends how reusable we want it to be - if it lives in SourceKit-LSP itself it becomes more of a standard UI component to all build systems SourceKit-LSP supports (if they provide the updates), otherwise it'll be on a per-build system basis (through its backchannel to the editor).

The limitation of the generalization here is limited UI control, including no interaction (unless we can somehow register a command to the status bar item and send it over the build system).

it's implemented here in the vscode extension itself, so it doesn't go though LSP at all

Thanks for writing this up!

When you edit a source file we will build a new AST - would you expect this to "processing file"? For example, if you're typing, would the expectation be that this "flickers" as it rebuilds?

If we want to keep the build system status separate, we need to mirror some of the status for cases where it impacts sourcekit-lsp:

  • Waiting for compiler arguments
  • Compiler arguments failed - e.g. unknown file, or some error loading the build information

Do you see this LSP extension as providing generic status messages that both the compiler and build system can provide messages for, but not trying to pass through a full build log?

Gotcha, it's also worth mentioning clangd also does something similar on a per-file basis.

I think that depends on how often this would appear + how long it lasts, if it's constantly there I don't think it really offers much (especially if it's normally fast). One option would be to show if it's taking some taking some (> few seconds), another would be to only show when initially loading / when reloading due to input/command changes.

Right, this is just meant to be a short status indicator to inform the user what actions the compiler and build system are performing in the background (not directly results of user actions). I'm imaging these are just messages (on a per-file basis) although we likely we will want to categorize them a bit.

So when you first open a file you can get an idea of what is going on, and potentially similar as things change (e.g. compiler arguments when a package is modified --> reload).

1 Like

:+1:

I like the suggestion of only showing it if it's taking time. The way I think about it, we're making the "all good" status a bit sticky, so that once you reach a steady state it doesn't blink at you just because it's updating, but only if it is taking a while (where a while might be 1-2 seconds).

Right, this is just meant to be a short status indicator to inform the user what actions the compiler and build system are performing in the background (not directly results of user actions). I'm imaging these are just messages (on a per-file basis) although we likely we will want to categorize them a bit.

Sounds good to me.


Unless someone pipes up to disagree, it sounds like we are generally agreed on

  • Lightweight status reporting mechanism, not kitchen sink
  • Build system status should only be exposed by this to the extent that it is blocking or significantly effecting sourcekit-lsp. More general "build progress" type notifications probably belong on a separate channel that does not involve sourcekit-lsp at all.
  • Per-file status

So these states seem fairly clear to me

  • Unknown - No response from compiler yet
  • Waiting for compiler arguments from build system
  • Loading/processing file (ie. building AST)
  • Ready/Idle

What about these?

  • Modules have not been built by build system yet
  • Failed to get compiler arguments - e.g. because the file is outside your project model

Some of these states are somewhat orthogonal as well - e.g. you can fail to get compiler arguments, but we will generate default "fallback" arguments and build an AST anyway, so you have "ready vs processing" states for the AST build, but it's still using the fallback arguments.

Yep, that SGTM. For the "show only if takes time", we could leave that delegation to the client (e.g. send the updated states as soon as we know them, leave it to the UI to debounce).

In particular I think the Loading/processing might be a little unclear depending on the LSP implementation. From speaking with some clangd folks, it looks like they have 2 threads - one for preambles and one for requests (with possibly stale preambles), but we should be able to work something out (e.g. preamble builds + processing AST after preamble ~= loading). Any idea of the difficulty here for sourcekitd?

The main question is the expected value of the fallback case - what sort of UX and correctness does the fallback case have? IMO it's not much, especially for more complex projects, I would prefer to have some distinction for these two states. They represent some sort of IDE<-->build system edge case, where we know things are wrong but we're trying our best to give partial results since they're better than nothing. And then we hold back diagnostics (just not emit them to the client) because they may just lead to user confusion.

The AST we use for code-completion is currently separate from the AST we use for everything else, so in edge cases where you're in a file that takes a long time to build an AST for, "ready/idle" might transition to "processing" just because you invoked code-completion, which might be surprising since you expect it is "ready". Code-completion may also require more expensive processing on top of the AST build, whereas things like quick help is basically free once you have an AST.

Options

  • Ignore code-completion for status, and just key off of the normal AST build
  • Make status "processing" if either a completion is happening or an AST is building

I guess it's a bit relative. There is the case where we have a primary build system for a workspace, but you are outside it => low confidence state, I agree. There is also the case where you opened a standalone file e.g. a swift script => fallback arguments are probably what you wanted anyway. Maybe the best we can do is show the diagnostics for fallback cases if and only if there is no primary build system detected. At least in editors I've played with moving the standalone file to its own window seems to give its own workspace, so that seems workable.

Hmm, I think some editors like VSCode have a separate Loading message for code complete, right? So I'm tempted to say the former, but we can try out either option and swap over based on user feedback.

Yeah that makes sense - do we want to support multiple build systems? We currently have a BuildSystemList which I'm not sure really fits the model that we want (primary and fallback)? Maybe it's worth separating build system does know about this file from this file is from a directory outside the build system's scope?

SGTM

Yeah that makes sense - do we want to support multiple build systems? We currently have a BuildSystemList which I'm not sure really fits the model that we want (primary and fallback)? Maybe it's worth separating build system does know about this file from this file is from a directory outside the build system's scope ?

I think multiple build systems might be interesting, but we don't have a real design or concrete use case for it right now. Primary + fallback makes more sense to me for now.

Ok, with this in mind we have roughly the following states:

  • Unknown (maybe this is a default state? e.g. LSP initializing)
  • Waiting for compiler arguments from build system (this should be short lived, right? depending on the build system, not sure how useful this is)
  • Build system edge cases
    • Modules not yet built (--> we modify our behavior). This will require new build system APIs to note this.
    • Fallback arguments (--> we modify our behavior). This will require us to clean up the BuildSystemList or equivalent in SourceKit-LSP.
  • Compiler loading/processing file
  • Ready/Idle (this should be sticky)

Do you think it's worth explicitly categorizing these? Only seems useful if we want to use different icons for them. Something like:

enum StatusCategory {
  case unknown
  case buildSystemLoading         // unclear how useful this one is
  case buildSystemPartialSupport  // need to build
  case buildSystemFallback        // using fallback system instead of primary

  case compilerLoading
  case compilerReady
}

We could instead split this into two separate statuses in SourceKit-LSP and then have logic to merge them. In the fallback/partial support case we just stick on that and ignore compiler status?

EDIT: One other interesting question is how to handle general SDK symbols/headers. I wonder if the BuildSystem itself should provide fallback args or be given the ability to.

Most build systems can satisfy this query quickly if they are already cached, but if they need to recalculate arguments from scratch it may be expensive. This could be during initial launch, or after a some kinds of change in configuration.

Do you think it's worth explicitly categorizing these? Only seems useful if we want to use different icons for them.

Would it be useful to provide a higher level status and separate out the details? E.g. at a high level we have

  • Initializing (could be initial state, could be waiting on build settings, basically means "waiting")
  • Working (AST is being built)
  • Ready (AST is available)
  • Low fidelity (AST is available, but we're missing modules or we're using fallback settings)

And then we can provide additional lower level information as needed? That could either be in the form of another enum with more precise information and that is more likely to grow new cases over time, or just a string description. My thinking is this lets clients bucket everything but give us more flexibility about changing the precise states.

How would this work with the current API? Would this method block or you're saying we would change this around?

Yeah those 4 sound reasonable, we can probably add in a string message to show more information on hover.

In the current API yeah it would have be based on that blocking call, but once we move to always calling back after registration it would be based on that.

Is it worth revisiting #183 then? IIRC, you still want to keep the one-off request for build settings around?

So here's a rough estimate of what work needs to be done to support this:

  • Update the BuildSystemList (or create a new impl) to have a primary and fallback BuildSystem, from the discussion a list doesn't really make sense.
  • Update the BuildSystemDelegate API to allow reporting status as well as noting the fallback/modules not yet built status.
  • Update the SourceKitServer to keep track of all these status, including status of a ToolchainLanguageServer.
    • integrate with sourcekitd
    • integrate with clangd
    • add support for withholding diagnostics in the fallback/missing modues case
  • Update the VSCode extension to support the file status.

Does that seem right?

We can remove the synchronous method from our internal BuildSystem protocol in that PR, we just need to update the fallback settings to work using the registration mechanism. Currently that PR skips the fallback settings entirely IIRC. The other thing is the the build server protocol (external), which still has the settings method. That should stay, since it's part of the protocol, but we don't need to expose that outside of the BuildServerBuildSystem implementation.

  • Update the SourceKitServer to keep track of all these status, including status of a ToolchainLanguageServer .

Couldn't we just do it in the toolchain server specifically?