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

go.tools/imports: Case where goimports output isn't gofmt-compatible on first try. #8035

Closed
dmitshur opened this issue May 20, 2014 · 5 comments
Labels
FrozenDueToAge Suggested Issues that may be good for new contributors looking for work to do.

Comments

@dmitshur
Copy link
Contributor

This is yet another case where goimports/imports.Process() produces non gofmt-compatible
output on first try (it becomes okay after goimports runs a second time, so this bug is
not as critical as ones where gofmt/goimports undo each other's changes).

http://play.golang.org/p/yr18m3KyVw

Press Format (with goimports) once.

Note that the comments after "fmt" and "go/ast" are not properly
indented.

This happens because Go source code is manipulated via text to insert newlines (which
happens as a last step, after gofmt-compatible output has already been printed). The 3rd
party Go package becomes a separate import group away from the standard "fmt"
and "go/ast", but the comments are not spaced properly.

Trying to get gofmt-compatible output by manipulating text is an endless source of bugs
similar to this, a category of bugs that would simply not exist if we had the ability to
manipulate(/generate) ASTs in code in a way that's easy and correct. That is something
I'll probably try to work on next; I think it will be very valuable in the long run and
it's a prerequisite for creation of new amazing tools that will make our lives better.
@rsc
Copy link
Contributor

rsc commented May 21, 2014

Comment 1:

Labels changed: added release-none.

@bradfitz
Copy link
Contributor

Comment 2:

shurcooL, were you going to work on this?

Labels changed: added suggested, repo-tools.

@dmitshur
Copy link
Contributor Author

Comment 3:

I don't have immediate plans to solve this one in the short term, only long term plans,
but it will be a while before I try to solve higher priority items first (like issue
5551).

@josharian
Copy link
Contributor

Comment 4:

Is https://golang.org/cl/120840044 too big a hammer for this?

Status changed to Accepted.

@josharian
Copy link
Contributor

Comment 5:

This issue was closed by revision golang/tools@3442faf.

Status changed to Fixed.

@dmitshur dmitshur added fixed Suggested Issues that may be good for new contributors looking for work to do. labels Jul 29, 2014
@golang golang locked and limited conversation to collaborators Jun 25, 2016
This issue was closed.
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
FrozenDueToAge Suggested Issues that may be good for new contributors looking for work to do.
Projects
None yet
Development

No branches or pull requests

5 participants