Skip to content
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

I will/want to reuse your LSP code #26

Open
Phaiax opened this issue Dec 21, 2018 · 12 comments
Open

I will/want to reuse your LSP code #26

Phaiax opened this issue Dec 21, 2018 · 12 comments

Comments

@Phaiax
Copy link

Phaiax commented Dec 21, 2018

Hello @Laegluin , Hello @marves

I want to make a language server for config files and I want to use your work from this repository, especially the stuff that glues together rcp, tokio and decoding. I'll refactor that out into a standalone lib as far as possible.

I don't know yet if I publish that on crates.io, because I am usually not able to provide long term maintenance, but in the case I do publish, I want to ask you if and how you would like to be credited for your work.

Thanks for your work,
Daniel

@Phaiax
Copy link
Author

Phaiax commented Dec 21, 2018

@Marwes of course :)

@Marwes
Copy link
Member

Marwes commented Dec 25, 2018

@Phaiax That's fine for me. I have wanted to such an extraction myself sometime, just never had the time to dig into it. I'd be interested in what you come up with, it did seem a bit tricky to know where to make the cut between generic and language specific code in the server.

@Laegluin
Copy link
Contributor

@Phaiax I only did some syntax highlighting stuff, the rest is all Marwes' work anyway, so go ahead! :)

@ebkalderon
Copy link

@Phaiax @Marwes In case you are both still interested in a standalone generic LSP server library, there is tower-lsp, which used some parts of gluon-language-server as an early source of inspiration. It also relies on jsonrpc-core under the hood and works with tokio 0.2 and async/await.

I think the current version in master should be able to support all of the JSON-RPC methods used in gluon-language-server. Perhaps you could take a look at the API and see if it suitably breaks the generic parts out from the language specific parts?

@Marwes
Copy link
Member

Marwes commented Feb 28, 2020

I will take a look (when I have time, got so many PRs and branches in flight in various repos atm...).

Something that would be quite useful from a generic LSP library like that is if it could generate the correct capability object based on which methods are implemented. Might be hard to make an API for that which works though.

@ebkalderon
Copy link

No worries! I know you've got plenty on your plate, and honestly, I wouldn't want to distract you from working on gluon, codespan-lsp, lsp-types, etc. But it's there, in case you want to evaluate some crate to modernize the server with.

I honestly would love that feature too! I didn't implement that in tower-lsp for the sake of simplicity and getting a functional library off the ground, but I wouldn't mind figuring out a better way to handle this kind of boilerplate in the future. See ebkalderon/tower-lsp#100 for some background on that.

@ebkalderon
Copy link

Honestly, I wonder if maybe a proc macro on the LanguageServer trait impl block could figure out which methods were implemented for you? I think that approach could be interesting. 🤔

@Marwes
Copy link
Member

Marwes commented Feb 28, 2020

A "worse is better" approach might be good enough here. tower-lsp could perhaps provide a verify_server function which calls each method in the server with some default data and checks that methods that have the capability set are implemented (it is ok if they return an error, just not "not implemented") and that any methods that haven't the capability set do return "not implemented".

This way there isn't any illusion about being perfect, it can be good enough and extended as cases crop up (and it might even serve as a simple test suite for downstream implementation if it checked proper shutdown etc, it would only need to be ran in tests on downstream crates).

@ebkalderon
Copy link

I was considering that in the linked issue too, yeah. I wondered about the potential startup costs of that, plus the fact that a manual mapping between trait methods and InitializeResult options will have to be curated and maintained as the spec evolves, and every time lsp-types is upgraded as a dependency, I would have to review that mapping again to ensure accuracy. Maybe it's actually a less difficult task than it seems, who knows? 😅

@ebkalderon
Copy link

For the time being, anyway, I left it out as a possible addition for future versions, even if it results in a breaking API change later on.

@Marwes
Copy link
Member

Marwes commented Feb 28, 2020

The work of maintaining a mapping would be there regardless of the approach, but yes that may be reason enough not to do it.

@ebkalderon
Copy link

That's a good point! The work would exist either way. Still, I was hoping for something a bit more strongly typed so that mismatches in this mapping would be caught by the compiler and wouldn't require as much manual checking. That would require some design beforehand, of course, but I'm pretty sure such a mapping is possible.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

4 participants