Support asynchronous diagnostic actions

remcohaszing commented 3 months ago

Describe the issue

I created codemirror-languageservice, which adds support for Language Server Protocol (aka LSP) based editor support.

CodeMirror supports Diagnostic.actions. This is an array of actions. These need to be defined as soon as the diagnostics are created. LSP supports code actions. These code actions are requested asynchronously, and only when the editor needs them.

Would it be possible to change CodeMirror diagnostic actions to allow an async function returning actions that only gets called when the user prompts for code actions? So the signature would be:

interface Diagnostic {
    // …
    actions?: readonly Action[] | (() => (readonly Action[] | PromiseLike<readonly Action[]>));
}

Browser and platform

No response

Reproduction link

No response

marijnh commented 3 months ago

The thing is that we have things like the diagnostic panel, which immediately renders all diagnostics (including their actions) when visible, and as such would match very poorly with asynchronously retrieved actions.

What would 'prompting' for actions look like?

remcohaszing commented 3 months ago

I did some investigation how VSCode does this based on ESLint language server. You can inspect the LSP requests that are made in the ESLint output panel if the VSCode option "eslint.trace.server.verbosity" is set to "compact".

VSCode renders diagnostics in 2 places (that I’m aware of): inline using squiggly underlines, and in the Problems panel. Within the editor, autofixes is displayed when the text cursor position is within the diagnostic range, and the user presses Ctrl + .. Within the Problems panel, autofixes are displayed when the user right-clicks the diagnostic.

For my own projects, I use ESLint rule no-console. This rule can be fixed in an editor using code actions.

Let’s say I have a JavaScript file with the following content, and | represents the text selection:

// Some newlines to help investigating behaviour

co|nsole.lo|g()

Pressing Ctrl + . triggers an LSP request with the following range:

{
  "start": {
    "line": 5,
    "character": 3
  },
  "end": {
    "line": 5,
    "character": 11
  }
}

The request also contains all ESLint related diagnostics that overlap with the selection range.

Now let’s move the cursor to an entirely different location, i.e. the beginning of the file. From the Problems panel, right-click the diagnostic. This sends a code action request with the range that matches the diagnostic range, ignoring the text selection:

{
  "start": {
    "line": 5,
    "character": 1
  },
  "end": {
    "line": 5,
    "character": 12
  }
}

The request only contains the one right-clicked diagnostic.

CodeMirror renders diagnostics in:

Squiggly lines
The lint gutter
The diagnostics panel

Since in CodeMirror action is applied on a per-diagnostic basis, I suggest to implement actions on hover different from VSCode.

The action function should be triggered when:

The user hovers the icon in the lint gutter. This calls action for all diagnostics on the line.
The user opens the diagnostics panel. This calls action for all diagnostics.
The user already has the diagnostics panel open and diagnostics are updated. This calls action for all new diagnostics.
The user hovers over a squiggly line. This calls action for all diagnostics diagnostics that match the cursor position. Ideally the action should know it’s called this way, so codemirror-languageservice knows it should use the selected range rather than the diagnostic range.

The UI should probably only be updated once all required information is fetched to avoid elements from jumping around.

marijnh commented 3 months ago

Since this approach would still have situations where the actions would all be pulled in, it doesn't seem very compelling over the approach of just fetching these right away in all situations and including that fetch in the asynchronous function that produces the list of diagnostics. If this is important to you, we can talk about a paid implementation, but I don't find the idea attractive enough to implement it for free.

remcohaszing commented 3 months ago

I’m just trying to figure out how to map LSP to CodeMirror, and in the process learning new things about both CodeMirror and LSP. I think you’re right this isn’t needed. I think the best approach for me is to get the LSP code actions directly after fetching the diagnostics, using the diagnostic range as the selection range, same as right-clicking the diagnostics in the VSCode Problems panel.

codemirror / dev