More LSP gen cleanup, some changes for bidirectional messages #812
Conversation
There was a problem hiding this comment.
Pull Request Overview
This PR refactors generated code for the LSP by replacing legacy type aliases (e.g. LSPAny and LSPObject) with standard Go types (any and map[string]any), and it reorganizes the unmarshalling functions to support bidirectional communication. Key changes include:
- Updating LSP structures to use built-in types.
- Replacing the unmarshaller mapping with dedicated functions (unmarshalPtrTo, unmarshalAny, and unmarshalEmpty).
- Revamping the code generation script to improve method naming and simplify enum handling.
Reviewed Changes
Copilot reviewed 4 out of 4 changed files in this pull request and generated no comments.
| File | Description |
|---|---|
| internal/lsp/lsproto/lsp_generated.go | Substitutes LSPAny/LSPObject with any/map[string]any and removes legacy types. |
| internal/lsp/lsproto/lsp.go | Updates unmarshal functions to improve error handling and parameter parsing. |
| internal/lsp/lsproto/jsonrpc.go | Integrates revised unmarshalParams to streamline JSON-RPC request handling. |
| internal/lsp/lsproto/_generate/generate.mjs | Adjusts code generation logic for unmarshallers, union types, and method naming. |
Comments suppressed due to low confidence (2)
internal/lsp/lsproto/_generate/generate.mjs:219
- [nitpick] Previously, union type names were generated using title casing for each member. Confirm that using plain join without title casing is intentional to maintain consistency in type naming.
const unionTypeName = memberNames.join("Or");
internal/lsp/lsproto/_generate/generate.mjs:430
- The previous version of the code performed duplicate-checking for enum value identifiers. Please verify that the new inline generation of enum values does not lead to identifier collisions.
const enumValues = enumeration.values.map(value => ({
| return nil | ||
| } | ||
|
|
||
| if strings.HasPrefix(string(r.Method), "@ts/") { |
There was a problem hiding this comment.
For my own understanding: were we planning on having @ts/.../custom requests and changed our minds, or something else?
There was a problem hiding this comment.
This is leftover from the API testing; I talked to Andrew about it last week and he said it was fine to remove.
If we are going to tack on our own requests, we'll want to then start patching the meta model to have those custom messages be generated too.
A bunch of refactors and cleanups from the code gen as I work to get async bidirectional LSP communication working. Most of this is cleaning up cruft leftover from me testing out agent mode, but also fixes some bugs surrounding
anydecoding, method generation / duplication, which were preexisting.