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

Look harder for goimports #11

Merged
merged 2 commits into from
Mar 28, 2023
Merged

Look harder for goimports #11

merged 2 commits into from
Mar 28, 2023

Conversation

asmaloney
Copy link
Contributor

goimports may be in GOPATH's bin directory which may not be on the user's PATH.

Add GOPATH (falling back to go's default if not set explicitly) to PATH when looking for goimports.

goimports may be in GOPATH's bin directory which may not be on the user's PATH.

Add GOPATH (falling back to go's default if not set explicitly) to PATH when looking for goimports.
@bbredesen
Copy link
Owner

I've been holding back on merging this only to ask if we should look at an alternative solution. If there is a real risk that goimports isn't in GOPATH, is it possible that it also isn't in PATH?

I haven't tested, but see https://github.com/golang/tools/blob/master/imports/forward.go - This is a direct import that appears to call the same underlying code as goimports. Assuming that this works, the format/imports procedure could be run do away entirely with running an os.Cmd, and would have no dependency on the shell environment.

@asmaloney
Copy link
Contributor Author

If there is a real risk that goimports isn't in GOPATH, is it possible that it also isn't in PATH?

It's actually the opposite case. It might not be in PATH, so we need to make sure we look for it in GOPATH as well. If GOPATH isn't set explicitly, then go will use the default (which is why it all works with go).

For example, I don't have GOPATH set explicitly. The default path that go uses to install these things ($HOME/go - or more specifically $HOME/go/bin) also isn't in my PATH. Yet when I install goimports using the VS Code extension for example, it knows how to install everything and find it because it uses go's internal default (build.Default.GOPATH).

This is what I'm emulating here - adding either $GOPATH/bin (if GOPATH is set) OR build.Default.GOPATH/bin (if it's not) to PATH. (Keeping in mind that GOPATH can be a list of paths...)

This is all just a way to provide a better user experience by avoiding the whole "make sure goimports is on your PATH". It's easy to miss that and then wonder why it doesn't work.

Assuming that this works, the format/imports procedure could be run do away entirely

That would certainly be a better solution if it works and is stable! I hadn't thought of reaching into that that since I tend to stick to the standard library (or "/x" occasionally 😄 ).

@bbredesen bbredesen added this to the v0.2.0 milestone Mar 28, 2023
@bbredesen bbredesen merged commit c8fa475 into bbredesen:main Mar 28, 2023
@asmaloney asmaloney deleted the find-goimports branch March 28, 2023 17:34
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants