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

Create/Use an alternative library for LSP structs #117

Closed
radeksimko opened this issue May 26, 2020 · 1 comment · Fixed by #311
Closed

Create/Use an alternative library for LSP structs #117

radeksimko opened this issue May 26, 2020 · 1 comment · Fixed by #311
Assignees
Labels
enhancement New feature or request upstream

Comments

@radeksimko
Copy link
Member

radeksimko commented May 26, 2020

Current Version

0.2.1

Use-cases

LS needs a library to represent LSP requests and responses and reflect the reality where clients and servers move at a different pace and hence may not use the exact same version of LSP which could be just strictly unmarshaled.

Attempted Solutions

Currently the LS uses a fork of sourcegraph/go-lsp. The main reason it uses the fork is because it allows us to tolerate missing fields in client requests - which could be missing because the client uses later version of LSP, or just because go-lsp has not codified that field yet - since all structs are maintained manually and often times fields are missing.

As pointed out by @njuCZ this solution however has a flaw. Because go-lsp uses embedded structs, custom unmarshalers may not work as expected and fields will be ignored due to the fact that only unmarshalers of the embedded structs are called, which are unaware of "parent" struct fields - which are in turn ignored.

Proposal

The go-lsp maintainer agreed that a better and more maintainable solution would be generating structs from the spec in the way gopls does it, instead of trying to keep up with the LSP spec.
sourcegraph/go-lsp#8 (comment)

gopls maintainers are hesitant to externalizing https://github.com/golang/tools/tree/master/internal/lsp/protocol just because they can't guarantee a stable API due to the code being generated from TypeScript "spec". However govim maintains their own copy of internal packages at https://github.com/govim/govim/tree/main/cmd/govim/internal/golang_org_x_tools, including the protocol library. I was told that most bugs related to breaking changes are dealt with during the generation phase and most of the remaining ones can be caught via compilation check https://github.com/govim/govim/blob/1b85ff518040c369babbb5a090eefc3fe37bd2f1/cmd/govim/gopls_server.go#L15

With that in mind I believe we could use the same approach as govim.

@ghost
Copy link

ghost commented Dec 20, 2020

I'm going to lock this issue because it has been closed for 30 days ⏳. This helps our maintainers find and focus on the active issues.
If you have found a problem that seems similar to this, please open a new issue and complete the issue template so we can capture all the context necessary to investigate further.

@ghost ghost locked as resolved and limited conversation to collaborators Dec 20, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
enhancement New feature or request upstream
Projects
None yet
Development

Successfully merging a pull request may close this issue.

1 participant