I'm not too concerned about this latter part, as I assume the inability to suggest something to fix is a consequence of being in a state where there is nothing to fix.
","upvoteCount":1,"answerCount":1,"acceptedAnswer":{"@type":"Answer","text":"I will implement the feature set check since it is mainstream implementation style. But I still hope all client integration can be fully spec-compliant.
\nNote, LSP protocol does not mandate server/client to not send unsupported requests or not to listen to unsupported notifications. Since the protocol does require a request to be responded to with an error, it is possible to try...except... the request without feature detection, a popular style in Python.
Help understanding 'publish diagnostics' and 'Fix diagnostic code action':s requirements #2211
-
|
There is an open pull request #4975 to add support for ast-grep to ALE (Asynchronous Lint Engine for vim), which I'm trying to help getting merged. As can be seen this comment, I'm struggling a bit with how ast-grep is intended to be used. The comment shows how running Communication messages seem to be exchanged, but no diagnostics are arriving. As can be seen, ale does call textDocument/didOpen. Both rootUri and rootPath seems to be correctly set and sent. Is something else expected, could there be a mistake in my setup or is there something else I fail to understand? As one can see from me referencing documentation in the pull request, I have attempted reading it. I have however not tried looked at ast-grep's source code to understand what's going on. While the conversation in the pull request leaves out some detail on the next steps, I've also attempted to use the code actions for fixers. I see ast-grep announcing itself as a codeActionProvider for quickfix.ast-grep and source.fixAll.ast-grep, but any attempts to call textDocument/codeAction only results in: { "jsonrpc": "2.0", "result": null, "id": 2 }I'm not too concerned about this latter part, as I assume the inability to suggest something to fix is a consequence of being in a state where there is nothing to fix. |
Beta Was this translation helpful? Give feedback.
Replies: 1 comment 12 replies
-
ast-grep's diagnostics are published using https://microsoft.github.io/language-server-protocol/specifications/lsp/3.17/specification/#textDocument_publishDiagnostics Note the code action provider also relies on the published diagnostics are passed by client with associated diagnostics. https://github.com/ast-grep/ast-grep/blob/main/crates/lsp/src/lib.rs#L481-L501 So I think the image above shows that no publishDiagnostics are received. Would you like to investigate on that? |
Beta Was this translation helpful? Give feedback.
-
|
Beta Was this translation helpful? Give feedback.
-
|
I checked several LSP server implementation and most of them retain the feature sets supported by client. I think this is a common practice for LSP server. That said, not returning an error is definitely a non-spec-following behavior, and LSP client developers shall NOT keep the ticket open for years. |
Beta Was this translation helpful? Give feedback.
-
|
Please lets keep this technical. Opinions about how uninvolved third parties ought to act are not actionable by us. You obviously have read the ale issue. Commenting there has a higher chance of reaching that particular LSP client's developer. Complaints would unlikely lead to anything positive though, since he has already made the situation clear:
If we focus on what you and me can practically do to improve things, these are the options I see:
I stroke out the first two ones. If I were to attempt writing mergeable vim script to reject unknown rpc requests, it would likely take several full days or even weeks with my levels of skill and familiarity with the ale sources. You could likely do it quicker, but my assumption is that it's not an activity you are interested in. Introducing a not yet fully understood, and possibly semver breaking, change to tower-lsp-server is not a task of lower complexity. Options 2-4 remain on the table. This is your project, and thus its your decision. Should I interpret the retain the feature sets… as that option 2 is the desired path after all? |
Beta Was this translation helpful? Give feedback.
-
|
I will implement the feature set check since it is mainstream implementation style. But I still hope all client integration can be fully spec-compliant. Note, LSP protocol does not mandate server/client to not send unsupported requests or not to listen to unsupported notifications. Since the protocol does require a request to be responded to with an error, it is possible to |
Beta Was this translation helpful? Give feedback.
-
|
fixed in 44e2cdd |
Beta Was this translation helpful? Give feedback.
I will implement the feature set check since it is mainstream implementation style. But I still hope all client integration can be fully spec-compliant.
Note, LSP protocol does not mandate server/client to not send unsupported requests or not to listen to unsupported notifications. Since the protocol does require a request to be responded to with an error, it is possible to
try...except...the request without feature detection, a popular style in Python.