Conversation
* Changed Notification/Method enum variant names to match those implicitly-defined in languageserver-types crate (used for fetching LSP method name from ServerMessage) * Implemented simple serialize method for ServerMessages * Removed string-based Message in tests, those now use ServerMessage in both mock_(raw_)server scenarios
nrc
left a comment
There was a problem hiding this comment.
Looks good, I think this improves the tests a lot.
Could we remove the 'raw' mock message reader and use this setup for those tests too?
src/test/mod.rs
Outdated
| const TEST_TIMEOUT_IN_SEC: u64 = 10; | ||
|
|
||
| impl ServerMessage { | ||
| pub fn simple_test_initialize(id: usize, root_path: Option<String>) -> ServerMessage { |
There was a problem hiding this comment.
I would just call this initialize (and does it need to be pub?)
There was a problem hiding this comment.
I'll rename it. It's pub because I also used it here. Should I get rid of that?
There was a problem hiding this comment.
No pub is fine if it needs to be, I missed that usage.
src/test/mod.rs
Outdated
|
|
||
| let messages = vec![ | ||
| ServerMessage::simple_test_initialize(0,root_path.as_os_str().to_str().map(|x| x.to_owned())), | ||
| ServerMessage::Request(Request { id: 11, method: Method::GotoDefinition(TextDocumentPositionParams { |
There was a problem hiding this comment.
style nit: put some newlines between the fields (or run it through rustfmt)
There was a problem hiding this comment.
I think you could probably factor out a request helper method (or macro)
There was a problem hiding this comment.
Would something like:
fn request(id: usize, method: Method) -> ServerMessage {
ServerMessage::Request(Request { id: id, method: method })
}
e.g.
ServerMessage::request(11, Method::GotoDefinition(TextDocumentPositionParams {
text_document: text_doc,
position: pos
}))
be good enough (also asking about style)?
I think it strikes a good balance between reducing redundancy and simplification while allowing to express every request type.
To get rid of explicit need to pass method params struct type I'd have to enumerate over variants (which I don't know how to do yet - I assume it needs macros 1.1 like here?) to get their payload types.
There was a problem hiding this comment.
Would something like:
fn request<F>(id: usize, ..., mk_method: F) -> ServerMessage
where F: FnOnce(Position) -> Method
{
let pos = ...;
ServerMessage::Request(Request { id: id, method: mk_method(pos) })
}
work?
Looks fine, style-wise.
|
@nrc yes, pushed mock_raw_server -> mock_server change just now. |
|
Awesome, thank you! |
Implemented #305 (comment).