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

Allow custom methods to override default method signature #393

Open
sdankel opened this issue Apr 25, 2023 · 2 comments
Open

Allow custom methods to override default method signature #393

sdankel opened this issue Apr 25, 2023 · 2 comments
Labels
enhancement New feature or request

Comments

@sdankel
Copy link
Contributor

sdankel commented Apr 25, 2023

There are some cases where we want to implement features that are not yet added to the LSP spec. For example, for hover requests, some editors like VSCode support extra actions in the hover docs.

Unfortunately, it's not possible to just write a custom_method with the new response type. When I try this for textDocument/hover, I get:

[Error - 3:18:21 PM] Request textDocument/hover failed.
  Message: Method not found
  Code: -32601 

Even with the same method signature, I get this error. It seems to me that any custom methods should override the default error implementation, regardless of their method signature (or at least, regardless of the contents of the response type). This would give users a lot more control and flexibility with this framework.

Here's exactly what I'm trying to do:

    let (service, socket) = LspService::build(Backend::new)
        .custom_method("textDocument/hover", Backend::hover)
        .finish();

// My custom response
pub struct MyHover {
    #[serde(flatten)]
    pub hover: lsp_types::Hover,
    #[serde(skip_serializing_if = "Vec::is_empty")]
    pub actions: Vec<CommandLinkGroup>,
    // ... any other fields I want
}

impl Backend {
    pub async fn hover_req(&self, params: HoverParams) -> jsonrpc::Result<Option<MyHover>> {
        /// implementation...
    }
}

@ebkalderon What do you think?

@sdankel
Copy link
Contributor Author

sdankel commented Apr 26, 2023

I found a workaround for this particular example, but I still think this would be useful to have. Happy to submit a PR if you agree.

@ebkalderon
Copy link
Owner

ebkalderon commented Aug 11, 2023

I think this might be useful in certain cases! Although this would technically make the server not compliant with the LSP specification, and so not a recommended practice, I don't see a reason to explicitly disallow this. I think the core design of the library would likely need to shift from the current LanguageServer trait based design, though. Thanks for the suggestion! Also, I'm glad you managed to find a workaround for your specific use case in the meantime.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

No branches or pull requests

2 participants