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.

Sign up for GitHub

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

Set GOPATH for both possible go guru execution paths #1193

Merged
merged 3 commits into from
Feb 19, 2017
Merged

Set GOPATH for both possible go guru execution paths #1193

Changes from 1 commit
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
83626b4
Set GOPATH for both possible go guru execution paths
Feb 2, 2017
d3b38ba
Address comments: naming and whitespace
Feb 2, 2017
0c348ec
Fix incorrect conditional flow
Feb 12, 2017
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
16 changes: 10 additions & 6 deletions autoload/go/guru.vim
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -120,8 +120,6 @@ function! s:sync_guru(args) abort
endif
endif

let old_gopath = $GOPATH
let $GOPATH = go#path#Detect()

" run, forrest run!!!
let command = join(result.cmd, " ")
Expand All @@ -131,8 +129,6 @@ function! s:sync_guru(args) abort
let out = go#util#System(command)
endif

let $GOPATH = old_gopath

if has_key(a:args, 'custom_parse')
call a:args.custom_parse(go#util#ShellError(), out)
else
Expand Down Expand Up @@ -217,11 +213,19 @@ endfunc

" run_guru runs the given guru argument
function! s:run_guru(args) abort

Copy link
Owner

Choose a reason for hiding this comment

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

Can you remove this newline please

let old_gopath = $GOPATH
let $GOPATH = go#path#Detect()
if go#util#has_job()
return s:async_guru(a:args)
let abort = s:async_guru(a:args)
Copy link
Owner

Choose a reason for hiding this comment

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

This has several meanings. For async it's the job id however for sync_guru it's the output. So the caller defines what to look. Can you rename it to res as for result? I think that would make more sense.

Copy link
Contributor Author

@jblebrun jblebrun Feb 2, 2017 •
edited
Loading

Choose a reason for hiding this comment

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

I have no idea why I called it abort. I think I was distracted by the abort in the function signature.

On closer examination... does the result need to be returned at all? No callers seem to use it. Can we just call the appropriate function and then return nothing here?

Copy link
Owner

Choose a reason for hiding this comment

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

I was returning with the previous one, as we might need it in the future and it's handy. You can also just return however I'm not sure how to unset it easily (as Vim doesn't have defer like we have in Go)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Ok, I've left it and changed the name to res as you suggested.

endif

return s:sync_guru(a:args)
let abort = s:sync_guru(a:args)

let $GOPATH = old_gopath

return abort

Copy link
Owner

Choose a reason for hiding this comment

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

Can you remove this newline as well please?

endfunction

" Show 'implements' relation for selected package
Expand Down