gluon_language-server icon indicating copy to clipboard operation
gluon_language-server copied to clipboard

I will/want to reuse your LSP code

Open Phaiax opened this issue 5 years ago • 12 comments

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 avatar Dec 21 '18 20:12 Phaiax

@Marwes of course :)

Phaiax avatar Dec 21 '18 20:12 Phaiax

@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.

Marwes avatar Dec 25 '18 13:12 Marwes

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

Laegluin avatar Dec 25 '18 20:12 Laegluin

@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?

ebkalderon avatar Feb 28 '20 15:02 ebkalderon

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.

Marwes avatar Feb 28 '20 15:02 Marwes

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 https://github.com/ebkalderon/tower-lsp/issues/100 for some background on that.

ebkalderon avatar Feb 28 '20 15:02 ebkalderon

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. 🤔

ebkalderon avatar Feb 28 '20 15:02 ebkalderon

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).

Marwes avatar Feb 28 '20 15:02 Marwes

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 avatar Feb 28 '20 15:02 ebkalderon

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.

ebkalderon avatar Feb 28 '20 15:02 ebkalderon

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

Marwes avatar Feb 28 '20 15:02 Marwes

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.

ebkalderon avatar Feb 28 '20 16:02 ebkalderon