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

x/tools/cmd/goimports: add -s flag #8759

Closed
cskr opened this issue Sep 17, 2014 · 10 comments
Closed

x/tools/cmd/goimports: add -s flag #8759

cskr opened this issue Sep 17, 2014 · 10 comments

Comments

@cskr
Copy link

@cskr cskr commented Sep 17, 2014

goimports claims to be a drop-in replacement for editor's gofmt-on-save hook and have
the same command-line interface as gofmt; but it does not support the -s flag.

-s flag can be useful in on-save hook unlike gofmt's -r flag.
@ianlancetaylor
Copy link
Contributor

@ianlancetaylor ianlancetaylor commented Sep 17, 2014

Comment 1:

Labels changed: added repo-tools, release-none.

@bradfitz
Copy link
Contributor

@bradfitz bradfitz commented Sep 17, 2014

Comment 2:

It also doesn't support -r.
I think this is a documentation bug. We can say it's "almost a drop-in replacement".
@cskr
Copy link
Author

@cskr cskr commented Sep 17, 2014

Comment 3:

In which case, can we treat this as a feature request for -s? 
While -r is not very useful in an on-save hook, it'll be nice to have goimports as the
on-save command and include the -s flag.
@bradfitz
Copy link
Contributor

@bradfitz bradfitz commented Sep 17, 2014

Comment 4:

I ripped it out on purpose, to not duplicate code.
But if Robert is okay with moving it to a package in go.tools so simplify can be shared
by both gofmt and goimports, then I'm okay with it.
@griesemer
Copy link
Contributor

@griesemer griesemer commented Sep 17, 2014

Comment 5:

I am not against it but gofmt (or go fmt) is not depending on anything but the std lib
at the moment.
Note that we are also considering removing that -r option from gofmt; so it may make
sense to remove the -s option as well and have a separate tool instead.
Not for 1.4 for sure.

Status changed to Thinking.

stapelberg added a commit to robustirc/robustirc that referenced this issue Mar 24, 2015
Let’s see if we can keep this up for long.

golang/go#8759 tracks adding -s to goimports,
so that we could use it in our editor configs.
@josharian josharian changed the title go.tools/cmd/goimports: Does not support -s flag goimports: add -s flag Apr 3, 2015
@rsc rsc added this to the Unplanned milestone Apr 10, 2015
@rsc rsc removed the release-none label Apr 10, 2015
@rsc rsc changed the title goimports: add -s flag x/tools/cmd/goimports: add -s flag Apr 14, 2015
@rsc rsc modified the milestones: Unreleased, Unplanned Apr 14, 2015
@rsc rsc removed the repo-tools label Apr 14, 2015
@stevekuznetsov
Copy link

@stevekuznetsov stevekuznetsov commented Aug 6, 2015

This is causing some issues on my end - is this ever going to be considered? Or, at the least, the documentation needs to be updated.

@jzelinskie
Copy link
Contributor

@jzelinskie jzelinskie commented Feb 25, 2016

I forked the tools repository and added this behavior if anyone is interested: go install github.com/jzelinskie/tools/cmd/goimports

I just copied the code from gofmt and removed the import comments: https://github.com/jzelinskie/tools/commit/348c54dbd0dc54b93cd91d71cfd483d55d00f971

If you're using vim-go you can add the following to your .vimrc to get both gofmt -s and goimports on save.

let g:go_fmt_command = "goimports"
let g:go_fmt_options = "-s"
@bradfitz
Copy link
Contributor

@bradfitz bradfitz commented Feb 25, 2016

Related: #14500

@akavel
Copy link
Contributor

@akavel akavel commented Jul 10, 2017

(Note FTR: per golang/tools@7d47e84, goimports is no longer claimed to be "a drop-in replacement [for gofmt]".)

@bradfitz
Copy link
Contributor

@bradfitz bradfitz commented Jul 10, 2017

@akavel, thanks.

I'll close this then.

@bradfitz bradfitz closed this Jul 10, 2017
@golang golang locked and limited conversation to collaborators Jul 10, 2018
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Linked pull requests

Successfully merging a pull request may close this issue.

None yet
9 participants
You can’t perform that action at this time.