Re: [PATCH 7/7] mergetool--lib: Improve show_tool_help() output

From: Junio C Hamano <gitster@pobox.com>
Date: Jan 25, 2013, 20:06 UTC
Message-ID: <7vwqv12fjk.fsf@alter.siamese.dyndns.org>
In-Reply-To: <20130125195446.GA7498@serenity.lan>

John Keeping <john@keeping.me.uk> writes:

>> +		tool="$(basename "$i")"
>
> Quotes are unnecessary here.

Yeah, the outer quotes aren't needed; the inner ones are.

Show 19 quoted lines

>> +		if test "$tool" = "defaults"
>> +		then
>> +			continue
>> +		elif merge_mode && ! can_merge
>> +		then
>> +			continue
>> +		elif diff_mode && ! can_diff
>> +		then
>> +			continue
>> +		fi
>
> Would this be better as:
>
>     test "$tool" = "defaults" && continue
>
>     can_merge || ! merge_mode || continue
>     can_diff || ! diff_mode || continue
>
> or is that a bit too concise?

It is beyond "too concise"; it is unreadable, and more importantly, the latter two lines are illogical (why do you even ask if it can be used for merging, before asking merge_mode to see if the answer to that question matters to you?)