Skip to content

Please, more reasonable defaults #3730

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

Open
mythemeria opened this issue Mar 24, 2025 · 6 comments
Open

Please, more reasonable defaults #3730

mythemeria opened this issue Mar 24, 2025 · 6 comments
Labels
NeedsTriage issues to review in the team triage meeting

Comments

@mythemeria
Copy link

Is your feature request related to a problem? Please describe.
Yes. I keep finding that it edits my file before I save. Normally this is just mildly annoying but if there's any bugs in the linter then I can't save and re-open vscode to fix the issue. I frequently have issues (not just in go) where I try to install a new package but I still get errors saying the package is missing. Simply restarting vscode almost always fixes this. Not in go, because I can't save it without it removing the "missing" packages. I have spent the last 20 minutes trying to work out where to disable this without accidentally messing up all the settings but it's not clear at all. The descriptions don't give me enough information to work out which setting does what I want as it doesn't clearly tell me which ones are me saying "yes, I want you to edit my code when I save".

Describe the solution you'd like
Change the default behaviour so that it doesn't impede basic problem solving like restarting the program, or at least make it clearer how I can turn this off.

Describe alternatives you've considered
I guess I will be spending reading all the docs to work out how to fix this, but it is still incredibly annoying that it will remove my code that is there for a reason. It's a bit unreasonable to expect people to only save when the code is perfect.

@gopherbot gopherbot added this to the Untriaged milestone Mar 24, 2025
@mythemeria
Copy link
Author

I have disabled everything that says it modifies the file on save but it still does it.

@h9jiang
Copy link
Member

h9jiang commented Mar 24, 2025

Hi mythemeria,

Thank you for reporting this issue. The go extension have some default behavior of formatting on save. Please see the FAQ here. If you follow instructions mentioned in this doc, you should be able to disable the format on save behavior (looks like the false is deprecated over "never", I will update the doc to reflect this change.)

I'm not aware of any other default behavior other than this (probably because that is the only thing I have noticed).

There is a problem reported earlier regarding this defaultConfiguration behavior. Like microsoft/vscode-go#3059 #1815, microsoft/vscode#58995. They are not visible or configurable from the Preferences: Open User Settings but they are configurable in Preferences: Open User Settings(JSON) (the default values is not visible).

I'm not sure what kind of text edits are you observing, if this is still happening, we might need to work on a reproduce and try to resolve your issue. (Like share the step by step instructions so I can reproduce it from our end, and dive in detail of the LSP communication).

Sorry for the confusion.

@h9jiang h9jiang added the WaitingForInfo Issue is not actionable because of missing required information, which needs to be provided. label Mar 24, 2025
@mythemeria
Copy link
Author

Here is an example of the code changes. I have this snippet taken from an example I found online:

package db

import (
	"database/sql"
        "github.com/lib/pq"
	"github.com/golang-migrate/migrate/v4"
	"github.com/golang-migrate/migrate/v4/database/postgres"
        "github.com/golang-migrate/migrate/v4/source/file"
)

func Migrate() {
	db, err := sql.Open("postgres", "postgres://localhost:5432/database?sslmode=enable")
	driver, err := postgres.WithInstance(db, &postgres.Config{})
	m, err := migrate.NewWithDatabaseInstance(
		"file:///migrations",
		"postgres", driver)
	m.Up() // or m.Steps(2) if you want to explicitly set the number of migrations to run
}

When I save, it changes to this:

package db

import (
	"database/sql"

	"github.com/golang-migrate/migrate/v4"
	"github.com/golang-migrate/migrate/v4/database/postgres"
)

func Migrate() {
	db, err := sql.Open("postgres", "postgres://localhost:5432/database?sslmode=enable")
	driver, err := postgres.WithInstance(db, &postgres.Config{})
	m, err := migrate.NewWithDatabaseInstance(
		"file:///migrations",
		"postgres", driver)
	m.Up() // or m.Steps(2) if you want to explicitly set the number of migrations to run
}

