Skip to content
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

Format on save for Go source files doesn't respect editor.formatOnSave setting #1815

Open
jpap opened this issue Sep 28, 2021 · 11 comments
Open
Labels
NeedsInvestigation Someone must examine and confirm this is a valid issue and not a duplicate of an existing one.

Comments

@jpap
Copy link

jpap commented Sep 28, 2021

What version of Go, VS Code & VS Code Go extension are you using?

Version Information
  • Run go version to get version of Go from the VS Code integrated terminal.
    • go version go1.17 darwin/amd64
  • Run gopls -v version to get version of Gopls from the VS Code integrated terminal.
Build info
----------
golang.org/x/tools/gopls v0.7.2
    golang.org/x/tools/gopls@v0.7.2 h1:kRKKdvA8GOzra8rhSFDClOR7hV/x8v0J0Vm4C/gWq8s=
    github.com/BurntSushi/toml@v0.4.1 h1:GaI7EiDXDRfa8VshkTj7Fym7ha+y8/XxIgD2okUIjLw=
    github.com/google/go-cmp@v0.5.6 h1:BKbKCqvP6I+rmFHt06ZmyQtvB8xAkWdhFyr0ZUNZcxQ=
    github.com/sergi/go-diff@v1.1.0 h1:we8PVUC3FE2uYfodKH/nBHMSetSfHDR6scGdBi+erh0=
    golang.org/x/mod@v0.4.2 h1:Gz96sIWK3OalVv/I/qNygP42zyoKp3xptRVCWRFEBvo=
    golang.org/x/sync@v0.0.0-20210220032951-036812b2e83c h1:5KslGYwFpkhGh+Q16bwMP3cOontH8FOep7tGV86Y7SQ=
    golang.org/x/sys@v0.0.0-20210809222454-d867a43fc93e h1:WUoyKPm6nCo1BnNUvPGnFG3T5DUVem42yDJZZ4CNxMA=
    golang.org/x/tools@v0.1.6-0.20210908190839-cf92b39a962c h1:C0nyHiBU2m0cR6hDiUORWqQIt3h37wsp1255QBSSXqY=
    golang.org/x/xerrors@v0.0.0-20200804184101-5ec99f83aff1 h1:go1bK/D/BFZV2I8cIQd1NKEZ+0owSTG1fDTci4IqFcE=
    honnef.co/go/tools@v0.2.0 h1:ws8AfbgTX3oIczLPNPCu5166oBg9ST2vNs0rcht+mDE=
    mvdan.cc/gofumpt@v0.1.1 h1:bi/1aS/5W00E2ny5q65w9SnKpWEF/UIOqDYBILpo9rA=
    mvdan.cc/xurls/v2@v2.3.0 h1:59Olnbt67UKpxF1EwVBopJvkSUBmgtb468E4GVWIZ1I=
  • Run code -v or code-insiders -v to get version of VS Code or VS Code Insiders.

    • 1.60.2
      7f6ab5485bbc008386c4386d08766667e155244e
      x64
  • Check your installed extensions to get the version of the VS Code Go extension

    • v0.28.1

Describe the bug

  • When I disable the editor.formatOnSave setting, vscode-go still formats Go files on save.
  • Only when I use the following manual JSON setting does vscode-go disable format on save:
  "[go]": {
    "editor.formatOnSave": false 
  }
  • It would be nice if vscode-go respects the global editor.formatOnSave setting, as the workaround above must be set up by editing the VSC settings json file; it is not selectable via the GUI settings panel.
@gopherbot gopherbot added this to the Untriaged milestone Sep 28, 2021
@jpap jpap changed the title Go "format on save" doesn't respect editor.formatOnSave Format on save for Go source files doesn't respect editor.formatOnSave setting Sep 28, 2021
@stamblerre
Copy link
Contributor

Can you please share your gopls logs? Information on how to capture them can be found here.

The setting works by not sending formatting requests to the extension when formatOnSave is disabled, so if there is a bug here, it would be a VS Code bug, not a Go extension bug.

@stamblerre stamblerre added the WaitingForInfo Issue is not actionable because of missing required information, which needs to be provided. label Sep 28, 2021
@stamblerre stamblerre modified the milestones: Untriaged, Backlog Sep 28, 2021
@jpap
Copy link
Author

jpap commented Sep 28, 2021

Thanks for your speedy response!

The setting works by not sending formatting requests to the extension when formatOnSave is disabled, so if there is a bug here, it would be a VS Code bug, not a Go extension bug.

The bug exists whether with gopls enabled or disabled.

Can you please share your gopls logs? Information on how to capture them can be found here.

The following was observed with the following VSCode setting:

 "editor.formatOnSave": false,
Click to expand I've redacted paths; but you should get the idea...
[Trace - 14:22:21.404 PM] Sending request 'textDocument/codeAction - (57)'.
Params: {"textDocument":{"uri":"file://$PATH_TO_PROJECT/main.go"},"range":{"start":{"line":0,"character":0},"end":{"line":12,"character":0}},"context":{"diagnostics":[{"range":{"start":{"line":3,"character":13},"end":{"line":3,"character":19}},"message":"\"redacted\": Unknown word.","severity":3,"source":"cSpell"}],"only":["source.organizeImports"]}}


