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

Allow gofumports as goimports alternative #2993

Closed
powerman opened this issue Sep 1, 2020 · 14 comments · Fixed by #2994
Closed

Allow gofumports as goimports alternative #2993

powerman opened this issue Sep 1, 2020 · 14 comments · Fixed by #2994

Comments

@powerman
Copy link
Contributor

powerman commented Sep 1, 2020

https://github.com/mvdan/gofumpt was added to golangci-lint, and it looks reasonable.
Please allow to set let g:go_fmt_command = 'gofumports'.
Also it may be a good idea to include it into list of installed/updated binaries.

@bhcleek
Copy link
Collaborator

bhcleek commented Sep 1, 2020

Have you tried setting g:go_fmt_command to gofumpt? I think it might just work, but I don't intend to add a fourth option for formatting.

@powerman
Copy link
Contributor Author

powerman commented Sep 1, 2020

No, because I do need goimports functionality in addition to gofmt/gofumpt.
I've tried to set it to gofumports (just as shown in first comment) and noticed it doesn't work. Checked the docs, and noticed this:

Valid options are gofmt, goimports, and gopls.

So it looks like other values are not supported and does not work.

@stamblerre
Copy link
Contributor

stamblerre commented Sep 1, 2020

gopls currently supports gofumpt as an opt-in setting (gofumpt: true in gopls settings).

@bhcleek
Copy link
Collaborator

bhcleek commented Sep 1, 2020

Ah, thanks, @stamblerre ! I'll add proper support for that.

@powerman
Copy link
Contributor Author

powerman commented Sep 1, 2020

@stamblerre Cool! Is gopls can use gofumports too?

@stamblerre
Copy link
Contributor

gofumports is just a wrapper that runs both goimports and gofumpt, and gopls already runs import organization separately, so you will still get the behavior of goimports and gofumpt when you enable gofumpt in gopls.

@bhcleek
Copy link
Collaborator

bhcleek commented Sep 1, 2020

I've tried to set it to gofumports (just as shown in first comment) and noticed it doesn't work

What happens when you try? Although setting g:go_fmt_command to gofumpt isn't officially supported, I believe it will work and testing on my local machine confirms that it does.

There are now separate options to configure goimports to be called on save, too. See :help g:go_imports_autosave and :help g:go_imports_mode.

@powerman
Copy link
Contributor Author

powerman commented Sep 4, 2020

What happens when you try? Although setting g:go_fmt_command to gofumpt isn't officially supported, I believe it will work and testing on my local machine confirms that it does.

Well, actually it works, thanks! Even setting it to gofumports works as expected. Not sure why it didn't work when I tried for the first time, before opening the issue, sorry.

I've also tried to use gopls to get goimports functionality, and while it works it reformat import () section is unusual way (without surrounding empty lines), e.g.:

// before:
package ...

type ...

// after:
package ...
import (
	...
)
type ...

(@stamblerre Should I report this as separate issue for gopls?)

@stamblerre
Copy link
Contributor

(@stamblerre Should I report this as separate issue for gopls?)

I haven't heard reports of this issue before, but you can easily check if it's caused by gopls (without gofumpt) by running gopls imports /path/to/file.go on the command line. If it is indeed gopls, please file a new issue here with an example of the code that reproduces the problem.

@powerman
Copy link
Contributor Author

powerman commented Sep 4, 2020

At a glance it looks like there is both an issue with vim-go and gopls (a different ones).

Relevant part of vim-go setup:

let g:go_fmt_command = 'gopls'
let g:go_imports_autosave = 1
let g:go_imports_mode = 'gopls'
$ gopls version
golang.org/x/tools/gopls 0.4.4
    golang.org/x/tools/gopls@v0.4.4 h1:8djGYsaZ0ByP0vaXg4T+mnyfDcHpWKSZ+tpQSGv9ahk=
$ cat fmt.go 
package main
func F(*testing.T, time.Time) {}
$ gopls imports fmt.go 
package main

import (
	"testing"
	"time"
)
func F(*testing.T, time.Time) {}
$ goimports fmt.go 
package main

import (
	"testing"
	"time"
)

func F(*testing.T, time.Time) {}
$ gofumports fmt.go 
package main

import (
	"testing"
	"time"
)

func F(*testing.T, time.Time) {}
$ vi fmt.go # :wq
$ cat fmt.go 
package main
import (
	"testing"
	"time"
)
func F(*testing.T, time.Time) {}

@powerman
Copy link
Contributor Author

powerman commented Sep 4, 2020

@bhcleek Sorry for moving discussion from one issue to another here. Should I close this one and open another about described above gopls imports issue or just change title of this one?

@bhcleek
Copy link
Collaborator

bhcleek commented Sep 4, 2020

This isn't the place to talk about gopls bugs generally.

I'm leaving this open until #2994 is merged.

@powerman
Copy link
Contributor Author

powerman commented Sep 4, 2020

@bhcleek I was talking not about gopls issue (I've already opened golang/go#41225), but about vim-go behaviour for using gopls as fmt/imports, which produces different output to gopls imports - see above.

@bhcleek
Copy link
Collaborator

bhcleek commented Sep 4, 2020

Feel free to open a separate issue about using gopls for adjusting imports when let g:go_imports_mode='gopls' if you want; that's a separate issue from your original description, and it's much easier to keep track of things when GitHub issues are focused on a single topic.

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 a pull request may close this issue.

3 participants