Goimport removes wrong import

[azr]

Hello there,

in this file, the import line "code.google.com/p/biogo.matrix" gets removed by goimports using go-plus.

If I edit it with vim with wrong spacing I get :

$ goimports -d radon.go
diff radon.go gofmt/radon.go
--- /var/folders/sq/zw92_59n6q9fvh0bw99jkfnw0000gn/T/gofmt836110842 2014-11-10 23:54:39.000000000 +0100
+++ /var/folders/sq/zw92_59n6q9fvh0bw99jkfnw0000gn/T/gofmt764491537 2014-11-10 23:54:39.000000000 +0100
@@ -7,7 +7,7 @@
    "log"
    "math"

-    "code.google.com/p/biogo.matrix"
+   "code.google.com/p/biogo.matrix"
    "code.google.com/p/graphics-go/graphics"
    "github.com/azr/phash/manipulator"
    "github.com/nfnt/resize"

If I do

import (
matrix "code.google.com/p/biogo.matrix"

it's not removed.

Cheers !

Edit: It's not removed if I use gofmt as a format tool.

[joefitzgerald]

Another one for goimports. Unfortunately this is likely a goimports issue, not a go-plus issue. See also #52, #69, and #87. /cc @bradfitz

[joefitzgerald]

@azr Just to rule out any go-plus factor, can you please paste the output from Packages > Go Plus > Display Go Information? It should look something like:

Using Go: go1.3.3 darwin/amd64 (@/usr/local/bin/go)
GOPATH: /Users/jfitzgerald/go
Cover Tool: /usr/local/Cellar/go/1.3.3/libexec/pkg/tool/darwin_amd64/cover
Vet Tool: /usr/local/Cellar/go/1.3.3/libexec/pkg/tool/darwin_amd64/vet
Format Tool: /Users/jfitzgerald/go/bin/goimports
Lint Tool: /Users/jfitzgerald/go/bin/golint
Git: /usr/bin/git
Mercurial: /usr/local/Cellar/mercurial/3.1.2/bin/hg
PATH: /Users/jfitzgerald/go/bin:/usr/local/bin:/Users/jfitzgerald/.rbenv/shims:/usr/local/bin:/usr/local/sbin:/Users/jfitzgerald/go/bin:/usr/bin:/bin:/usr/sbin:/sbin:/usr/local/MacGPG2/bin:/usr/texbin
Atom: 0.144.0 (darwin x64 14.0.0)

[joefitzgerald]

FYI, there was an issue with Atom < 0.142.0 (introduced by Yosemite) that causes the PATH to not get passed to atom when you launch it from the command line. The workaround was released in 0.142.0. So if you're running Yosemite and an older version of Atom, this could be impacting you. See #52 and electron/electron#550.

[azr]

Hello there,

I'm on Yosemite, but I never launch atom command line.
I also try to keep atom up to date.

Here are my go-plus infos :

Go: go1.3.3 darwin/amd64 (@/usr/local/bin/go)
GOPATH: /Users/azer/go
Cover Tool: /usr/local/Cellar/go/1.3.3/libexec/pkg/tool/darwin_amd64/cover
Vet Tool: /usr/local/Cellar/go/1.3.3/libexec/pkg/tool/darwin_amd64/vet
Format Tool: /Users/azer/go/bin/goimports
Lint Tool: /Users/azer/go/bin/golint
Git: /usr/bin/git
Mercurial: /usr/local/Cellar/mercurial/3.2/bin/hg
PATH: /usr/local/bin:/usr/bin:/bin:/usr/sbin:/sbin:/opt/X11/bin
Atom: 0.144.0 (darwin x64 14.0.0)

[bradfitz]

I filed https://code.google.com/p/go/issues/detail?id=9087 for somebody to investigate.

[joefitzgerald]

@azr Thanks - can you launch Atom from the command line to determine if there is any change in behavior?

[azr]

@joefitzgerald, no pbm, same happens if I launch it from cmd.
@bradfitz cool, thanks !

[cespare]

Is the library installed? If it's not, goimports has no way of knowing that code.google.com/p/biogo.matrix has a package name of matrix.

[cespare]

In any case, I cannot repro this issue using your test file (after installing the biogo.matrix library) and goimports directly from the command-line, or through vim-go.

[azr]

Hey @cespare I dont know if I said it right, but from the command line if you look at the diff up there, the import is not removed, the spacing is just fixed.
I think the problem is just in atom.

[joefitzgerald]

@azr, I'm not sure I agree with your assertion, but I'm willing to suspend disbelief for a moment to further investigate.

Do you have time to do a screen sharing session so I can debug this with you interactively? If so, please follow @joefitzgerald on Twitter so that we can direct message.

[azr]

Okay, I don't really know how to direct message with twitter.
Never was really interested into it.

But here is a gif ! I just learned to do that, funny.

Jokes appart, I'm following you right now :)

[joefitzgerald]

Sent you those DM's - they're a little hidden in Twitter's UX; you'll have to click on the 'Messages' button.

[joefitzgerald]

I got an email from "Chance Zibolski" with a comment that is no longer visible here - perhaps it was deleted?

"I'm experiencing the same issue. If I use sublime or vim with goimports, it's fine. But if I use atom with goimports, it is removing a needed import.

The import is github.com/mattbaird/elastigo/lib, which has a package name of elastigo, even though the package path ends in lib. If I change the path to the non existent github.com/mattbaird/elastigo/elastigo it stops removing it, but obviously that doesn't work for compiling."

[chancez]

Turns out I was testing on the wrong file, and it was working fine actually.

[joefitzgerald]

@ecnahc515 I wonder if there's a nugget of wisdom in your findings though... Can you describe more about the scenario (the "wrong file") and why it was wrong?

[joefitzgerald]

After a screensharing session with @azr, I can confirm the issue is with go-plus. When a user launches Atom from Finder / Dock / Spotlight, the environment comes from launchd, and so most people do not have things like GOPATH set.

go-plus has logic to ensure the environment is setup correctly even in this scenario (using the GOPATH preference), but this doesn't appear to be making it to the environment currently. I'll have a fix for this soon.

[azr]

Glad I could help :)

[joefitzgerald]

@azr can you please update go-plus and let me know if the issue is fixed on your end?

[azr]

Hey @joefitzgerald the lib is no longer removed ! Yay.

It still leaves a gap between the imports :

import (
    "errors"
    "image"
    "image/color"
    "math"

    "code.google.com/p/biogo.matrix"
    "code.google.com/p/graphics-go/graphics"
    "github.com/azr/phash/floats"
    "github.com/azr/phash/manipulator"
    "github.com/nfnt/resize"
)

But it seems it's done on purpose ? To separate official libs from others ?

[joefitzgerald]

Yes, that's done on purpose by goimports. Glad to hear this has fixed the issue; thanks for helping me track down the root cause.

[azr]

Ok, cool. No problem :)