[Trace - 14:22:21.416 PM] Received response 'textDocument/codeAction - (57)' in 11ms.
Result: [{"title":"Organize Imports","kind":"source.organizeImports","edit":{"documentChanges":[{"textDocument":{"version":17,"uri":"file://$PATH_TO_PROJECT/main.go"},"edits":[{"range":{"start":{"line":3,"character":7},"end":{"line":3,"character":7}},"newText":"(\n\t\"fmt\"\n\n\t"},{"range":{"start":{"line":3,"character":27},"end":{"line":6,"character":4}},"newText":""}]}]}}]


[Trace - 14:22:21.432 PM] Sending notification 'textDocument/didChange'.
Params: {"textDocument":{"uri":"file://$PATH_TO_PROJECT/main.go","version":18},"contentChanges":[{"range":{"start":{"line":3,"character":27},"end":{"line":6,"character":4}},"rangeLength":17,"text":""},{"range":{"start":{"line":3,"character":7},"end":{"line":3,"character":7}},"rangeLength":0,"text":"(\n\t\"fmt\"\n\n\t"}]}


[Trace - 14:22:21.433 PM] Sending request 'textDocument/formatting - (58)'.
Params: {"textDocument":{"uri":"file://$PATH_TO_PROJECT/main.go"},"options":{"tabSize":2,"insertSpaces":false}}


[Trace - 14:22:21.441 PM] Received notification 'window/logMessage'.
Params: {"type":1,"message":"2021/09/28 14:22:21 command-line-arguments:file=$PATH_TO_PROJECT/main.go: no dep handle for $PkgPath: no metadata for $PkgPath\n\tsnapshot=24\n"}


[Trace - 14:22:21.441 PM] Received response 'textDocument/formatting - (58)' in 8ms.
Result: []


[Error - 2:22:21 PM] 2021/09/28 14:22:21 command-line-arguments:file=$PATH_TO_PROJECT/main.go: no dep handle for $PkgPath: no metadata for $PkgPath
	snapshot=24

[Trace - 14:22:21.488 PM] Sending notification 'textDocument/didSave'.
Params: {"textDocument":{"uri":"file://$PATH_TO_PROJECT/main.go"}}


[Trace - 14:22:21.495 PM] Received notification 'window/logMessage'.
Params: {"type":1,"message":"2021/09/28 14:22:21 command-line-arguments:file=$PATH_TO_PROJECT/main.go: no dep handle for $PkgPath: no metadata for $PkgPath\n\tsnapshot=25\n"}


[Error - 2:22:21 PM] 2021/09/28 14:22:21 command-line-arguments:file=$PATH_TO_PROJECT/main.go: no dep handle for $PkgPath: no metadata for $PkgPath
	snapshot=25

[Trace - 14:22:21.627 PM] Sending request 'textDocument/foldingRange - (59)'.
Params: {"textDocument":{"uri":"file://$PATH_TO_PROJECT/main.go"}}


[Trace - 14:22:21.627 PM] Received response 'textDocument/foldingRange - (59)' in 0ms.
Result: [{"startLine":3,"startCharacter":8,"endLine":6,"endCharacter":23,"kind":"imports"},{"startLine":9,"startCharacter":13,"endLine":10,"endCharacter":28}]


[Trace - 14:22:21.746 PM] Sending request 'textDocument/codeLens - (60)'.
Params: {"textDocument":{"uri":"file://$PATH_TO_PROJECT/main.go"}}


[Trace - 14:22:21.748 PM] Sending request 'textDocument/documentSymbol - (61)'.
Params: {"textDocument":{"uri":"file://$PATH_TO_PROJECT/main.go"}}


[Trace - 14:22:21.762 PM] Received response 'textDocument/codeLens - (60)' in 16ms.
Result: null


[Trace - 14:22:21.762 PM] Received response 'textDocument/documentSymbol - (61)' in 14ms.
Result: [{"name":"main","detail":"()","kind":12,"range":{"start":{"line":9,"character":0},"end":{"line":11,"character":1}},"selectionRange":{"start":{"line":9,"character":5},"end":{"line":9,"character":9}}}]


[Trace - 14:22:22.290 PM] Sending notification 'workspace/didChangeWatchedFiles'.
Params: {"changes":[{"uri":"file://$PATH_TO_PROJECT/main.go","type":2}]}


[Trace - 14:22:22.302 PM] Received notification 'window/logMessage'.
Params: {"type":1,"message":"2021/09/28 14:22:22 command-line-arguments:file=$PATH_TO_PROJECT/main.go: no dep handle for $PkgPath: no metadata for $PkgPath\n\tsnapshot=26\n"}


[Error - 2:22:22 PM] 2021/09/28 14:22:22 command-line-arguments:file=$PATH_TO_PROJECT/main.go: no dep handle for $PkgPath: no metadata for $PkgPath
	snapshot=26

