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

Various fixes for paths and GOPATH with spaces #1374

Merged
merged 2 commits into from
Jul 24, 2017
Merged

Various fixes for paths and GOPATH with spaces #1374

merged 2 commits into from
Jul 24, 2017

Conversation

arp242
Copy link
Contributor

@arp242 arp242 commented Jul 24, 2017

Fixes #1288, as well as a number of other commands.

There may be other places as well; this is just the result from a quick test
with my personal default settings. Since there are a lot of if/else branches
relating to jobs etc. it's kind of hard to test everything without a full audit.

It's perhaps a good idea to refactor some of this in a slightly more structured
way at some point: instead of passing strings we should pass a list which is
always escaped (like os.Exec(), Python's subprocess, etc.)

For example: `~/go/src/space path/a/a.go`

Tested with both `godef` and `guru`.

Continuation of #1288
Test setup:

	export GOPATH="/home/martin/go path/go"
	export PATH="/usr/local/sbin:/usr/local/bin:/usr/bin"

	vim "/home/martin/go path/go/src/a/a.go"

There may be other places as well; this is just the result from a quick test
with my personal default settings. Since there are a lot of if/else branches
relating to jobs etc. it's kind of hard to test everything without a full audit.

It's perhaps a good idea to refactor some of this in a slightly more structured
way at some point: instead of passing strings we should pass a list which is
always escaped (like `os.Exec()`, Python's subprocess, etc.)

This should fix #1288
@fatih fatih merged commit 307180c into fatih:master Jul 24, 2017
@fatih
Copy link
Owner

fatih commented Jul 24, 2017

Thanks @Carpetsmoker much appreciated 👍

It's perhaps a good idea to refactor some of this in a slightly more structured
way at some point: instead of passing strings we should pass a list which is
always escaped (like os.Exec(), Python's subprocess, etc.)

We do it already for jobs, but not for old style system call. For those for some of the newly refactored code (Such as lint.vim (gometalinter) I'm converting the list to a string before passing it to system(). But it's a constant improvement so we can do these kind of things one by one :)

@arp242 arp242 deleted the spaces branch July 24, 2017 12:30
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