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

eliminate errors trying to run a command in a directory that does not exist #1527

Merged
merged 2 commits into from
Nov 17, 2017

Conversation

bhcleek
Copy link
Collaborator

@bhcleek bhcleek commented Oct 20, 2017

Fixes #1399

" that don't actually exist on the file system (e.g. vim-fugitive's
" GitDiff).
if !isdirectory(expand("%:p:h"))
let out = go#util#System("exit 1")
Copy link
Contributor

Choose a reason for hiding this comment

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

Could you use false, instead of exit? The former is in POSIX, the latter isn't. Also exit is usually a shell built-in, so this code depends on the shell.

You should also use go#util#Exec() for all new code, the System() function will be removed once all calls are rewritten to use that.

@bhcleek
Copy link
Collaborator Author

bhcleek commented Oct 30, 2017

@Carpetsmoker PTAL

@arp242
Copy link
Contributor

arp242 commented Nov 4, 2017

I just added pushed a basic test for this.

I do wonder if silently doing nothing is really the best behaviour though. For example Errcheck will now simply do nothing. I wonder if that's really the best thing to do?

@arp242
Copy link
Contributor

arp242 commented Nov 4, 2017

Meh, also seems that Neovim behaves slightly different

@bhcleek
Copy link
Collaborator Author

bhcleek commented Nov 4, 2017

If go#lint#Errcheck doesn't check the ShellError or is hiding errors for some other reason, then that's a completely separate problem. This change merely makes sure to return early and avoid a bunch of useless, noisy messages when the directory that go#util#ExecuteInDir is supposed to execute in does not exist, but I don't see that behavior. I see a message, "package is not inside GOPATH src"

I'm working on a fix for the nvim behavior. Thanks for adding some tests!

@bhcleek
Copy link
Collaborator Author

bhcleek commented Nov 4, 2017

PTAL.

I'll rebase before merging to apply the fixups correctly.

@bhcleek
Copy link
Collaborator Author

bhcleek commented Nov 17, 2017

I'll rebase now and then merge.

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.

2 participants