Skip to content

refactor(dev): switch to gopls for formatting to fix race condition#4

Merged
cungminh2710 merged 1 commit into
mainfrom
lsp-formatter
Mar 28, 2026
Merged

refactor(dev): switch to gopls for formatting to fix race condition#4
cungminh2710 merged 1 commit into
mainfrom
lsp-formatter

Conversation

@MuenYu
Copy link
Copy Markdown
Collaborator

@MuenYu MuenYu commented Mar 28, 2026

What this PR for?

Replace external formatter (gci + shell script with locking) with gopls LSP formatting. This eliminates file corruption issues when saving rapidly in Zed.

  • Update .zed/settings.json to use gopls with format_on_save
  • Replace gci with goimports in Makefile to match gopls behavior
  • Remove scripts/fmt-single.sh (no longer needed)
  • Enable gofumpt formatting via gopls settings

What did you do for validating the changes?

Anything else you want to add? (Optional)

@greptile-apps
Copy link
Copy Markdown
Contributor

greptile-apps Bot commented Mar 28, 2026

Greptile Summary

This PR replaces the race-prone external formatter pipeline (a shell-locked gci invocation triggered via make fmt-file) with gopls' built-in format_on_save in Zed, eliminating file corruption on rapid saves. The Makefile's fmt/fmt-file targets are updated to swap gci for goimports, and the tool ordering is now correctly goimportsgofumpt so that gofumpt always has the final word on formatting. Alongside the formatter changes the PR also runs go mod tidy, which removes gci from the tool block, promotes modernc.org/sqlite from // indirect to a direct require (correct, since sqlite_integration_test.go blank-imports it), and adds the transitive modernc.org/* entries to go.sum.

Key changes:

  • .zed/settings.json: format_on_save switched to \"on\" using gopls; formatting.gofumpt: true enables stricter formatting through the LSP
  • Makefile: gci replaced by goimports, tool order fixed to goimportsgofumpt in both fmt and fmt-file
  • go.mod: gci removed from tool block; modernc.org/sqlite correctly promoted to a direct dependency
  • go.sum: cleaned up stale golang.org/x/exp version pin; transitive sqlite deps added

Note: The PR description mentions removing scripts/fmt-single.sh, but that file did not exist in the base branch — the description appears to reference an earlier iteration of this work.

Confidence Score: 5/5

Safe to merge — all previously flagged issues are resolved and no new issues were found.

All P1 concerns from prior review threads (gci in tool block, tool ordering in Makefile) are correctly addressed. The go.mod/go.sum changes are consistent with a go mod tidy run. No new logic bugs, security issues, or regressions were identified. All four changed files are in good shape.

No files require special attention.

Important Files Changed

Filename Overview
.zed/settings.json Switches format-on-save from external make command to gopls built-in; enables gofumpt via gopls settings. Clean, correct change.
Makefile Replaces gci with goimports in fmt/fmt-file targets; order is now goimports→gofumpt (correct, gofumpt gets the last word).
go.mod Removes gci from tool block; promotes modernc.org/sqlite to a direct require (correct — it is blank-imported in sqlite_integration_test.go). gci remains as an indirect dep pulled in by golangci-lint, which is expected.
go.sum Adds transitive deps for modernc.org/sqlite (now direct); cleans up stale golang.org/x/exp v0.0.0-20250620022241 entries; consistent with go mod tidy output.

Flowchart

%%{init: {'theme': 'neutral'}}%%
flowchart TD
    subgraph Before["Before (race-prone)"]
        A1["File save in Zed"] --> B1["External: make fmt-file FILE=path"]
        B1 --> C1["gofumpt -w FILE"]
        C1 --> D1["gci write FILE\n(shell lock script)"]
        D1 --> E1["⚠️ Race: rapid saves\ncorrupt file"]
    end

    subgraph After["After (this PR)"]
        A2["File save in Zed"] --> B2["gopls format_on_save"]
        B2 --> C2["gofumpt via gopls\n(formatting.gofumpt: true)"]
        C2 --> D2["✅ Single LSP op,\nno race condition"]
    end

    subgraph Makefile["Makefile fmt target (CI)"]
        M1["goimports -w"] --> M2["gofumpt -w\n(gofumpt wins)"]
    end
Loading

Reviews (2): Last reviewed commit: "refactor(dev): switch to gopls for forma..." | Re-trigger Greptile

Comment thread Makefile
Comment thread Makefile Outdated
Comment thread Makefile Outdated
Replace external formatter (gci + shell script with locking) with gopls
LSP formatting. This eliminates file corruption issues when saving
rapidly in Zed.

- Update .zed/settings.json to use gopls with format_on_save
- Replace gci with goimports in Makefile to match gopls behavior
- Remove scripts/fmt-single.sh (no longer needed)
- Enable gofumpt formatting via gopls settings
@MuenYu
Copy link
Copy Markdown
Collaborator Author

MuenYu commented Mar 28, 2026

@greptile review

@cungminh2710 cungminh2710 merged commit 1676df7 into main Mar 28, 2026
2 checks passed
@cungminh2710 cungminh2710 deleted the lsp-formatter branch March 28, 2026 21:47
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants