Re: [PATCHv2 5/7] web--browse: better support for chromium

[Junio C Hamano <gitster@xxxxxxxxx>]

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?
--
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