On Fri, Mar 03, 2017 at 03:57:38AM -0800, Denton Liu wrote:
> This fixes the discrepancy between difftool and mergetool where the
> former has the --gui flag and the latter does not by adding the
> functionality to mergetool.
>
> Signed-off-by: Denton Liu <[email protected]>
> ---
> Documentation/git-mergetool.txt | 8 +++++++-
> contrib/completion/git-completion.bash | 3 ++-
> git-mergetool.sh | 5 ++++-
> t/t7610-mergetool.sh | 28 +++++++++++++++++++++++++++-
> 4 files changed, 40 insertions(+), 4 deletions(-)
Would you mind splitting up this patch so that the
completion part is done separately?
> diff --git a/t/t7610-mergetool.sh b/t/t7610-mergetool.sh
> index 381b7df45..5683907ab 100755
> --- a/t/t7610-mergetool.sh
> +++ b/t/t7610-mergetool.sh
> @@ -145,6 +147,30 @@ test_expect_success 'custom mergetool' '
> git commit -m "branch1 resolved with mergetool"
> '
>
> +test_expect_success 'gui mergetool' '
> + test_when_finished "git reset --hard" &&
> + test_when_finished "git config merge.tool mytool" &&
> + test_when_finished "git config --unset merge.guitool" &&
> + git config merge.tool badtool &&
> + git config merge.guitool mytool &&
> + git checkout -b test$test_count branch1 &&
> + git submodule update -N &&
> + test_must_fail git merge master >/dev/null 2>&1 &&
It'd probably be better to use test_expect_code instead of
test_must_fail here:
test_expect_code 1 git merge master ...
> + ( yes "" | git mergetool -g both >/dev/null 2>&1 ) &&
> + ( yes "" | git mergetool -g file1 file1 ) &&
> + ( yes "" | git mergetool --gui file2 "spaced name" >/dev/null 2>&1 ) &&
> + ( yes "" | git mergetool --gui subdir/file3 >/dev/null 2>&1 ) &&
I realize that this test is based on an existing one, but I'm curious..
is "yes" used above because it's prompting, and using -y or --no-prompt
here would eliminate the need for the 'yes ""' parts?
Looks good otherwise.
Thanks,
--
David