These were the same packages that were being removed before but now they're being removed because the packages are unused, not because I am missing them. I disabled the extension so I could save and restart vscode, which fixed the missing package error as expected.

But anyway, adding this to my json settings fixed it, thanks. I had already modified the formatOnSave part.

"editor.codeActionsOnSave": {
    "source.organizeImports": "never"
}

Keeping this issue open because I still think this can be made way less annoying.

@firelizzard18 firelizzard18 removed the WaitingForInfo Issue is not actionable because of missing required information, which needs to be provided. label Mar 25, 2025
@firelizzard18
Copy link
Contributor

Keeping this issue open because I still think this can be made way less annoying.

@mythemeria What specific change are you asking for? Changing the default value of editor.formatOnSave (for Go files)? Changing the default value of source.organizeImports? Changing how the formatter formats your code?

Simply restarting vscode almost always fixes this.

I suggest trying Go: Restart Language Server first. For me that solves the majority of issues like this.

These were the same packages that were being removed before but now they're being removed because the packages are unused, not because I am missing them. I disabled the extension so I could save and restart vscode, which fixed the missing package error as expected.

What do you expect to happen? Do you not want unused imports to automatically be removed? Or is your issue specifically with packages that have not yet been added to go.mod? When I test your example (the second version), the packages are not removed even though I don't have them installed. Of course, if those packages aren't referenced then the organize imports operation will remove them whether or not they're installed, but they should stay if they're referenced.

Image

Side note, if you use the code action to install missing packages, gopls will recognize that they've been installed without needing to restart it (or vscode).

Image

@firelizzard18 firelizzard18 added the WaitingForInfo Issue is not actionable because of missing required information, which needs to be provided. label Mar 25, 2025
@mythemeria
Copy link
Author

When I test your example (the second version), the packages are not removed even though I don't have them installed. Of course, if those packages aren't referenced then the organize imports operation will remove them whether or not they're installed, but they should stay if they're referenced.

It only gave me an error saying that the package was not installed, which I fixed with the quick action like you mention. No mention of the unused package error until I disabled the extension and restarted, so I assumed that the missing package was the reason it was removed.

What specific change are you asking for? Changing the default value of editor.formatOnSave (for Go files)? Changing the default value of source.organizeImports? Changing how the formatter formats your code?

I am not sure what the best solution would be to appease everyone since I haven't exactly gone and asked other users what they expect it to do. I personally expect the save button to actually save my work, not change it and then save. Perhaps match the vscode settings by default? For sure it should be made easier to find how to disable this behaviour.

I suggest trying Go: Restart Language Server first. For me that solves the majority of issues like this.

Neat, I will use that in future

What do you expect to happen? Do you not want unused imports to automatically be removed?

I expect the save button the function the way every other save button works

@firelizzard18
Copy link
Contributor

No mention of the unused package error until I disabled the extension and restarted, so I assumed that the missing package was the reason it was removed.

An unused import is always an error in Go, that's part of the language itself and the import organizer is designed partially to make it easier to comply with that. If an import is referenced and the organizer removes it, that is a bug. Whether the package is missing should not have any effect on that.

I personally expect the save button to actually save my work, not change it and then save.

It sounds like you want to disable any on-save functionality (such as format-on-save). It is confusing that you have to disable both editor.formatOnSave and editor.codeActionsOnSave/source.organizeImports, and vscode's logic for how different setting scopes override each other is confusing, but those are best solved by improvements to how vscode presents settings. Maybe there should be a global "don't do anything on safe" option, but that's also a change best made to vscode. Granted, you could argue that vscode-go should choose different defaults for those settings but that's a matter of opinion and I expect the majority of users (including me) are happy with those defaults.

@firelizzard18 firelizzard18 added NeedsTriage issues to review in the team triage meeting and removed WaitingForInfo Issue is not actionable because of missing required information, which needs to be provided. labels Mar 26, 2025
@h9jiang h9jiang modified the milestones: Untriaged, vscode-go/backlog Apr 23, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
NeedsTriage issues to review in the team triage meeting
Projects
None yet
Development

No branches or pull requests

4 participants