fix(lsp): respect editor formatting in organize imports#4602
fix(lsp): respect editor formatting in organize imports#4602TorinAsakura wants to merge 2 commits into
Conversation
There was a problem hiding this comment.
Pull request overview
Updates the tsgo language server and its VS Code extension integration so “Organize Imports” emits edits using the active editor’s resolved formatting options (tab size / insert spaces), with workspace settings as a fallback.
Changes:
- Extend
textDocument/codeActionrequest params with optionalformattingOptionsand plumb those options through organize-imports code actions. - Normalize editor formatting settings (
tabSize/insertSpaces) into the server’s expected formatting keys when parsing user preferences. - Add fourslash coverage validating that organize-imports respects passed formatting options, and add VS Code client middleware to send per-editor options.
Reviewed changes
Copilot reviewed 9 out of 10 changed files in this pull request and generated no comments.
Show a summary per file
| File | Description |
|---|---|
| internal/lsp/lsproto/lsp_generated.go | Adds formattingOptions to CodeActionParams in generated LSP types. |
| internal/lsp/lsproto/_generate/generate.mts | Patches the generator model so CodeActionParams includes formattingOptions. |
| internal/ls/organizeimports.go | Threads formatting settings into the change tracker used to produce organize-imports edits. |
| internal/ls/codeactions.go | Passes code-action formatting options through to organize-imports action creation. |
| internal/ls/lsutil/userpreferences.go | Normalizes editor formatting keys into server format settings keys during preference parsing. |
| internal/ls/lsutil/userpreferences_test.go | Adds a unit test for editor formatting normalization in preferences parsing. |
| internal/fourslash/fourslash.go | Extends fourslash harness to allow sending code-action formatting options. |
| internal/fourslash/tests/organizeImports_removeUnused_preservesMultiline_test.go | Adds a fourslash test asserting organize-imports respects formatting options. |
| _extension/src/formattingMiddleware.ts | Adds request middleware to attach active editor tabSize/insertSpaces to code-action requests. |
| _extension/src/client.ts | Registers the new request middleware with the language client. |
Files not reviewed (1)
- internal/lsp/lsproto/lsp_generated.go: Generated file
|
I find this to be super strange. I don't think we should have to patch the LSP and introduce whole middlewares to fix this bug. Can you explain why the current code that just uses the LS formatting options does not work? |
Well, the original idea was to make organize imports follow the editor's actual settings, especially with EditorConfig in the mix. I got too focused on getting those values from the editor to the server and completely missed that this path already exists; we're just reading the settings incorrectly. So there's still something useful in the PR, but the custom plumbing around it is unnecessary. I'll strip that out and leave the small fix where the settings are read, plus the regression test. |
Summary
Fixes #4571.
Before
With tabSize set to 2 and insertSpaces disabled, organize imports still emitted four spaces:
After
Organize imports uses the editor formatting settings already provided to the language service: