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

Properly use exepath for CheckBinPath #864

Merged
merged 3 commits into from
Jun 1, 2016
Merged

Properly use exepath for CheckBinPath #864

merged 3 commits into from
Jun 1, 2016

Conversation

luan
Copy link
Contributor

@luan luan commented May 20, 2016

#823 added this the wrong way, it was reverted back on 9f0cf00 but still not achieving the right functionality.
This uses it properly now, also adding it to how we lookup goimports for srcdir support.

luan added 2 commits May 19, 2016 23:59
 fatih#823 added this the wrong way, it was reverted back on 9f0cf00 but still not achieving the right functionality.
 This uses it properly now, also adding it to how we lookup goimports for srcdir support.
@luan
Copy link
Contributor Author

luan commented May 20, 2016

I realized that #839 had issues with this, it's because they were running an older patch of vim 7.4 that doesn't have exepath. I added a guard for that now.

@@ -115,7 +115,8 @@ function! go#fmt#Format(withGoimport)

if fmt_command == "goimports"
if !exists('b:goimports_vendor_compatible')
let out = go#util#System("goimports --help")
let binpath = go#path#CheckBinPath("goimports")
let out = go#util#System(binpath . " --help")
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please remove this as we check above already.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pushed a fix for this.

@luan
Copy link
Contributor Author

luan commented May 26, 2016

@fatih thoughts?

@atomatt
Copy link

atomatt commented May 26, 2016

fwiw, I manually applied the patch and it fixed things for me.

@fatih fatih merged commit a9611d7 into fatih:master Jun 1, 2016
@fatih
Copy link
Owner

fatih commented Jun 1, 2016

Thanks @luan 👍

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.

3 participants