You signed in with another tab or window. Reload to refresh your session.You signed out in another tab or window. Reload to refresh your session.You switched accounts on another tab or window. Reload to refresh your session.Dismiss alert
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)
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
locked as resolved and limited conversation to collaborators
Dec 20, 2020
Sign up for freeto subscribe to this conversation on GitHub.
Already have an account?
Sign in.
Current Version
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.
The text was updated successfully, but these errors were encountered: