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

Support for gofumports #354

Open
theckman opened this issue Apr 18, 2020 · 3 comments
Open

Support for gofumports #354

theckman opened this issue Apr 18, 2020 · 3 comments

Comments

@theckman
Copy link

theckman commented Apr 18, 2020

Hi there,

I wanted to see whether you'd be interested in having support for gofumports. It's a goimports version of gofumpt, which means it also needs the -srcdir command line argument passed to it (much like goimports).

The issue is that if the command is anything but goimports, go-mode treats it like gofmt. I have tested that it works by symlinking goimports to gofumports, but it feels dirty.

The simplest solution seems to be to turn this to an or statement and have it also look for gofumports here:

go-mode.el/go-mode.el

Lines 1877 to 1878 in 10d6ab4

(defun gofmt--is-goimports-p ()
(string-equal (file-name-base gofmt-command) "goimports"))

I was thinking something like this, but I'm an elisp noob and haven't tested it:

(defun gofmt--is-goimports-p ()
  (let ((cmdbase (file-name-base gofmt-command)))
        (or (string-equal cmdbase "goimports")
            (string-equal cmdbase "gofumports"))))

What do you think? I can raise a PR If that seems okay.

@psanford
Copy link
Collaborator

It makes sense to be able to swap in other commands that are the same "shape" as goimports.

My only suggestion is that we should make the list of commands that take a -srcdir flag a customizable variable so if you've got a really obscure fork of goimports you can just add it to that list instead of making a change to the (gofmt--is-goimports-p) function.

Something like:

(defcustom gofmt-srcdir-commands '("goimports" "gofumports")
  "Gofmt-like commands that require the -srcdir flag"
  :type '(repeat string)
  :group 'go)

(defun gofmt--is-goimports-p ()
  (let ((cmdbase (file-name-base gofmt-command)))
    (member cmdbase gofmt-srcdir-commands)))

@theckman
Copy link
Author

Sorry for the delay, dealing with other workstation issues. 😛

Definitely! I can take a look at doing that and raising a PR after testing that it works as expected.

@dominikh
Copy link
Owner

gofumports is no longer a thing. Upstream now suggests to either let gopls handle the imports, or to run goimports followed by gofumpt (https://github.com/mvdan/gofumpt#frequently-asked-questions).

The former model doesn't require any changes by us, the latter would require a way to run both goimports and gofumpt and pass different flags to them.

I predict that most people will be using gopls in the future, so I'm not sure making this work is all too important in the long-term.

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

No branches or pull requests

3 participants