[Trace - 14:22:22.427 PM] Sending request 'textDocument/documentLink - (62)'.
Params: {"textDocument":{"uri":"file://$PATH_TO_PROJECT/main.go"}}


[Trace - 14:22:22.431 PM] Received response 'textDocument/documentLink - (62)' in 3ms.
Result: [{"range":{"start":{"line":4,"character":2},"end":{"line":4,"character":5}},"target":"https://pkg.go.dev/fmt?utm_source=gopls"}]

@stamblerre
Copy link
Contributor

Thanks for sharing this log. Looks like the textDocument/formatting request is still present in the log, so I would guess that this could be an issue with your VS Code settings or with VS Code itself. Can you file an issue with the https://github.com/microsoft/vscode repository instead?

@jpap
Copy link
Author

jpap commented Sep 28, 2021

I'm happy to file an issue against vscode, though it would be good to rule out vscode-go first. Are you able to reproduce, with and without gopls enabled? (The bug is present with both here.)

@stamblerre
Copy link
Contributor

Yeah, I actually did repro. I think it's because the VS Code Go extension defaults to formatOnSave being enabled:

"editor.formatOnSave": true,
, so probably VS Code doesn't correctly override the setting. Please file an issue with VS Code.

@jpap
Copy link
Author

jpap commented Sep 30, 2021

Thanks for that line reference; with it I was able to find #65844 and #58995 which refer to the same problem for a variety of VSC-supported languages and associated settings.

The "official" method to control the format-on-save, or any of the other settings under configurationDefaults is to override them manually in the user's json config. 😲

Given that #58995 is almost three years old and it may not get addressed any time soon, is there any way to flip this issue into a documentation change, or a hack in the vscode-go settings panel to educate the user so they don't end up following the same path of confusion as I did?

@hyangah
Copy link
Contributor

hyangah commented Oct 4, 2021

My understanding is that the language-specific configuration overrides non-language-specific configuration setting (https://code.visualstudio.com/api/references/vscode-api#WorkspaceConfiguration) so this is working as expected from VSCode's point of view even though surfacing these to users (microsoft/vscode#58995) definitely needs improvement.

This is not the first time this language-specific default override caused confusion.
(#1231 #1805 and many in the old repo)

Some users request basically not to set language-specific defaults but stay away from configurationDefaults. However, given that most users go with the default setting, recommending the best configuration and encouraging the best practice for 'go' development is also a role of the extension.

If we want to turn this into a documentation change, where is the best place to surface this info? The editor.formatOnSave setting is owned by vscode so I don't think documentation surfaced through setting UI is an option. We had related doc in https://github.com/golang/vscode-go/blob/master/docs/advanced.md#formatting-code-and-organizing-imports but it doesn't seem easy to discover.

Popup to suggest the best practice setting instead of configurationDefaults is an option too but I think for Cloud-based IDEs (e.g.codespaces), popups are not considered a great way to set up environments for the best practice.

Any idea to improve documentation is welcome.

@hyangah hyangah added NeedsInvestigation Someone must examine and confirm this is a valid issue and not a duplicate of an existing one. and removed WaitingForInfo Issue is not actionable because of missing required information, which needs to be provided. labels Oct 4, 2021
@mukunda-
Copy link

"source.organizeImports" has some weird behavior if there are no imports (formatting the whole file). Would this be the root cause or similar? Here's a short video.

@hyangah
Copy link
Contributor

hyangah commented Jan 14, 2022

@mukunda- Thanks for the report. It sounds like that is a different issue because you have all the per-language settings override. Can you please file a separate issue with a smaller repro case and the gopls trace (instruction)? Thanks!

hyangah added a commit to hyangah/vscode-go that referenced this issue Jun 13, 2022
And add back the title that was dropped by accident.

Updates golang#2268
Updates golang#1815
Updates golang#1805

Change-Id: Iea464b1e54f0eb5d421d28c7bea40a86e49dc517
hyangah added a commit to hyangah/vscode-go that referenced this issue Jun 14, 2022
And add back the title that was dropped by accident.

Updates golang#2268
Updates golang#1815
Updates golang#1805

Change-Id: Iea464b1e54f0eb5d421d28c7bea40a86e49dc517
@gopherbot
Copy link
Collaborator

Change https://go.dev/cl/412054 mentions this issue: docs/faq.md: entries on formatting issues

gopherbot pushed a commit that referenced this issue Jun 14, 2022
And add back the title that was dropped by accident.

Updates #2268
Updates #1815
Updates #1805

Change-Id: Iea464b1e54f0eb5d421d28c7bea40a86e49dc517
Reviewed-on: https://go-review.googlesource.com/c/vscode-go/+/412054
Reviewed-by: Jamal Carvalho <jamal@golang.org>
@gopherbot
Copy link
Collaborator

Change https://go.dev/cl/412383 mentions this issue: docs/faq.md: entries on formatting issues

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
NeedsInvestigation Someone must examine and confirm this is a valid issue and not a duplicate of an existing one.
Projects
None yet
Development

No branches or pull requests

5 participants