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

2013-01-25 Thread John Keeping
On Fri, Jan 25, 2013 at 01:43:54AM -0800, David Aguilar wrote: Check the can_diff and can_merge functions before deciding whether to add the tool to the available/unavailable lists. This makes --tool-help context- sensitive so that git mergetool --tool-help displays merge tools only and git

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

2013-01-25 Thread Junio C Hamano
John Keeping j...@keeping.me.uk writes: +tool=$(basename $i) Quotes are unnecessary here. Yeah, the outer quotes aren't needed; the inner ones are. +if test $tool = defaults +then +continue +elif merge_mode ! can_merge

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

2013-01-25 Thread John Keeping
On Fri, Jan 25, 2013 at 07:54:46PM +, John Keeping wrote: On Fri, Jan 25, 2013 at 01:43:54AM -0800, David Aguilar wrote: Check the can_diff and can_merge functions before deciding whether to add the tool to the available/unavailable lists. This makes --tool-help context- sensitive

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

2013-01-25 Thread Junio C Hamano
John Keeping j...@keeping.me.uk writes: Actually, can we just change all of the above part of the loop to: test $tool = defaults continue merge_tool_path=$( setup_tool $tool /dev/null 21 translate_merge_tool_path $tool ) || continue Meaning

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

2013-01-25 Thread John Keeping
On Fri, Jan 25, 2013 at 12:16:42PM -0800, Junio C Hamano wrote: John Keeping j...@keeping.me.uk writes: Actually, can we just change all of the above part of the loop to: test $tool = defaults continue merge_tool_path=$( setup_tool $tool /dev/null 21

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

2013-01-25 Thread Junio C Hamano
John Keeping j...@keeping.me.uk writes: On Fri, Jan 25, 2013 at 12:16:42PM -0800, Junio C Hamano wrote: John Keeping j...@keeping.me.uk writes: Actually, can we just change all of the above part of the loop to: test $tool = defaults continue merge_tool_path=$(

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

2013-01-25 Thread John Keeping
On Fri, Jan 25, 2013 at 12:56:37PM -0800, Junio C Hamano wrote: John Keeping j...@keeping.me.uk writes: On Fri, Jan 25, 2013 at 12:16:42PM -0800, Junio C Hamano wrote: John Keeping j...@keeping.me.uk writes: Actually, can we just change all of the above part of the loop to:

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

2013-01-25 Thread Junio C Hamano
John Keeping j...@keeping.me.uk writes: 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

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

2013-01-25 Thread John Keeping
On Fri, Jan 25, 2013 at 01:47:59PM -0800, Junio C Hamano wrote: John Keeping j...@keeping.me.uk writes: With the patch above, the block of code at the top becomes: test $tool = defaults continue setup_tool $tool 2/dev/null || continue