mdempsky / unconvert

Remove unnecessary type conversions from Go source
BSD 3-Clause "New" or "Revised" License
377 stars 26 forks source link

Handle [package ...] arguments like most Go tools. #9

Closed dmitshur closed 8 years ago

dmitshur commented 8 years ago

Support import path patterns, including within relative import paths. When given 0 arguments, use the package in current working directory. If the import path patterns match no packages, print a warning and exit with 0 exit code.

Rely on go/loader to resolve relative import paths relative to current working directory.

Fixes #4. Fixes #8.

dmitshur commented 8 years ago

Okay, so it seems that unconvert does not need resolveRelative functionality. I think it's usually nice to always normalize import paths (i.e., convert all relative to full import paths), because some tools/libraries may not support relative (e.g., golang.org/x/tools/refactor/importgraph doesn't), but it appears that loader does (see documentation written at https://godoc.org/golang.org/x/tools/go/loader#Config.Cwd).

So, I'm not going to include that since it's not required to make things work, and I don't want to have more code than necessary. If you ever change unconvert in a way that requires full import paths, just insert these 4 lines after the call to gotool.ImportPaths:

importPaths, err := resolveRelative(importPaths)
if err != nil {
    // handle error
}

(And resolveRelative itself is just this code.)

dmitshur commented 8 years ago

I would also make the recommendation to change your README to say go get -u github.com/mdempsky/unconvert instead of go get github.com/mdempsky/unconvert, so that users don't accidentally run into problems if they happen to have outdated versions of unconvert or its dependencies, which may or may not work depending on luck. But it's up to you.