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

Support custom request and notification handlers #44

Open
wants to merge 3 commits into
base: master
Choose a base branch
from

Conversation

ferdinand-beyer
Copy link
Contributor

This PR allows users to specify custom handler functions instead of the receive-* multi-methods.

The main reason for that is to enable middleware, e.g. to log/time/trace requests, execute them on different threads, etc. Using the :request-handler and :notification-handler options, one could create Ring-style middleware:

(defn wrap-vthread [handler]
  (fn [method context params]
    (p/vthread (handler method context params))))

;; ...

(defmulti receive-request (fn [method _ _] method))

(def server (server/chan-server {:request-handler (wrap-vthread #'receive-request)}))

Avoiding global state in the multi-methods also allows having multiple LSP implementations on the classpath. Not a common use case, but we found ourselves in the situation where we had both clojure-lsp (to use the linter) and our custom LSP on the dev classpath.

Some tests could now be simplified, using :request-handler instead of with-redefs.

Copy link
Contributor

@mainej mainej left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Would you update the Receive messages section of the README to describe how to use the custom handlers?

As you do so, would you explain how the custom handlers should deal with requests or notifications that they don't know how to handle?

They have a few options. They can either return :lsp4clj.server/method-not-found, in which case lsp4clj will arrange to log the message and, in the case of requests, reply to the client with a not-found response. Or they can do these tasks themselves.

@ferdinand-beyer
Copy link
Contributor Author

@mainej -- Done. Will you please review these? You are better with words than I am :)

Copy link
Contributor

@mainej mainej left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good! Optional, but it might be nice to include the middleware example in the README.

- allows multiple server instances that don't compete on implementing the multi-methods
- allows ring-style middleware
@ferdinand-beyer
Copy link
Contributor Author

Added a middleware example to the README and added :response-executor to chan-server's doc-string. Should be ready to merge now.

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

Successfully merging this pull request may close these issues.

2 participants