streetsidesoftware / vscode-spell-checker

A simple source code spell checker for code
https://streetsidesoftware.github.io/vscode-spell-checker/
Other
1.45k stars 131 forks source link

Add native lsp support #3406

Open quolpr opened 5 months ago

quolpr commented 5 months ago

Hello! I've been working on adding LSP support to nvim and encountered a few issues:

  1. The LSP is sending _onDiagnostics instead of the expected textDocument/publishDiagnostics.
  2. The LSP doesn't send "edits" with the codeAction. Below is an example of the edit JSON: edit json
  3. It's necessary to handle the _onWorkspaceConfigForDocumentRequest command to make cspell work correctly.
  4. Nvim sends the cursor position instead of the range of the selected word on codeAction. I had to patch the server with the following code to make it work:
diff --git a/packages/_server/src/codeActions.mts b/packages/_server/src/codeActions.mts
index 5023ca80..f8f3a603 100644
--- a/packages/_server/src/codeActions.mts
+++ b/packages/_server/src/codeActions.mts
@@ -93,7 +93,16 @@ class CodeActionHandler {
             textDocument: { uri },
         } = params;
         const { diagnostics } = context;
-        const spellCheckerDiags = diagnostics.filter((diag) => diag.source === Validator.diagSource);
+
+        const firstIntersection = diagnostics.find(
+            (diag) => diag.source === Validator.diagSource && range.intersect(diag.range, params.range),
+        );
+        if (firstIntersection) {
+            params.range = firstIntersection.range;
+        }
+        const spellCheckerDiags = diagnostics.filter(
+            (diag) => diag.source === Validator.diagSource && range.intersect(diag.range, params.range),
+        );
         const eslintSpellCheckerDiags = diagnostics.filter((diag) => diag.source === 'eslint' && diag.code == '@cspell/spellchecker');

         if (!spellCheckerDiags.length && !eslintSpellCheckerDiags.length) return [];

These issues make integrating cspell into nvim and other editors challenging. I'm considering adding a new setting, like cSpell.isNativeMode, which would make cspell function like a standard LSP. This could be a significant improvement for other editors!

Here is a workaround for nvim - nvim workaround And here is one for sublime - sublime workaround

Jason3S commented 5 months ago

@quolpr,

Thank you for the detailed explanation.

As you have seen, the LSP has been strongly tailored to work with the VS Code extension. I'm open to making changes so it is easier to integration with nvim and sublime.

A compatibility mode as you suggest with cSpell.isNativeMode is a viable suggestion. It might also be easier/better to create a pure LSP CSpell Server, since there are other uses for an LSP in addition to an editor.

Background Information

  1. The LSP is sending _onDiagnostics instead of the expected textDocument/publishDiagnostics.

One of the biggest requests / complaints was to show spelling issues independent from the normal coding issues. It was requested to hide them from the problems pane and to use custom decorations. To accomplish this, it was necessary to send Diagnostics as a custom notification instead of using the LSP Diagnostics Reporting. The format of the Diagnostics have not changed, so it should still be possible, it just needs to be a setting telling it to use publishDiagnostics. Related code: https://github.com/streetsidesoftware/vscode-spell-checker/blob/07031265e648c7ed2ec0e9da6a9927e6c60949ec/packages/_server/src/server.mts#L482-L506

  1. The LSP doesn't send "edits" with the codeAction. Below is an example of the edit JSON:

It sends a command instead of edits. The command gets turned into edits on the client side. I was necessary to do it this way to use the VSCode rename feature. It is not too difficult to send edits instead.

Related code: https://github.com/streetsidesoftware/vscode-spell-checker/blob/07031265e648c7ed2ec0e9da6a9927e6c60949ec/packages/_server/src/codeActions.mts#L163-L173

The codeAction also has options to add words to the user's dictionaries. But that requires significant client side support. Set cSpellhideAddToDictionaryCodeActions to true to hide those not implemented options.

Related code: https://github.com/streetsidesoftware/vscode-spell-checker/blob/07031265e648c7ed2ec0e9da6a9927e6c60949ec/packages/_server/src/codeActions.mts#L182-L184

  1. It's necessary to handle the _onWorkspaceConfigForDocumentRequest command to make cspell work correctly.

This configuration information is connected to adding words to dictionaries (the targets) in the client. If it is not necessary to support adding words to dictionaries, then It should be possible to ignore this.

  1. Nvim sends the cursor position instead of the range of the selected word on codeAction. I had to patch the server with the following code to make it work:

I can see how that might be an issue.

Related code: https://github.com/streetsidesoftware/vscode-spell-checker/blob/07031265e648c7ed2ec0e9da6a9927e6c60949ec/packages/_server/src/codeActions.mts#L96-L111

Jason3S commented 5 months ago

@quolpr,

One of the challenges here is that I don't have any insight into who is trying to use this as a LSP service, making it very easy to break things.

In the latest version, the server sends RPC requests to the client to load file if they use a virtual protocol. See: _server/src/vfs/CSpellFileSystemProvider.mts

quolpr commented 5 months ago

@Jason3S thank you for all your comments! That will really help me. I hope I will start soon on working this 🙂

BaptisteRoseau commented 1 month ago

We would also be interested by a native LSP support to use it in the Zed editor, there is a dedicated issue on their end for this purpose: https://github.com/zed-industries/extensions/issues/1414

quolpr commented 1 month ago

Update: I've developed a working solution for Neovim LSP integration. You can find the implementation here: https://gist.github.com/quolpr/2d9560c0ad5e77796a068061c8ea439c

The workaround functions as intended, so I decided to stop working on native mode. By the way, the codebase is well-structured, making it straightforward to implement this native mode if anyone wants to take it on 🙂

Jason3S commented 1 month ago

@quolpr, Thank you. Would you make your gist public? I'm curious about your workaround.

quolpr commented 1 month ago

@Jason3S oops, here is updated URL https://gist.github.com/quolpr/2d9560c0ad5e77796a068061c8ea439c