lapce / lapce/lapce-plugin-rust
Make handling requests/notifications nicer
Nobody has claimed this yet.
- Dominant language
- Rust
- Stars
- 31
- Forks
- 12
- PR merge metrics
- No merged PRs in 30d
Description
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 lean towards yes, though do we alert the client editor about there being nothing handling the method?
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.
Contributor guide
No contributing guide indexed for this repository
First steps
- Read the whole issue, then the project's contributing guide.
- Comment on the issue to say you are picking it up — it saves two people doing the same work.
- Fork the repository and make your change on a branch.
- Open a pull request that references the issue number.
Research direction
Start by reading the existing LapcePlugin and register_plugin interfaces, then compare them with the proposed MsgHandler, ResponseSender, and Handler designs in this issue. The work is done when the request and notification handling API, handler registration approach, and compatibility with existing plugin implementations have been agreed and implemented.
Written by the indexing model from the issue text.
Assessment
- Tech stack
- rust
- Domain
- api, developer-experience, tooling
- Issue type
- Feature
- Difficulty
- 5/5
- Estimated time
- Over a week
- Activity status
- Stale
- Clarity
- Needs clarification
- Newbie friendliness
- 25/100