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

Fix to fails of GoMetalinter result filepath in subdir. resolve #1413 #1414

Merged

Conversation

hori-ryota
Copy link
Contributor

Execute :GoMetalinter in subdir, failed to parse filepath.

e.g.

Execute to foo/bar.go.
Now execute gometalinter in foo/ dir.
So, result path will bar.go but want foo/bar.go

@fatih
Copy link
Owner

fatih commented Aug 18, 2017

I think Martin changed this so it's consistent with others, adding him so he can chime as well. /cc @Carpetsmoker

@arp242
Copy link
Contributor

arp242 commented Aug 21, 2017

I tested this, and both go#tool#ExecuteInDir() and go#util#System() seem to work fine for me. I made sure to comment out the async part, and tested both inside outside of GOPATH.

This is because the command that gets run is:

/home/martin/go/bin/gometalinter --disable-all --enable=vet --enable=golint --enable=errcheck /home/martin/go/src/a/b --deadline=5s

Which will report back with the full path in the output.

I'm not sure why it's failing for you, but ExecuteInDir() is at best an unnecessary abstraction here, so removing it sounds like a good idea 👍

I think Martin changed this so it's consistent with others, adding him so he can chime as well. /cc @Carpetsmoker

Nope, didn't touch this :-) I did make a similar fix in #1381 though.

@@ -87,7 +87,7 @@ function! go#lint#Gometa(autosave, ...) abort

let meta_command = join(cmd, " ")

let out = go#tool#ExecuteInDir(meta_command)
let out = go#util#System(meta_command)
Copy link
Contributor

@arp242 arp242 Aug 21, 2017

Choose a reason for hiding this comment

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

Small thing, but a comment a few lines up (line 58) says:

" For sync mode (go#tool#ExecuteInDir), always explicitly pass the 5 seconds
" deadline if there is no other deadline configured. If a deadline is
" configured, then use it.

But this is no longer accurate.

Could you also update the comment? This is the sort of small function/code mismatch that can cause a lot of confusion in the (distant) future ;-)

@hori-ryota
Copy link
Contributor Author

My gometalinter returns relative path.

Perhaps this behavior may have changed the behavior of metalinter

Convert absolute paths to package paths for some linters by dnephin · Pull Request #319 · alecthomas/gometalinter

@mirza-s
Copy link
Contributor

mirza-s commented Aug 26, 2017

I can confirm this fixes #1417 for me.

@arp242
Copy link
Contributor

arp242 commented Sep 6, 2017

I just noticed that someone else made a PR for this previously: #1402; this PR is (slightly) better though, since it also updates a comment :-)

@fatih
Copy link
Owner

fatih commented Sep 6, 2017

@hori-ryota can you remove the accidentally added tags file please?

@hori-ryota
Copy link
Contributor Author

Oh, sorry and thank you for fixing it!

Remove accidentally committed file; update CHANGELOG · fatih/vim-go@ea33466

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.

4 participants