-
-
Notifications
You must be signed in to change notification settings - Fork 2.6k
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
Vendor lsp-types
#11355
Vendor lsp-types
#11355
Conversation
There are some parts of LSP we don't use/will never support. I wonder if it's wroth stripping down the types a bit to only the functionality we actually use to reduce compile time. I think I would also like to experiment eith improving the API in places in the future. There are some instances where I remeber wishing lsp-types exposing a different API. |
3e5cc35
to
d51dee4
Compare
I think a newtpye wrapper would be convenient (or at least a type alias). That makes it easy to model URLs with a different type in the future (just replace all uses of that type) and is a bit more self-documenting |
} | ||
|
||
#[derive(Debug, Eq, PartialEq, Hash, PartialOrd, Clone, Deserialize, Serialize)] | ||
pub struct CodeActionKind(Cow<'static, str>); |
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.
This may work better as an enum for example (just one example where we could improve the api)
I think we end up using most of the type definitions actually. We could probably drop some of the LSIF stuff like monikers but it wouldn't be a lot of code |
hmm yeah those two I guess. I was also thinking of dynamic registration stuff. We do need it in some cases where LSP doens't offer any alternative bu t in general I don't really want to implement support for it. I still don't really see the point of it and it's very much optional for a client according to spec. I think there is also some minor stuff that could be cleanedup even within the code we do use (but don't use some variants). I have some things in mind but it would probably make sense to just switch over to a minimally modified version first. And once that is working we can push cleanups in self contained commits |
763884d
to
f915a37
Compare
f915a37
to
af2ac55
Compare
This pulls in the
lsp-types
crate almost as-is from the 0.95.1 release. The only change is the crate name so far. The plan is to replace the URL type used throughout the crate with aString
orBox<str>
which we can parse on our end and convert to ahelix-core::Uri
. That is left to future work - this PR just vendors the crate.