Support vendor imports #18
Conversation
|
Thanks for this. Funny that both PRs should land in one day. This one is closer to the desired final form, so maybe we can iterate on it? I'd rather use a flag than a positional arg for this, since I think it will be rare. That is: |
|
+1
Will do. I have a vim-go adjustment for it as well which I'll push out
when github puts out this fire.
…On Tue, Jun 6, 2017 at 4:07 PM, Josh Bleecher Snyder < ***@***.***> wrote:
Thanks for this. Funny that both PRs should land in one day. This one is
closer to the desired final form, so maybe we can iterate on it?
I'd rather use a flag than a positional arg for this, since I think it
will be rare. That is: impl [-dir dir] <recv> <iface>. If it is not
provided, it should default to os.Getwd(), and if os.Getwd() returns an
error, it should default to "" (and not display an error to the user).
—
You are receiving this because you authored the thread.
Reply to this email directly, view it on GitHub
<#18 (comment)>, or mute
the thread
<https://github.com/notifications/unsubscribe-auth/AAAxCRQ2PjOj_gIviQLBQP7CveEeQH0pks5sBduwgaJpZM4NyBSN>
.
|
boz
force-pushed
the
master
branch
2 times, most recently
from
2d98cef
to
269f090
Compare
|
@josharian take a look. had to add faux filename to make things work correctly. I'd push up the vim-go change but my fork is a github crash casualty at the moment. |
* attempt to use `-dir` argument from josharian/impl#18 if `impl` fails. * backwards compatible with previous `impl` versions.
impl.go
Outdated
| fmt.Fprint(os.Stderr, usage) | ||
| os.Exit(2) | ||
| } | ||
| recv, iface := os.Args[1], os.Args[2] | ||
|
|
||
| recv, iface := flag.Args()[0], flag.Args()[1] |
| @@ -34,7 +34,7 @@ func TestFindInterface(t *testing.T) { | |||
| } | |||
|
|
|||
| for _, tt := range cases { | |||
| path, id, err := findInterface(tt.iface) | |||
| path, id, err := findInterface(tt.iface, ".") | |||
impl.go
Outdated
| if !validReceiver(recv) { | ||
| fatal(fmt.Sprintf("invalid receiver: %q", recv)) | ||
| } | ||
|
|
||
| fns, err := funcs(iface) | ||
| if *sourceDirectory == "" { | ||
| *sourceDirectory, _ = os.Getwd() |
There was a problem hiding this comment.
If err is non-nil, we should ignore the other return value, unless that behavior is specifically documented:
if dir, err := os.Getwd(); err != nil {
*sourceDirectory = dir
}
There was a problem hiding this comment.
See #18 (comment). I believe you have a typo in your snippet, it should be err == nil instead of err != nil, since inside the if statement is the success path rather than failure path.
impl.go
Outdated
|
|
||
| Don't forget the single quotes around the receiver type | ||
| to prevent shell globbing. | ||
| ` | ||
|
|
||
| var ( | ||
| sourceDirectory = flag.String("dir", "", "package source directory") |
There was a problem hiding this comment.
Nit: please name this flagSourceDirectory or flagSrcDir. It helps highlight that it is a flag value, even far from its declaration.
If you can, it'd be nice to add a short clause hinting to the reader why/when they would want this, maybe something like "package source directory, useful for vendored code".
boz
force-pushed
the
master
branch
3 times, most recently
from
6589e61
to
0f159c4
Compare
* allow specifying directory of import
|
Thanks again for doing this. |
| if !validReceiver(recv) { | ||
| fatal(fmt.Sprintf("invalid receiver: %q", recv)) | ||
| } | ||
|
|
||
| fns, err := funcs(iface) | ||
| if *flagSrcDir == "" { | ||
| if dir, err := os.Getwd(); err != nil { |
There was a problem hiding this comment.
Should this be err == nil instead?
There was a problem hiding this comment.
Are you planning on taking this @boz?
It's been a few days and it's still not fixed, and I'm just trying to figure out whether I need to send a PR or not.
There was a problem hiding this comment.
@shurcooL please send a PR. Thanks!
The body of the if statement is the success case, so it should run when err == nil. Fixup for #18 (comment).
* attempt to use `-dir` argument from josharian/impl#18 if `impl` fails. * backwards compatible with previous `impl` versions.
* attempt to use `-dir` argument from josharian/impl#18 if `impl` fails. * backwards compatible with previous `impl` versions.
After putting this together, I noticed that #17 does something similar.
This helps support vendor imports. Defaulting it to
$PWDwould work for my workflow but I don't know about others.If this is accepted I'll add support for it in https://github.com/fatih/vim-go