Denton Liu <liu.denton@xxxxxxxxx> writes:
> Subject: Re: [PATCH 1/2] mergetool: Accept -g/--[no-]gui as arguments
Other people may point it out, but s/Accept/accept/.
> In line with how difftool accepts a -g/--[no-]gui option, make mergetool
> accept the same option in order to use the `merge.guitool` variable to
> find the default mergetool instead of `merge.tool`.
> ...
> diff --git a/git-mergetool--lib.sh b/git-mergetool--lib.sh
> index 9a8b97a2a..e317ae003 100644
> --- a/git-mergetool--lib.sh
> +++ b/git-mergetool--lib.sh
> @@ -350,17 +350,23 @@ guess_merge_tool () {
> }
>
> get_configured_merge_tool () {
> - # Diff mode first tries diff.tool and falls back to merge.tool.
> - # Merge mode only checks merge.tool
> + # If first argument is true, find the guitool instead
> + if [ "$1" = true ]
Don't use [] in our shell script (see Documentation/CodingGuidelines);
say
if "$1" = true
instead.
> + then
> + gui_prefix=gui
> + fi
> +
> + # Diff mode first tries diff.(gui)tool and falls back to merge.(gui)tool.
> + # Merge mode only checks merge.(gui)tool
> if diff_mode
> then
> - merge_tool=$(git config diff.tool || git config merge.tool)
> + merge_tool=$(git config diff.${gui_prefix}tool || git config merge.${gui_prefix}tool)
> else
> - merge_tool=$(git config merge.tool)
> + merge_tool=$(git config merge.${gui_prefix}tool)
> fi
In mode_ok shell function, we seem to be prepared to deal with a
case where neither diff_mode nor merge_mode is true. Should this
codepath also be prepared to?
This is not a comment to point out an issue with this patch. It is
a genuine question on the code after (or before for that matter)
this patch is applied.
Thanks.
