-
Notifications
You must be signed in to change notification settings - Fork 18k
x/tools/gopls: update protocol generation to support unions #52969
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
Comments
Change https://go.dev/cl/424214 mentions this issue: |
This CL updates the LSP to 3.17.0. It is a DANGEROUS CL as the stubs are being generated by Go code reading vscode's language independent description of the protocol (in metaMode.json in the vscode-languageserver-node repository.) Some of the union types in the protocol have Go types with names containing 'Or'. These types have custom marshaling and unmarshaling code. Embedded structures in the protocol are broken out as their own types, with names constructed from the context in which they occur. The natural output has been modified to minimize the number of changes needed for gopls. (e.g., Workspace6Gn is preserved for compatibility.0 Thus, many types that are union types in the LSP description have been replaced by the types gopls already uses. Updates golang/go#52969 Change-Id: I16f6d877215155ac9e782b0f5bcbdab3f1aa2593 Reviewed-on: https://go-review.googlesource.com/c/tools/+/424214 TryBot-Result: Gopher Robot <[email protected]> gopls-CI: kokoro <[email protected]> Run-TryBot: Peter Weinberger <[email protected]> Reviewed-by: Robert Findley <[email protected]>
@pjweinb what's left here? Of the top of my head:
Anything else? |
the second has already happened, and I'm struggling with the first.
…On Wed, Sep 14, 2022 at 10:23 AM findleyr ***@***.***> wrote:
@pjweinb <https://github.com/pjweinb> what's left here? Of the top of my
head:
- check in the new generator
- sync with Dylan's change for DocumentChanges
Anything else?
—
Reply to this email directly, view it on GitHub
<#52969 (comment)>, or
unsubscribe
<https://github.com/notifications/unsubscribe-auth/ABJIAI5P6M74I4B444VEUNDV6HNULANCNFSM5WIWF7HQ>
.
You are receiving this because you were mentioned.Message ID:
***@***.***>
|
FYI: just filed #55080 for an issue I'm seeing with JSON marshalling now. |
I think we should attempt to do some additional conformance testing following #55099. One possibility would be the following:
|
Change https://go.dev/cl/443055 mentions this issue: |
This is the first in a series of CLs implementing the new stub generator. The code is intended to reproduce exactly the current state of the generated code. This CL has the final file layout, but primarily consists of the parsing of the specification. The LSP maintainers now provide a .json file describing the messages and types used in the protocol. The new code in this CL, written in Go, parses this file and generates Go definitions. The tests need to be run by hand because the metaModel.json file is not available to the presubmit tests. Related golang/go#52969 Change-Id: Id2fc58c973a92c39ba98c936f2af03b1c40ada44 Reviewed-on: https://go-review.googlesource.com/c/tools/+/443055 Reviewed-by: Robert Findley <[email protected]> Reviewed-by: Alan Donovan <[email protected]> gopls-CI: kokoro <[email protected]> TryBot-Result: Gopher Robot <[email protected]> Run-TryBot: Peter Weinberger <[email protected]>
Change https://go.dev/cl/446595 mentions this issue: |
I think this is done. |
In the implementation of #41567, it is likely that we will need to extend our support for workspace edits.
Specifically, the WorkspaceEdit protocol type supports resource operations creating, renaming, and deleting files:
However, our protocol generation only supports
[]TextDocumentEdit
:https://cs.opensource.google/go/x/tools/+/master:internal/lsp/protocol/tsprotocol.go;l=5702;drc=b7d757405fe14c924baefbdedd785e4c6a7a0f88
In order to implement package renaming, we'll want to rename directories, and so will need to be able to express this in our
WorkspaceEdit
response.CC @dle8 @pjweinb
The text was updated successfully, but these errors were encountered: