-
Notifications
You must be signed in to change notification settings - Fork 85
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
add proposed selectionRange API #102
Conversation
src/request.rs
Outdated
@@ -108,6 +108,9 @@ macro_rules! lsp_request { | |||
("textDocument/typeDefinition") => { | |||
$crate::request::GotoTypeDefinition | |||
}; | |||
("textDocument/selectionRange") => { | |||
$crate::request::SelectionRange |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Should be SelectionRangeRequest
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
yeah...
I hate how some of these are XRequest
and some are just X
, but this inconsistent naming comes from the protocol itself.
I wonder if "maintaining consistency with inconsistent protocol" is worth it...
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Whatever makes it easier I think. I'd recommend using the macros to refer to them in most cases anyway since that reflects their actual RPC name. So the name of the underlying type shouldn't matter most of the time.
3a439bd
to
1141faa
Compare
9878f00
to
6e37d45
Compare
@Marwes marked as ready for review: the protocol extension is still provisional, but is released and usable: |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
In #103 we added a proposed
feature as a way to opt-in to addition not yet in the space. If you could hide this changes behind a cfg then this looks good to me.
6e37d45
to
6b8b717
Compare
Good idea, fixed! |
Released as 0.57.1 |
It seems like this is a semver breaking change? rls can't build with 0.57.1. |
This is the proposes extension to the protocol, which looks like it's going to be stabilized soon.
See microsoft/vscode-languageserver-node#474
Let's not merge the PR until it is set in stone though