Plugin tutorial, more changes#4570
Conversation
39a170a to
519de6e
Compare
519de6e to
808ddbf
Compare
VeryMilkyJoe
left a comment
There was a problem hiding this comment.
Thanks for picking up this PR!
I have some comments and suggestions.
| * Request handlers respond to requests from an LSP client. They must fulfill these requests as quickly as possible. | ||
| * Notification handlers receive notifications from code not directly triggered by a user or client. | ||
| * Rules add new targets to the Shake build graph. Most plugins do not need to define new rules. | ||
| * Commands are an LSP abstraction for user-initiated actions that the server handles. These actions can be long-running and involve multiple modules. |
There was a problem hiding this comment.
I think the rules could be discussed later on instead of here.
I would rather write this as:
| * Request handlers respond to requests from an LSP client. They must fulfill these requests as quickly as possible. | |
| * Notification handlers receive notifications from code not directly triggered by a user or client. | |
| * Rules add new targets to the Shake build graph. Most plugins do not need to define new rules. | |
| * Commands are an LSP abstraction for user-initiated actions that the server handles. These actions can be long-running and involve multiple modules. | |
| There are two main groups of communication via the Language Server Protocol, namely: | |
| * A request-response type interaction where one party sends a message which the other party has to respond to somehow | |
| * `pluginHandlers` handle client requests and provide a response | |
| * Example: When you want to format a file the client sends the [`textDocument/formatting`](https://microsoft.github.io/language-server-protocol/specifications/lsp/3.17/specification/#textDocument_formatting) to the server which triggers the server to format the file, the server then responds with the formatted file content | |
| * A notification, which is a one-way interaction, where one party sends a message without expecting any response | |
| * `pluginNotificationHandlers` handle notifications sent from the client to the server | |
| * `pluginCommands` are a special type of notification, these are also sent to the server but can be triggered by the user instead of happening automatically | |
| * Example: Whenever you modify a Haskell file, the client sends a notification, informing HLS about how the file has changed | |
| > **Note:** The LSP client and server can both send requests or notifications to the other party. |
There was a problem hiding this comment.
This is a good summary of the LSP communication scheme. I moved part of the summary before "The anatomy of a plugin" into a subsection called "Notes about the Language Server Protocol" and tried to consolidate the rest. I think the text has improved a lot.
| return (Right Null) | ||
| ``` | ||
|
|
||
| `runImportCommand` [sends a request](https://hackage.haskell.org/package/lsp/docs/Language-LSP-Server.html#v:sendRequest) to the client using the method `SWorkspaceApplyEdit` and the parameters `ApplyWorkspaceEditParams Nothing edit`, providing a response handler that does nothing. It then returns `Right Null`, which is an empty `Aeson.Value` wrapped in `Right`. |
There was a problem hiding this comment.
Maybe explain why we have an Either type here, and that Left is used for failures?
There was a problem hiding this comment.
I have moved the description of the Either up to when we introduce CommandFunction. Please have a look.
|
Thanks for your detailed review @VeryMilkyJoe , I have this on my radar, just didn't get to it yet! |
808ddbf to
0730a8c
Compare
|
Hi @VeryMilkyJoe , I have implemented your proposed changes, thank you. Please have a look if the result is to your liking. One more involved task is still left: The EDIT: Ah, and I forgot, I also update the Nix Flake lock file. This changes the default GHC from 9.6 to 9.8 for Nix users. I guess we should outsource this into a separate PR, but just so you know. |
VeryMilkyJoe
left a comment
There was a problem hiding this comment.
Looks great thank you, I just spotted a spelling mistake!
222c35a to
776301a
Compare
|
I pushed an update where we are compiling the In the process, I had to re-implement large parts of the code in the tutorial, which I think is a net-positive. |
|
That's great, thank you! I focussed on the text, and mostly copied the code over, so it's great if we can fix that too! |
fendor
left a comment
There was a problem hiding this comment.
LGTM, thanks @dschrempf for pushing this over the finish line!
Some changes are mine, but many have been cherry picked and amended from PR haskell#3655 by Christian Georgii <cgeorgii@gmail.com>.
Makes sure the plugin-tutorial can never be out-of-date again.
|
Wuhuu! 🎉 |
Some changes are mine, but many have been cherry picked and amended from PR #3655 by Christian Georgii cgeorgii@gmail.com. @cgeorgii
This PR now includes most changes of #3655. I did not include the last sections which are behind a "what shall we do with those" comment in the aforementioned PR. I am not sure how to deal with those.
Another issue is that the link to the gist is outdated (and still links to the original one by Pepe Iborra). So I think we either have to remove the link or write a new gist and replace the link. What are your opinions?
I did not implement the literate file stuff, it seems too complicated to achieve in one PR.
Supersedes #3655.