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 handle projects/workspaces with spaces in the name #55

Closed
gfontenot opened this issue Jul 12, 2016 · 10 comments
Closed

Properly handle projects/workspaces with spaces in the name #55

gfontenot opened this issue Jul 12, 2016 · 10 comments
Labels

Comments

@gfontenot
Copy link
Owner

We aren't doing this correctly everywhere right now.

@gfontenot
Copy link
Owner Author

I think right now we're just messing it up for the run actions.

@gfontenot gfontenot added the bug label Jul 12, 2016
@sharplet
Copy link
Contributor

This should include scheme names with spaces, too.

@gfontenot
Copy link
Owner Author

I wonder if someone like @gabebw or @pbrisbin have ideas here. We're passing the scheme and workspace/project to the run_ios_app script correctly, and then using $@ to pass those to the xcodebuild command, but it seems like the quotes are getting stripped somewhere along the line.

@gfontenot
Copy link
Owner Author

More specifically:

My assumption is that by the time the flags get to $@ they have lost the quotes, but I'm not sure how to fix that.

@gabebw
Copy link

gabebw commented Jul 12, 2016

The part that jumps out to me is that in your last link, $@ isn't quoted like "$@".

I wrote about the differences, but the idea is that $@ without quotes doesn't respect quotes even if the arguments are quoted.

@gabebw
Copy link

gabebw commented Jul 12, 2016

shellcheck agrees:

$ shellcheck run_ios_app 

In run_ios_app line 5:
build_path=$(xcodebuild -showBuildSettings $@ 2>/dev/null | egrep "\bBUILD_DIR\b" | sed -E "s/[[:space:]]+BUILD_DIR = //")
                                           ^-- SC2068: Double quote array expansions to avoid re-splitting elements.

@gabebw
Copy link

gabebw commented Jul 12, 2016

shellcheck found the same problem in some of the other scripts in bin. This is the full output:

$ shellcheck bin/*

In bin/list_schemes.sh line 5:
xcrun xcodebuild -list $@ 2>/dev/null \
                       ^-- SC2068: Double quote array expansions to avoid re-splitting elements.


In bin/run_ios_app line 5:
build_path=$(xcodebuild -showBuildSettings $@ 2>/dev/null | egrep "\bBUILD_DIR\b" | sed -E "s/[[:space:]]+BUILD_DIR = //")
                                           ^-- SC2068: Double quote array expansions to avoid re-splitting elements.


In bin/run_mac_app line 3:
build_path=$(xcodebuild -showBuildSettings $@ 2>/dev/null | egrep "\bBUILD_DIR\b" | sed -E "s/[[:space:]]+BUILD_DIR = //")
                                           ^-- SC2068: Double quote array expansions to avoid re-splitting elements.


In bin/use_simulator.sh line 5:
xcrun xcodebuild -showBuildSettings $@ 2>/dev/null \
                                    ^-- SC2068: Double quote array expansions to avoid re-splitting elements.

@gfontenot
Copy link
Owner Author

Ah, I didn't realizing that quoting $@ worked that way. Neat. I should look into getting ci set up to run shellcheck against PRs.

@pbrisbin
Copy link

I should look into getting ci set up to run shellcheck against PRs.

You could also use Code Climate :D

https://codeclimate.com/github/pbrisbin/aurget/issues

@gfontenot gfontenot mentioned this issue Jul 13, 2016
@pbrisbin
Copy link

pbrisbin commented Jul 13, 2016

In addition to the shellcheck engine, there's also vint. I'm not sure how good it is, but worth a shot.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

4 participants