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

From: Junio C Hamano <gitster@pobox.com>
Date: Dec 3, 2010, 21:57 UTC
Message-ID: <7vipzamsde.fsf@alter.siamese.dyndns.org>
In-Reply-To: <1291394861-11989-6-git-send-email-giuseppe.bilotta@gmail.com>

Giuseppe Bilotta <giuseppe.bilotta@gmail.com> writes:

Show 6 quoted lines

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