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

From: Junio C Hamano <gitster@pobox.com>
Date: Jan 25, 2013, 21:47 UTC
Message-ID: <7vbocd2auo.fsf@alter.siamese.dyndns.org>
In-Reply-To: <20130125211601.GD7498@serenity.lan>

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

Show 6 quoted lines

>> > It doesn't - the "|| continue" is to catch errors from setup_tool.
>> 
>> Ugh.
>
> Is that targeted at my suggestion at the top of this email or calling
> exit in setup_tool?

At the fact that you had to go a convoluted route because you cannot just run setup_tool in subshell and do translate_merge_tool_path after that, because the latter needs to look at the shell variable the former sets.

Show 8 quoted lines

> With the patch above, the block of code at the top becomes:
>
>  	test "$tool" = defaults && continue
>
>  	setup_tool "$tool" 2>/dev/null || continue
>  	merge_tool_path=$(translate_merge_tool_path "$tool")
>
> which IMHO is pretty readable.

Of course it is. The current callers of setup_tool may need some adjustments, but that should be fairly trivial, I hope.