Message traits #29
Message traits #29
Conversation
|
Yep! This is pretty much what I wanted in #19. I actually did some further experimentation with this idea a few weeks ago but it never went anywhere since I was never completely happy with the result. Basically I wanted a way to also generate a trait with handler methods as well such as RustLSP uses but where it was fully customisable with respect to method return types. It turned out to not quite work so my efforts fizzled out at that point without me actually implementing this part even... I think one idea from it can be salvaged to work with this PR. I thought that it might be better to describe the method/notification in terms of their actual name in the spec. So instead of |
src/lib.rs
Outdated
| pub trait Notification { | ||
| type Params; | ||
| const METHOD: &'static str; | ||
| } |
There was a problem hiding this comment.
I wonder, would it be better to move the traits and their implementations to submodules so they are not confused with the data holding types? mod rpc or mod notification + mod request?
There was a problem hiding this comment.
Yeah, actually the thought crossed my mind as well. I'll try to separate it into mod notification + mod request.
There was a problem hiding this comment.
@Marwes should I move there helper union return values that aren't defined by the protocol, but needed to encode the result type? e.g. https://github.com/gluon-lang/languageserver-types/pull/29/files#diff-b4aea3e418ccdb71239b96952d9cddb6R2000
There was a problem hiding this comment.
I could see either place being good. Maybe an argument for keeping them here is that it makes the submodules only hold types that you aren't meant to instantiate (since they don't hold data).
There was a problem hiding this comment.
Keeping them here is referring to the result type you linked to.
src/lib.rs
Outdated
| @@ -57,6 +70,14 @@ pub enum NumberOrString { | |||
| /// A request that got canceled still needs to return from the server and send a response back. | |||
| /// It can not be left open / hanging. This is in line with the JSON RPC protocol that requires | |||
| /// that every request sends a response back. In addition it allows for returning partial results on cancel. | |||
| #[derive(Debug)] | |||
| pub struct Cancel; | |||
There was a problem hiding this comment.
Since these types aren't meant to be used as values we could have them as uninhabited enums maybe?
pub enum Cancel { }That way they can only be used in as types and not values. Or pub struct Cancel(()); so they can't be instantiated.
Xanewok
force-pushed
the
message-traits
branch
from
95c34c9 to
020b870
Compare
|
Rebased and force-pushed. I toyed with a simple |
|
Looks good now I think! For doc comments/attributes in macros this should match on them, in case you need it in the future https://github.com/Marwes/combine/blob/0dba46ca7a8682e1a048dee7fc9b63d6a588ea21/src/lib.rs#L295-L301 |
|
Publishes as 0.17.0 I will probably try and port gluon's language server to this over the weekend :) |
|
Cool, thanks! RLS will probably also benefit from this as well 😄 |
Attempt at #19. I took some sweet time with this, sorry for doing it so late.
Currently LSP spec provides new
CompletionParamsandCompletionContexttypes to be used with Completion Request, so I used LSP commit in the comment and usedTextDocumentPositionParamsas param type instead.@Marwes does this align with what you wanted in #19?