Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Support asynchronous diagnostic actions #1425

Closed
remcohaszing opened this issue Aug 16, 2024 · 4 comments
Closed

Support asynchronous diagnostic actions #1425

remcohaszing opened this issue Aug 16, 2024 · 4 comments

Comments

@remcohaszing
Copy link

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
Copy link
Member

marijnh commented Aug 18, 2024

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
Copy link
Author

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
Copy link
Member

marijnh commented Aug 19, 2024

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
Copy link
Author

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.

@remcohaszing remcohaszing closed this as not planned Won't fix, can't repro, duplicate, stale Aug 22, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants