goimports has been updated

[Brad Fitzpatrick]

goimports has been updated.

If you've been frustrated by its speed lately, run:

   $ go get -u golang.org/x/tools/cmd/goimports

... and things should be much nicer.

Details at https://golang.org/cl/24941

If I broke something, file a bug: https://golang.org/issues/new

The general speed tracking bug is https://golang.org/issue/16367 (don't use for new bugs, only for speed)

[Tyler Compton]

As helpful a tool as goimports is, I've really felt it slogging as of late, as well. It's great to hear that this has been given attention!

[Sathish VJ]

hey brad, is it possible to have a ```goimports version``` so that we know what versions we currently have?

[Ian Davis]

 

On Fri, Jul 15, 2016, at 06:34 AM, Brad Fitzpatrick wrote:

goimports has been updated.

 

If you've been frustrated by its speed lately, run:

 

   $ go get -u golang.org/x/tools/cmd/goimports

 

... and things should be much nicer.

 

Details at https://golang.org/cl/24941

 

Some really neat changes in there. Out of interest does it prioritise packages that are already imported by other files in the package you are working on? Seems like that might be a useful heuristic to help speed up searching for packages.

 

-- Ian

[Darren Hoo]

goimports won't remove empty lines. is that a feature?

goimports on this:

import (

"bufio"

"fmt"

"math"

"strconv"

"time"

"math/rand"

"os"

)

will not become

import (

"bufio"

"fmt"

"math"

"math/rand"

"os"

"strconv"

"time"

)

[Matthew Singletary]

Thanks Brad!

[Damian Gryski]

Leaving blank lines in the imports section allows grouping: https://github.com/golang/go/wiki/CodeReviewComments#imports

Damian

[Peter Waller]

Awesome!

For me it brings the CPU time from 6.5s to 3s. Wall from 2.9s to 2s. A noticable improvement.

One thing that still makes it slow for me (2s instead of 500ms, I just tested), is that I have several of deep trees which aren't go code in my $GOPATH/src. To name a few: linux, clang, llvm.

I feel silly for asking, but any chance of having a mechanism to ignore these non-go trees, or does anyone have any other clever ideas how to avoid this?

I'd prefer not to store other directories separately just because they aren't go code.

[Zlatko Čalušić]

I'd like to second this!

Also have the linux tree in $GOPATH, and ever since I put it there goimports slowed down. Some kind of subtree exclusion mechanism would be great.

Nevertheless, thank you for your work on probably the most useful Go tool out there, Brad! I can't imagine programming in Go without goimports anymore, it's that addictive. ;)

Best regards,

--
You received this message because you are subscribed to the Google Groups "golang-nuts" group.
To unsubscribe from this group and stop receiving emails from it, send an email to golang-nuts...@googlegroups.com.
For more options, visit https://groups.google.com/d/optout.

-- 
Zlatko

[Thomas Frössman]

Yeah. I also have (several) linux trees, gcc's and stuff in my GOPATH.. I would really like some global config way to tell goimports/gorename/... to ignore a bunch of subtrees.

[Thomas Frössman]

[Jan Mercl]

On Fri, Jul 15, 2016 at 4:29 PM Thomas Frössman <tho...@jossystem.se> wrote:

> Yeah. I also have (several) linux trees, gcc's and stuff in my GOPATH.. I would really like some global config way to tell goimports/gorename/... to ignore a bunch of subtrees.

I do not use goimports. My 2c anyway: What about checking for .goimportsignore in the root of the subtree that should be skipped?

--

-j

[Peter Waller]

On 15 July 2016 at 15:31, Jan Mercl <0xj...@gmail.com> wrote:

I do not use goimports. My 2c anyway: What about checking for .goimportsignore in the root of the subtree that should be skipped?

+1. I was going to suggest this but before seeing support for the idea thought it sounded too unsavoury to suggest :) 

Another suggestion which might be even nicer for the implementation: if a directory contains `.goimportsignore`, that directory is skipped.

It's less flexible, but... any horrible behaviour where the ignore file has a high CPU cost can be avoided (see also: .dockerignore causing docker build to slow down drastically for large trees).

[Zlatko Čalušić]

As we're speaking of Git trees here, I wouldn't like poluting them with extra files. Git is very picky about that, for a good reason.

Also, it could be another performance hit, because now goimports tool would need to check the existence of .goimportsignore file in each and every folder (not unlike Apache's .htaccess).

I think I'd rather see a single ~/.config/goimports/goimports.ini config file, where we could have a section along these lines:

[Ignore]
git.kernel.org/torvalds/linux
...

The config file might become useful later for other possible customizations.

Regards,

-- 
Zlatko

[Sam Whited]

On Fri, Jul 15, 2016 at 9:49 AM, Peter Waller <pe...@pdftables.com> wrote:
> Another suggestion which might be even nicer for the implementation: if a
> directory contains `.goimportsignore`, that directory is skipped.

That would have to be in addition to a file that lists directories,
otherwise you can't skip submodules and subtree merges and the like
(external code where you don't have the ability to add a
.goimportsignore file).

—Sam



--
Sam Whited
pub 4096R/54083AE104EA7AD3

[Peter Waller]

On 15 July 2016 at 16:35, Sam Whited <s...@samwhited.com> wrote:

On Fri, Jul 15, 2016 at 9:49 AM, Peter Waller <pe...@pdftables.com> wrote:
> Another suggestion which might be even nicer for the implementation: if a
> directory contains `.goimportsignore`, that directory is skipped.

That would have to be in addition to a file that lists directories,
otherwise you can't skip submodules and subtree merges and the like
(external code where you don't have the ability to add a
.goimportsignore file).

Ah yes, of course. I retract my suggestion. 

[Thomas Frössman]

Environment variables like (maybe) GOIMPORTSIGNORE/GORENAMEIGNORE fits better with how go tools are currently configured than config files.

--

Thomas Frössman

http://t.jossystem.se

[Nathan Youngman]

Thanks Brad.

I also use $GOPATH ($HOME/src) for code non-Go projects.

I'd be in favour of a more automated solution to ignoring folders that don't contain any *.go files (until such time as they start using *.go).

Nathan.

[Brad Fitzpatrick]

On Fri, Jul 15, 2016 at 9:19 AM, Nathan Youngman <gop...@nathany.com> wrote:

Thanks Brad.

I also use $GOPATH ($HOME/src) for code non-Go projects.

I'd be in favour of a more automated solution to ignoring folders that don't contain any *.go files (until such time as they start using *.go).

It already does ignore folders without *.go files.

The slow part for people with gigantic GOPATHs is traversing down to the directory to discover it has no Go files.

And without a daemon, you can't cut off a subtree higher "until such time as they start using *.go" because you don't know when that is.

So currently goimports does (when first needed), the equivalent of a find $GOPATH/src -name '*.go' but skipping dirs starting with ".", "_", etc (so skipping ".git" dirs).

An explicit blacklist sounds fine. I filed https://github.com/golang/go/issues/16386

[dgla...@gmail.com]

On Thursday, July 14, 2016 at 10:34:36 PM UTC-7, bradfitz wrote:

goimports has been updated.

If you've been frustrated by its speed lately, run:

   $ go get -u golang.org/x/tools/cmd/goimports

... and things should be much nicer.

Details at https://golang.org/cl/24941

In addition to all the performance work, I'm excited to see:

> * when adding imports, add names to imports when the imported package name > doesn't match the baes of its import path. For example: > import foo "example.net/foo/v1"

 

if for no other reason than that godef works better with these imports (https://github.com/rogpeppe/godef/issues/40).

--dave

[Brad Fitzpatrick]

On Fri, Jul 15, 2016 at 5:47 AM, Peter Waller <pe...@pdftables.com> wrote:

Awesome!

For me it brings the CPU time from 6.5s to 3s. Wall from 2.9s to 2s. A noticable improvement.

One thing that still makes it slow for me (2s instead of 500ms, I just tested), is that I have several of deep trees which aren't go code in my $GOPATH/src. To name a few: linux, clang, llvm.

I feel silly for asking, but any chance of having a mechanism to ignore these non-go trees, or does anyone have any other clever ideas how to avoid this?

Done: https://go-review.googlesource.com/24971

[Peter Waller]

Just saw this was merged - this is excellent. 

These latest changes bring the runtime down to 400ms. Wonderful! I had no idea how much this was interrupting my flow before it was fixed :)

On 15 July 2016 at 19:34, Brad Fitzpatrick <brad...@golang.org> wrote:

Done: https://go-review.googlesource.com/24971

[Brad Fitzpatrick]

I am just as relieved. Imagine the pain and frustration you felt on every slow save but with an additional feeling of guilt that it's probably your fault and you should fix it, and other people use this thing nowadays. :)

[Peter Waller]

Muahah. Well I'm sure I can speak for many people when I say we're grateful it's being improved! :)

[Dave Cheney]

Why not put non go code in another directory tree? That seems much simpler.

[Peter Waller]

On 15 July 2016 at 22:30, Dave Cheney <da...@cheney.net> wrote:

Why not put non go code in another directory tree? That seems much simpler.

 

I agree with you and see where you're coming from. That was where my original hesitation came from, but...

My $GOPATH is .local/src. Why set it anywhere else? Then, why restrict it to go code?

And what about large directory structures with mixed sources?

[Daniel Skinner]

$HOME/local/src usage predates Go usage for me, and besides, the structure and content of a GOPATH has merit stretching beyond languages as a simple method for source organization.

On Fri, Jul 15, 2016, 4:30 PM Dave Cheney <da...@cheney.net> wrote:

Why not put non go code in another directory tree? That seems much simpler.

[Dmitri Shuralyov]

hey brad, is it possible to have a ```goimports version``` so that we know what versions we currently have?

I suggest using the command:

go get -u -v golang.org/x/tools/cmd/goimports

The -v flag will print packages that are rebuilt. If the output is empty, that means you already had the latest version. If the output is not empty, that means you had an outdated version, but now it's up to date.

If you want to find out whether your version is up to date before actually updating it, you can use gostatus and binstale tools.

[Vince Prignano]

I just noticed that import paths that start with an uppercase letter don't get imported automatically after the update.

Looking at the code and  https://github.com/golang/tools/blob/master/imports/fix.go#L651,

I tried to change the behavior, using strings.ToLower on `lastTwoComponents` like:

for _, pkg := range dirScan {

if !strings.Contains(strings.ToLower(lastTwoComponents(pkg.importPathShort)), pkgName) {

// Speed optimization to minimize disk I/O:

// the last two components on disk must contain the

// package name somewhere.

//

// This permits mismatch naming like directory

// "go-foo" being package "foo", or "pkg.v3" being "pkg",

// or directory "google.golang.org/api/cloudbilling/v1"

// being package "cloudbilling", but doesn't

// permit a directory "foo" to be package

// "bar", which is strongly discouraged

// anyway. There's no reason goimports needs

// to be slow just to accomodate that.

continue

}

if !canUse(filename, pkg.dir) {

continue

}

candidates = append(candidates, pkg)

}

and the imports work again like before.

This is the test case:

 

func TestFindImportGoPath(t *testing.T) {

goroot, err := ioutil.TempDir("", "goimports-")

if err != nil {

t.Fatal(err)

}

defer os.RemoveAll(goroot)

origStdlib := stdlib

defer func() {

stdlib = origStdlib

}()

stdlib = nil

withEmptyGoPath(func() {

// Test against imaginary bits/bytes package in std lib

bytesDir := filepath.Join(goroot, "src", "pkg", "bits", "Bytes")

for _, tag := range build.Default.ReleaseTags {

// Go 1.4 rearranged the GOROOT tree to remove the "pkg" path component.

if tag == "go1.4" {

bytesDir = filepath.Join(goroot, "src", "bits", "Bytes")

}

}

if err := os.MkdirAll(bytesDir, 0755); err != nil {

t.Fatal(err)

}

bytesSrcPath := filepath.Join(bytesDir, "bytes.go")

bytesPkgPath := "bits/Bytes"

bytesSrc := []byte(`package bytes

type Buffer2 struct {}

`)

if err := ioutil.WriteFile(bytesSrcPath, bytesSrc, 0775); err != nil {

t.Fatal(err)

}

build.Default.GOROOT = goroot

got, rename, err := findImportGoPath("bytes", map[string]bool{"Buffer2": true}, "x.go")

if err != nil {

t.Fatal(err)

}

if got != bytesPkgPath || !rename {

t.Errorf(`findImportGoPath("bytes", Buffer2 ...)=%q, %t, want "%s", false`, got, rename, bytesPkgPath)

}

got, rename, err = findImportGoPath("bytes", map[string]bool{"Missing": true}, "x.go")

if err != nil {

t.Fatal(err)

}

if got != "" || rename {

t.Errorf(`findImportGoPath("bytes", Missing ...)=%q, %t, want "", false`, got, rename)

}

})

}

-Vince

On Thursday, July 14, 2016 at 10:34:36 PM UTC-7, bradfitz wrote:

goimports has been updated.

If you've been frustrated by its speed lately, run:

   $ go get -u golang.org/x/tools/cmd/goimports

... and things should be much nicer.

Details at https://golang.org/cl/24941

[Brad Fitzpatrick]

Please file a bug. I lose emails.

[Vince Prignano]

https://github.com/golang/go/issues/16402

[roger peppe]

FWIW that particular issue isn't usually a problem for the
way that most people use godef (as an editor plugin), as
it only affects you if you use it directly on the command line,
something that I assume is fairly unusual.

The more general import name problem in godef has been
fixed for a little while now (unfortunately at some inevitable
speed cost).

cheers,
rog.

[David Glasser]

On Jul 19, 2016 3:16 AM, "roger peppe" <rogp...@gmail.com> wrote:
>
> On 15 July 2016 at 16:44,  <dgla...@gmail.com> wrote:
> > On Thursday, July 14, 2016 at 10:34:36 PM UTC-7, bradfitz wrote:
> >>
> >> goimports has been updated.
> >>
> >> If you've been frustrated by its speed lately, run:
> >>
> >>    $ go get -u golang.org/x/tools/cmd/goimports
> >>
> >> ... and things should be much nicer.
> >>
> >> Details at https://golang.org/cl/24941
> >
> >
> > In addition to all the performance work, I'm excited to see:
> >
> >> * when adding imports, add names to imports when the imported package name
> >> > doesn't match the baes of its import path. For example: > import foo
> >> "example.net/foo/v1"
> >
> > if for no other reason than that godef works better with these imports
> > (https://github.com/rogpeppe/godef/issues/40).
>
> FWIW that particular issue isn't usually a problem for the
> way that most people use godef (as an editor plugin), as
> it only affects you if you use it directly on the command line,
> something that I assume is fairly unusual.

Huh, I do see that issue when using godef in emacs. But maybe either my godef or its emacs integration is out of date.

[roger peppe]

Try updating your godef. If you still see the issue, get back to me, as AFAIK it
*should* be fixed!

[David Glasser]

My mistake: not only was I not up to date, I was running somebody else's unmerged PR that added vendor support. Looks good now!

--

gla...@davidglasser.net | langtonlabs.org | flickr.com/photos/glasser/

[roger peppe]

Good to hear - it's really my fault for taking ages to add vendor
support, sorry!

[Nathan Fisher]

+1 to this. Would a vendor folder make sense at the same level as the src folder? Make it an override able default.

I guess it doesn't address a monorepo approach though or the collocation of generated files like client/server stubs for protobuf/thrift/etc.

On Fri, 15 Jul 2016 at 22:30, Dave Cheney <da...@cheney.net> wrote:

Why not put non go code in another directory tree? That seems much simpler.

--
You received this message because you are subscribed to the Google Groups "golang-nuts" group.
To unsubscribe from this group and stop receiving emails from it, send an email to golang-nuts...@googlegroups.com.
For more options, visit https://groups.google.com/d/optout.

--

- from my thumbs to yours