ebkalderon / tower-lsp

Language Server Protocol implementation written in Rust
Apache License 2.0
962 stars 54 forks source link

Introduce a template for tower-lsp #317

Closed IWANABETHATGUY closed 2 years ago

IWANABETHATGUY commented 2 years ago

I wrote a tower-lsp boilerplate template https://github.com/IWANABETHATGUY/tower-lsp-boilerplate, which have implemented some common language features. The issue is that i used the custom request that has not been landed yet, so I am wonder when this support-custom-server-requests branch would be merged.

ebkalderon commented 2 years ago

I suspect this branch will land in mainline sometime by the end of this week. I'm trying to find some time in the evenings after work to review all the open PRs, address feedback, and merge them in the correct order. Once everything is merged, along with any follow-up PRs, I'll update the changelog and draft a new release shortly thereafter.

ebkalderon commented 2 years ago

As an aside, the Backend::test custom request handler written in your repo (source) can be rewritten in a simpler way:

-     async fn test(&self, params: serde_json::Value) -> Result<Vec<(usize, usize, String)>> {
+     async fn test(&self, params: InlayHintParams) -> Result<Vec<(usize, usize, String)>> {
         let mut hashmap = HashMap::new();
-         if let Ok(InlayHintParams { path }) = serde_json::from_value::<InlayHintParams>(params) {
-             if let Some(ast) = self.ast_map.get(&path) {
-                 ast.iter().for_each(|(k, v)| {
-                     type_inference(&v.body, &mut hashmap);
-                 });
-             }
-         }
+         if let Some(ast) = self.ast_map.get(&params.path) {
+             ast.iter().for_each(|(k, v)| {
+                  type_inference(&v.body, &mut hashmap);
+             });
+         }

The LspServiceBuilder::method() method may accept any params type which implements serde::Deserialize. Similarly, handlers may also return any jsonrpc::Result<T> where T implements serde::Serialize.

I think #313 should be updated to include clearer documentation on how to define custom requests, to make this fact more obvious.

IWANABETHATGUY commented 2 years ago

As an aside, the Backend::test custom request handler written in your repo (source) can be rewritten in a simpler way:

-     async fn test(&self, params: serde_json::Value) -> Result<Vec<(usize, usize, String)>> {
+     async fn test(&self, params: InlayHintParams) -> Result<Vec<(usize, usize, String)>> {
         let mut hashmap = HashMap::new();
-         if let Ok(InlayHintParams { path }) = serde_json::from_value::<InlayHintParams>(params) {
-             if let Some(ast) = self.ast_map.get(&path) {
-                 ast.iter().for_each(|(k, v)| {
-                     type_inference(&v.body, &mut hashmap);
-                 });
-             }
-         }
+         if let Some(ast) = self.ast_map.get(&params.path) {
+             ast.iter().for_each(|(k, v)| {
+                  type_inference(&v.body, &mut hashmap);
+             });
+         }

The LspServiceBuilder::method() method may accept any params type which implements serde::Deserialize. Similarly, handlers may also return any jsonrpc::Result<T> where T implements serde::Serialize.

I think #313 should be updated to include clearer documentation on how to define custom requests, to make this fact more obvious.

Thanks!

ebkalderon commented 2 years ago

No worries! I've updated the documentation to elaborate further on the requirements for defining custom JSON-RPC methods on servers in 357e062. Hopefully this communicates the expected usage a bit better. :blush:

EDIT: Force-pushed to 3b7e06e with some slight tweaks.

IWANABETHATGUY commented 2 years ago

357e062

I see, the documentation is very helpful, thanks.

IWANABETHATGUY commented 2 years ago

It's time to close this issue, since the branch has been merged.