-
Notifications
You must be signed in to change notification settings - Fork 4
Proper shell quoting #2
base: master
Are you sure you want to change the base?
Conversation
To support any paths that contain spaces or start with -. Also,
removes nonsensical use of ${} because it does nothing anyway.
|
This is probably good to go. I haven't tested it so someone else will probably need to confirm that nothing is broken. |
|
Not very keen on merging if we don't know it works. I also prefer ${} as less ambiguous in some cases. I'll use your PR to finish quoting the bash variables anyway so thanks for that |
| # https://github.com/stmuk/rakudup | ||
|
|
||
| RAKUDO_ROOT="${HOME}/.rakudup" | ||
| RAKUDO_ROOT=$HOME/.rakudup |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Why did you remove "" here?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
No need really, just for consistency with the other line below.
| then | ||
| set -x | ||
| if [[ $DEBUG ]]; then | ||
| set -x |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This breaks DEBUG=0
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
You sure it does?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes! I think it would have to be [ $DEBUG ] since [[ 0 ]] is [ "0" ] which is true.
I now remember why I use Perl! grrrr
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
DEBUG=0
if [[ $DEBUG ]]; then echo foo; fi # foo
if [ ! -z "$DEBUG" ]; then echo bar; fi # bar
Looks identical to me
| cd ${RAKUDO_SRC} | ||
| echo "Building Rakudo ${RELEASE}. This will take several minutes on a modern CPU." | ||
| echo "'tail -f ${RAKUDO_SRC}/build.log' for progress" | ||
| cd -- "$RAKUDO_SRC" || exit 1 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'll add the exit but don't care about the case where directory starts with dash! (Although it is interesting!)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
What? But that's the whole point…
To support any paths that contain spaces or start with -. Also,
removes nonsensical use of ${} because it does nothing anyway.