lapce-plugin-rust icon indicating copy to clipboard operation
lapce-plugin-rust copied to clipboard

Make handling requests/notifications nicer

Open MinusGix opened this issue 1 year ago • 0 comments

Currently LapcePlugin is defined as:

#[allow(unused_variables)]
pub trait LapcePlugin {
    fn handle_request(&mut self, id: u64, method: String, params: Value) {}
    fn handle_notification(&mut self, method: String, params: Value) {}
}

This is a bit eh in that it 'forces' the caller to do a big match statement over the method and then do the deserialization themselves. Ideally we would lessen that friction by default.

My initial thoughts are to have something vaguely like:
(I spell out types to make it more clear, but a lot of the callback types could be inferred)

// T is state.
// Q: A shorter/better name for `ResponseSender`?
type RequestHandler<T, V> = Box<dyn Fn(&mut T, V, ResponseSender)>;
type NotifHandler<T, V> = Box<dyn Fn(&mut T, V)>;
struct MsgHandler<T> {
    request_handlers: HashMap<String, RequestHandler<T, serde_json::Value>>,
    notif_handlers: HashMap<String, NotifHandler<T, serde_json::Value>>
}


impl<T> MsgHandler<T> {
    pub fn new() -> MsgHandler { /* ... */ }

    // Deserialize bound is a bit trickier in reality, but basically this
    // Q: The error `_` should probably be serde_json's deserialize error type but perhaps it should be more general?
    pub fn on_request<V: Deserialize, R: Serialize>(mut self, method: impl Into<String>, cb: impl Fn(&mut T, Result<V, _>, ResponseSender) + 'static) -> Self {
        // The outline of how request handlers work is that you're supposed to call the `send` function on `ResponseSender`` with your data.
        //   (We can't have it simply be returned from the function, because sometimes you only want to run LSP commands *after* you respond, like in initialize)
        let cb = Box::new(move |state: &mut T, data: serde_json::Value, resp: ResponseSender| {
            let data: Result<V, _> = serde_json::from_value(data);
            cb(state, data, resp);
        });
        self.request_handlers.insert(method.into(), cb);
    }

    pub fn on_notif<V: Deserialize>(mut self, method: impl Into<String>, cb: impl Fn(&mut T, Result<V, _>) + 'static) -> Self {
        let cb = Box::new(move |state: &mut T, data: serde_json::Value| {
            let data: Result<V, _> = serde_json::from_value(data);
            cb(state, data);
        });
        self.notif_handlers.insert(method.into(), cb);
    }

    /// Listen for the [`Initialize`] request.
    /// ... common docs here so that plugin authors don't have to read the lsp spec ...
    pub fn on_initialize(mut self, cb: impl Fn(&mut T, Result<InitializeParams, _>, ResponseSender) + 'static) -> Self {
        self.on_request(Initialize::Method, cb)
    }

    // ...
    // Various common functions.
    // We might not want to implement functions for literally every random LSP feature. Just common ones.
}

pub struct ResponseSender {
    // ... whatever is needed to send a response to the request ...
    // We could do FUN generic PhantomData to make so the closure can only send values of the allowed response type. I think that would be nice without adding complexity.

    // Q: we can log a warning if you didn't send a response/err back? Not sure if that's desirable in general.
    // sent_response: bool,
}
// ... obvious response sender impl ...

This would allow some common way of implementing a LSP startup to be like:

MsgHandler::new()
    .on_initialize(|state: &mut State, res: Result<InitializeParams, RpcError>, resp: ResponseSender| {
        resp.send(InitializeResult {
            // ...
        });

        PLUGIN_RPC.start_lsp(
            // ...
        );
    });

There's now two questions:

  • How do we say that we're using a specific message handler? There's no main function that the plugin can nicely 'start' in.
  • Do we want to allow dynamically registering handlers?
    • I lean towards yes, though do we alert the client editor about there being nothing handling the method?
      • RA logs warnings if it receives methods it doesn't handle. Maybe we should just automatically send out a command to log that, to encourage the plugin author to handle it nicely?

I'd say that what we can do is make the register_plugin have an alternate sort if you specify a 'handler creation function',

register_plugin!(State; handler: handler);

fn handler() -> MsgHandler<State> {
    MsgHandler::new()
        .on_initialize(|state: &mut State, res: Result<InitializeParams, RpcError>, resp: ResponseSender| {
            resp.send(InitializeResult {
                // ...
            });

            PLUGIN_RPC.start_lsp(
                // ...
            );
        })
}

We also introduce a new trait that acts as trait which LapcePlugin requires.

// ... inside lapce-plugin-rust
pub trait Handler {
    fn handle_request(&mut self, id: u64, method: String, params: Value);

    fn handle_notification(&mut self, method: String, params: Value);
}

pub trait LapcePlugin: Handler {}

The register_plugin trait would simply implement handler automatically.

// Example possible output of register plugin when specifying a handler function.
thread_local! {
    static STATE: std::cell::RefCell<State> = std::cell::RefCell::new(Default::default());
    static HANDLER: OnceCell<RefCell<MsgHandler<State>>> = OnceCell::new();
}

fn main() {}

pub fn handle_rpc() {
    // ... typical handle_rpc ...
}

impl Handler for State {
    fn handle_request(&mut self, id: u64, method: String, params: Value) {
        STATE.with(move |state| {
            HANDLER.get_or_init(|| RefCell::new(handler())).borrow_mut().handle_request(state, id, method, params)
        })
    }

    fn handle_notification(&mut self, method: String, params: Value) {
        STATE.with(move |state| {
            HANDLER.get_or_init(|| RefCell::new(handler())).borrow_mut().handle_notification(state, method, params)
        })
    }
}

This would allow the existing plugin implementations to just automatically work without any changes, and would allow swapping it out for a completely custom handler if desired.

MinusGix avatar Sep 09 '23 12:09 MinusGix