Clean up the style in the ninja shell wrapper.

Clean up the style in the ninja shell wrapper.

Use case rather than inconsistently using [ and [[.

Committed: http://src.chromium.org/viewvc/chrome?view=rev&revision=172343

[tony, 2012-12-11 00:12:44]

Robert mentioned the weird style for this file over IM.

[iannucci, 2012-12-11 00:25:34]

https://codereview.chromium.org/11522008/diff/1/ninja
File ninja (right):

https://codereview.chromium.org/11522008/diff/1/ninja#newcode14
ninja:14: 64)   exec "${THIS_DIR}/ninja-linux64" "$@";;
I might do something like:

32|64) exec "${THIS_DIR}/ninja-linux${MACHINE}" "$@";;

But I'm not sure how you feel about that.

https://codereview.chromium.org/11522008/diff/1/ninja#newcode21
ninja:21: Darwin)
Maybe these can all be on one line each like for the case $MACHINE ones?

https://codereview.chromium.org/11522008/diff/1/ninja#newcode25
ninja:25: exec cmd.exe /c `cygpath -t windows $0`.exe "$@"
Should use $() over ``

[tony, 2012-12-11 00:34:51]

https://codereview.chromium.org/11522008/diff/1/ninja
File ninja (right):

https://codereview.chromium.org/11522008/diff/1/ninja#newcode14
ninja:14: 64)   exec "${THIS_DIR}/ninja-linux64" "$@";;
On 2012/12/11 00:25:34, iannucci wrote:
> I might do something like:
> 
> 32|64) exec "${THIS_DIR}/ninja-linux${MACHINE}" "$@";;
> 
> But I'm not sure how you feel about that.

Done.

https://codereview.chromium.org/11522008/diff/1/ninja#newcode21
ninja:21: Darwin)
On 2012/12/11 00:25:34, iannucci wrote:
> Maybe these can all be on one line each like for the case $MACHINE ones?

Done.

https://codereview.chromium.org/11522008/diff/1/ninja#newcode25
ninja:25: exec cmd.exe /c `cygpath -t windows $0`.exe "$@"
On 2012/12/11 00:25:34, iannucci wrote:
> Should use $() over ``

Done.

[iannucci, 2012-12-11 00:44:39]

Awesome, lgtm. Thanks :)

https://codereview.chromium.org/11522008/diff/5001/ninja
File ninja (right):

https://codereview.chromium.org/11522008/diff/5001/ninja#newcode23
ninja:23: *)         echo "Unsupported OS ${OS}" ; exit 1;;
This could can be on 2 lines since it's really 2 commands

[tony, 2012-12-11 17:40:58]

cmp, can I get owners approval?

https://codereview.chromium.org/11522008/diff/5001/ninja
File ninja (right):

https://codereview.chromium.org/11522008/diff/5001/ninja#newcode23
ninja:23: *)         echo "Unsupported OS ${OS}" ; exit 1;;
On 2012/12/11 00:44:40, iannucci wrote:
> This could can be on 2 lines since it's really 2 commands

Done.

[M-A Ruel, 2012-12-11 17:43:46]

https://codereview.chromium.org/11522008/diff/9001/ninja
File ninja (right):

https://codereview.chromium.org/11522008/diff/9001/ninja#newcode11
ninja:11: Linux)
Please align at +2 and the inner at +4.

https://codereview.chromium.org/11522008/diff/9001/ninja#newcode14
ninja:14: 32|64)   exec "${THIS_DIR}/ninja-linux${MACHINE}" "$@";;
Same

[tony, 2012-12-11 17:53:33]

https://codereview.chromium.org/11522008/diff/9001/ninja
File ninja (right):

https://codereview.chromium.org/11522008/diff/9001/ninja#newcode11
ninja:11: Linux)
On 2012/12/11 17:43:46, Marc-Antoine Ruel wrote:
> Please align at +2 and the inner at +4.

Done.

[M-A Ruel, 2012-12-11 17:57:42]

lgtm

[commit-bot: I haz the power, 2012-12-11 17:58:02]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/tony@chromium.org/11522008/15001

[commit-bot: I haz the power, 2012-12-11 18:00:19]

Change committed as 172343

[Nico, 2012-12-11 22:09:43]

could have waited until my tweak landed :-(

[Nico, 2012-12-11 22:11:03]

Also, the before code is a lot easier to read for people not familar in shell
(like me) imho. *shrug*