Re: [PATCH 1/4] mergetool--lib: Simplify command expressions

From: David Aguilar <davvid@gmail.com>
Date: Jan 27, 2013, 22:18 UTC
Message-ID: <CAJDDKr61wS1ruPYDW4+exWBWveyS0Dp7Gmu5gWRh_99-frc_7Q@mail.gmail.com>
In-Reply-To: <5105A56E.4010002@kdbg.org>

On Sun, Jan 27, 2013 at 2:08 PM, Johannes Sixt <j6t@kdbg.org> wrote:

Show 16 quoted lines

> Am 27.01.2013 22:24, schrieb David Aguilar:
>> Use $(command "$arg") instead of "$(command "$arg")" as the latter is
>> harder to read.
>
> If at all, you should restrict yourself to simplify only variable
> assignments. Because this case:
>
>> -     if test -z "$(get_merge_tool_cmd "$merge_tool")" &&
>> +     if test -z $(get_merge_tool_cmd "$merge_tool") &&
>
> cannot work as intended: If the output of $() is empty, then without the
> outer quotes this becomes
>
>   test -z
>
> without an operand for -z, which is a syntax error (of the test command).

Definitely. I learned this the hard way when the tests broke on me while working it ;-) My patch rewrites things to always use var=$(command) expressions with separate test "$var" evaluating them. Thanks for the tip,

-- 
David