On Fri, Dec 3, 2010 at 10:57 PM, Junio C Hamano <gitster@xxxxxxxxx> wrote:
> Giuseppe Bilotta <giuseppe.bilotta@xxxxxxxxx> writes:
>
>> init_browser_path() {
>> browser_path=$(git config "browser.$1.path")
>> + if test -z "$browser_path" -a "$1" = chromium ; then
>> + type chromium-browser > /dev/null 2>&1 && browser_path=chromium-browser
>> + fi
>> test -z "$browser_path" && browser_path="$1"
>
> We tolerate
>
> test && test && effect
>
> and even encourage when the construct is short enough, over
>
> if test && test
> then
> effect
> fi
>
> But because you are writing an "if" block anyway, I think the above should
> be like this:
>
> if test -z "$browser_path" &&
> test "$1" = chromium &&
> type chromium-browser >/dev/null 2>&1
> then
> browser_path=chromium-browser
> fi
> browser_path=${browser_path:-"$1"}
>
> Yours is:
>
> if test && test
> then
> test && effect
> fi
>
> which is less than readable, no?
Absolutely. Actually, this final form is a side-effect on some
experimentations I was doing, thinking about doing something similar
with google-chrome vs chrome (in which case the body of the if would
get a case switch or something like that). I'll clean it up, unless
somebody can actually confirm that we do want these other special
cases.
--
Giuseppe "Oblomov" Bilotta
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html