From e9d309e2fcd4faaeee9d20ef2a46bf86a4847bd0 Mon Sep 17 00:00:00 2001 From: Denton Liu Date: Wed, 24 Apr 2019 15:46:57 -0700 Subject: t7610: unsuppress output The output for commands used to be suppressed by redirecting both stdout and stderr to /dev/null. However, this should not happen since the output is useful for debugging and, without the "-v" flag, test scripts don't output anyway. Unsuppress the output by removing the redirections to /dev/null. Signed-off-by: Denton Liu Signed-off-by: Junio C Hamano diff --git a/t/t7610-mergetool.sh b/t/t7610-mergetool.sh index a9fb971..6971148 100755 --- a/t/t7610-mergetool.sh +++ b/t/t7610-mergetool.sh @@ -130,14 +130,14 @@ test_expect_success 'custom mergetool' ' test_when_finished "git reset --hard" && git checkout -b test$test_count branch1 && git submodule update -N && - test_must_fail git merge master >/dev/null 2>&1 && - ( yes "" | git mergetool both >/dev/null 2>&1 ) && + test_must_fail git merge master && + ( yes "" | git mergetool both ) && ( yes "" | git mergetool file1 file1 ) && - ( yes "" | git mergetool file2 "spaced name" >/dev/null 2>&1 ) && - ( yes "" | git mergetool subdir/file3 >/dev/null 2>&1 ) && - ( yes "d" | git mergetool file11 >/dev/null 2>&1 ) && - ( yes "d" | git mergetool file12 >/dev/null 2>&1 ) && - ( yes "l" | git mergetool submod >/dev/null 2>&1 ) && + ( yes "" | git mergetool file2 "spaced name" ) && + ( yes "" | git mergetool subdir/file3 ) && + ( yes "d" | git mergetool file11 ) && + ( yes "d" | git mergetool file12 ) && + ( yes "l" | git mergetool submod ) && test "$(cat file1)" = "master updated" && test "$(cat file2)" = "master new" && test "$(cat subdir/file3)" = "master new sub" && @@ -153,15 +153,15 @@ test_expect_success 'mergetool crlf' ' # test_when_finished is LIFO.) test_config core.autocrlf true && git checkout -b test$test_count branch1 && - test_must_fail git merge master >/dev/null 2>&1 && - ( yes "" | git mergetool file1 >/dev/null 2>&1 ) && - ( yes "" | git mergetool file2 >/dev/null 2>&1 ) && - ( yes "" | git mergetool "spaced name" >/dev/null 2>&1 ) && - ( yes "" | git mergetool both >/dev/null 2>&1 ) && - ( yes "" | git mergetool subdir/file3 >/dev/null 2>&1 ) && - ( yes "d" | git mergetool file11 >/dev/null 2>&1 ) && - ( yes "d" | git mergetool file12 >/dev/null 2>&1 ) && - ( yes "r" | git mergetool submod >/dev/null 2>&1 ) && + test_must_fail git merge master && + ( yes "" | git mergetool file1 ) && + ( yes "" | git mergetool file2 ) && + ( yes "" | git mergetool "spaced name" ) && + ( yes "" | git mergetool both ) && + ( yes "" | git mergetool subdir/file3 ) && + ( yes "d" | git mergetool file11 ) && + ( yes "d" | git mergetool file12 ) && + ( yes "r" | git mergetool submod ) && test "$(printf x | cat file1 -)" = "$(printf "master updated\r\nx")" && test "$(printf x | cat file2 -)" = "$(printf "master new\r\nx")" && test "$(printf x | cat subdir/file3 -)" = "$(printf "master new sub\r\nx")" && @@ -176,8 +176,8 @@ test_expect_success 'mergetool in subdir' ' git submodule update -N && ( cd subdir && - test_must_fail git merge master >/dev/null 2>&1 && - ( yes "" | git mergetool file3 >/dev/null 2>&1 ) && + test_must_fail git merge master && + ( yes "" | git mergetool file3 ) && test "$(cat file3)" = "master new sub" ) ' @@ -188,14 +188,14 @@ test_expect_success 'mergetool on file in parent dir' ' git submodule update -N && ( cd subdir && - test_must_fail git merge master >/dev/null 2>&1 && - ( yes "" | git mergetool file3 >/dev/null 2>&1 ) && - ( yes "" | git mergetool ../file1 >/dev/null 2>&1 ) && - ( yes "" | git mergetool ../file2 ../spaced\ name >/dev/null 2>&1 ) && - ( yes "" | git mergetool ../both >/dev/null 2>&1 ) && - ( yes "d" | git mergetool ../file11 >/dev/null 2>&1 ) && - ( yes "d" | git mergetool ../file12 >/dev/null 2>&1 ) && - ( yes "l" | git mergetool ../submod >/dev/null 2>&1 ) && + test_must_fail git merge master && + ( yes "" | git mergetool file3 ) && + ( yes "" | git mergetool ../file1 ) && + ( yes "" | git mergetool ../file2 ../spaced\ name ) && + ( yes "" | git mergetool ../both ) && + ( yes "d" | git mergetool ../file11 ) && + ( yes "d" | git mergetool ../file12 ) && + ( yes "l" | git mergetool ../submod ) && test "$(cat ../file1)" = "master updated" && test "$(cat ../file2)" = "master new" && test "$(cat ../submod/bar)" = "branch1 submodule" && @@ -209,9 +209,9 @@ test_expect_success 'mergetool skips autoresolved' ' git submodule update -N && test_must_fail git merge master && test -n "$(git ls-files -u)" && - ( yes "d" | git mergetool file11 >/dev/null 2>&1 ) && - ( yes "d" | git mergetool file12 >/dev/null 2>&1 ) && - ( yes "l" | git mergetool submod >/dev/null 2>&1 ) && + ( yes "d" | git mergetool file11 ) && + ( yes "d" | git mergetool file12 ) && + ( yes "l" | git mergetool submod ) && output="$(git mergetool --no-prompt)" && test "$output" = "No files need merging" ' @@ -259,9 +259,9 @@ test_expect_success 'mergetool skips resolved paths when rerere is active' ' rm -rf .git/rr-cache && git checkout -b test$test_count branch1 && git submodule update -N && - test_must_fail git merge master >/dev/null 2>&1 && - ( yes "l" | git mergetool --no-prompt submod >/dev/null 2>&1 ) && - ( yes "d" "d" | git mergetool --no-prompt >/dev/null 2>&1 ) && + test_must_fail git merge master && + ( yes "l" | git mergetool --no-prompt submod ) && + ( yes "d" "d" | git mergetool --no-prompt ) && git submodule update -N && output="$(yes "n" | git mergetool --no-prompt)" && test "$output" = "No files need merging" @@ -369,9 +369,9 @@ test_expect_success 'deleted vs modified submodule' ' git checkout -b test$test_count.a test$test_count && test_must_fail git merge master && test -n "$(git ls-files -u)" && - ( yes "" | git mergetool file1 file2 spaced\ name subdir/file3 >/dev/null 2>&1 ) && - ( yes "" | git mergetool both >/dev/null 2>&1 ) && - ( yes "d" | git mergetool file11 file12 >/dev/null 2>&1 ) && + ( yes "" | git mergetool file1 file2 spaced\ name subdir/file3 ) && + ( yes "" | git mergetool both ) && + ( yes "d" | git mergetool file11 file12 ) && ( yes "r" | git mergetool submod ) && rmdir submod && mv submod-movedaside submod && test "$(cat submod/bar)" = "branch1 submodule" && @@ -386,9 +386,9 @@ test_expect_success 'deleted vs modified submodule' ' git submodule update -N && test_must_fail git merge master && test -n "$(git ls-files -u)" && - ( yes "" | git mergetool file1 file2 spaced\ name subdir/file3 >/dev/null 2>&1 ) && - ( yes "" | git mergetool both >/dev/null 2>&1 ) && - ( yes "d" | git mergetool file11 file12 >/dev/null 2>&1 ) && + ( yes "" | git mergetool file1 file2 spaced\ name subdir/file3 ) && + ( yes "" | git mergetool both ) && + ( yes "d" | git mergetool file11 file12 ) && ( yes "l" | git mergetool submod ) && test ! -e submod && output="$(git mergetool --no-prompt)" && @@ -400,9 +400,9 @@ test_expect_success 'deleted vs modified submodule' ' git submodule update -N && test_must_fail git merge test$test_count && test -n "$(git ls-files -u)" && - ( yes "" | git mergetool file1 file2 spaced\ name subdir/file3 >/dev/null 2>&1 ) && - ( yes "" | git mergetool both >/dev/null 2>&1 ) && - ( yes "d" | git mergetool file11 file12 >/dev/null 2>&1 ) && + ( yes "" | git mergetool file1 file2 spaced\ name subdir/file3 ) && + ( yes "" | git mergetool both ) && + ( yes "d" | git mergetool file11 file12 ) && ( yes "r" | git mergetool submod ) && test ! -e submod && test -d submod.orig && @@ -416,9 +416,9 @@ test_expect_success 'deleted vs modified submodule' ' git submodule update -N && test_must_fail git merge test$test_count && test -n "$(git ls-files -u)" && - ( yes "" | git mergetool file1 file2 spaced\ name subdir/file3 >/dev/null 2>&1 ) && - ( yes "" | git mergetool both >/dev/null 2>&1 ) && - ( yes "d" | git mergetool file11 file12 >/dev/null 2>&1 ) && + ( yes "" | git mergetool file1 file2 spaced\ name subdir/file3 ) && + ( yes "" | git mergetool both ) && + ( yes "d" | git mergetool file11 file12 ) && ( yes "l" | git mergetool submod ) && test "$(cat submod/bar)" = "master submodule" && git submodule update -N && @@ -440,9 +440,9 @@ test_expect_success 'file vs modified submodule' ' git checkout -b test$test_count.a branch1 && test_must_fail git merge master && test -n "$(git ls-files -u)" && - ( yes "" | git mergetool file1 file2 spaced\ name subdir/file3 >/dev/null 2>&1 ) && - ( yes "" | git mergetool both >/dev/null 2>&1 ) && - ( yes "d" | git mergetool file11 file12 >/dev/null 2>&1 ) && + ( yes "" | git mergetool file1 file2 spaced\ name subdir/file3 ) && + ( yes "" | git mergetool both ) && + ( yes "d" | git mergetool file11 file12 ) && ( yes "r" | git mergetool submod ) && rmdir submod && mv submod-movedaside submod && test "$(cat submod/bar)" = "branch1 submodule" && @@ -456,9 +456,9 @@ test_expect_success 'file vs modified submodule' ' git checkout -b test$test_count.b test$test_count && test_must_fail git merge master && test -n "$(git ls-files -u)" && - ( yes "" | git mergetool file1 file2 spaced\ name subdir/file3 >/dev/null 2>&1 ) && - ( yes "" | git mergetool both >/dev/null 2>&1 ) && - ( yes "d" | git mergetool file11 file12 >/dev/null 2>&1 ) && + ( yes "" | git mergetool file1 file2 spaced\ name subdir/file3 ) && + ( yes "" | git mergetool both ) && + ( yes "d" | git mergetool file11 file12 ) && ( yes "l" | git mergetool submod ) && git submodule update -N && test "$(cat submod)" = "not a submodule" && @@ -472,9 +472,9 @@ test_expect_success 'file vs modified submodule' ' git submodule update -N && test_must_fail git merge test$test_count && test -n "$(git ls-files -u)" && - ( yes "" | git mergetool file1 file2 spaced\ name subdir/file3 >/dev/null 2>&1 ) && - ( yes "" | git mergetool both >/dev/null 2>&1 ) && - ( yes "d" | git mergetool file11 file12 >/dev/null 2>&1 ) && + ( yes "" | git mergetool file1 file2 spaced\ name subdir/file3 ) && + ( yes "" | git mergetool both ) && + ( yes "d" | git mergetool file11 file12 ) && ( yes "r" | git mergetool submod ) && test -d submod.orig && git submodule update -N && @@ -488,9 +488,9 @@ test_expect_success 'file vs modified submodule' ' git submodule update -N && test_must_fail git merge test$test_count && test -n "$(git ls-files -u)" && - ( yes "" | git mergetool file1 file2 spaced\ name subdir/file3 >/dev/null 2>&1 ) && - ( yes "" | git mergetool both>/dev/null 2>&1 ) && - ( yes "d" | git mergetool file11 file12 >/dev/null 2>&1 ) && + ( yes "" | git mergetool file1 file2 spaced\ name subdir/file3 ) && + ( yes "" | git mergetool both ) && + ( yes "d" | git mergetool file11 file12 ) && ( yes "l" | git mergetool submod ) && test "$(cat submod/bar)" = "master submodule" && git submodule update -N && @@ -543,7 +543,7 @@ test_expect_success 'submodule in subdirectory' ' git add subdir/subdir_module && git commit -m "change submodule in subdirectory on test$test_count.b" && - test_must_fail git merge test$test_count.a >/dev/null 2>&1 && + test_must_fail git merge test$test_count.a && ( cd subdir && ( yes "l" | git mergetool subdir_module ) @@ -554,7 +554,7 @@ test_expect_success 'submodule in subdirectory' ' git reset --hard && git submodule update -N && - test_must_fail git merge test$test_count.a >/dev/null 2>&1 && + test_must_fail git merge test$test_count.a && ( yes "r" | git mergetool subdir/subdir_module ) && test "$(cat subdir/subdir_module/file15)" = "test$test_count.b" && git submodule update -N && @@ -641,7 +641,7 @@ test_expect_success 'filenames seen by tools start with ./' ' test_config mergetool.myecho.trustExitCode true && test_must_fail git merge master && git mergetool --no-prompt --tool myecho -- both >actual && - grep ^\./both_LOCAL_ actual >/dev/null + grep ^\./both_LOCAL_ actual ' test_lazy_prereq MKTEMP ' @@ -658,8 +658,8 @@ test_expect_success MKTEMP 'temporary filenames are used with mergetool.writeToT test_config mergetool.myecho.trustExitCode true && test_must_fail git merge master && git mergetool --no-prompt --tool myecho -- both >actual && - ! grep ^\./both_LOCAL_ actual >/dev/null && - grep /both_LOCAL_ actual >/dev/null + ! grep ^\./both_LOCAL_ actual && + grep /both_LOCAL_ actual ' test_expect_success 'diff.orderFile configuration is honored' ' -- cgit v0.10.2-6-g49f6 From 57d93c1d2ce7deddf485e7c826278fef6815ec6b Mon Sep 17 00:00:00 2001 From: Denton Liu Date: Wed, 24 Apr 2019 15:46:59 -0700 Subject: t7610: add mergetool --gui tests In 063f2bdbf7 (mergetool: accept -g/--[no-]gui as arguments, 2018-10-24), mergetool was taught the --gui option but no tests were added to ensure that it was working properly. Add a test to ensure that it works. Signed-off-by: Denton Liu Signed-off-by: Junio C Hamano diff --git a/t/t7610-mergetool.sh b/t/t7610-mergetool.sh index 6971148..dad607e 100755 --- a/t/t7610-mergetool.sh +++ b/t/t7610-mergetool.sh @@ -145,6 +145,28 @@ test_expect_success 'custom mergetool' ' git commit -m "branch1 resolved with mergetool" ' +test_expect_success 'gui mergetool' ' + test_config merge.guitool myguitool && + test_config mergetool.myguitool.cmd "(printf \"gui \" && cat \"\$REMOTE\") >\"\$MERGED\"" && + test_config mergetool.myguitool.trustExitCode true && + test_when_finished "git reset --hard" && + git checkout -b test$test_count branch1 && + git submodule update -N && + test_must_fail git merge master && + ( yes "" | git mergetool --gui both ) && + ( yes "" | git mergetool -g file1 file1 ) && + ( yes "" | git mergetool --gui file2 "spaced name" ) && + ( yes "" | git mergetool --gui subdir/file3 ) && + ( yes "d" | git mergetool --gui file11 ) && + ( yes "d" | git mergetool --gui file12 ) && + ( yes "l" | git mergetool --gui submod ) && + test "$(cat file1)" = "gui master updated" && + test "$(cat file2)" = "gui master new" && + test "$(cat subdir/file3)" = "gui master new sub" && + test "$(cat submod/bar)" = "branch1 submodule" && + git commit -m "branch1 resolved with mergetool" +' + test_expect_success 'mergetool crlf' ' test_when_finished "git reset --hard" && # This test_config line must go after the above reset line so that -- cgit v0.10.2-6-g49f6 From 05fb8726cccc74908853c166248ff9b6abdafae5 Mon Sep 17 00:00:00 2001 From: Denton Liu Date: Mon, 29 Apr 2019 02:21:08 -0400 Subject: mergetool: use get_merge_tool function In git-mergetool, the logic for getting which merge tool to use is duplicated in git-mergetool--lib, except for the fact that it needs to know whether the tool was guessed or not. Rewrite `get_merge_tool` to return whether or not the tool was guessed through the return code and make git-mergetool call this function instead of duplicating the logic. Note that 1 was chosen to be the return code of when a tool is guessed because it seems like a slightly more abnormal condition than getting a tool that's explicitly specified but this is completely arbitrary. Also, let `$GIT_MERGETOOL_GUI` be set to determine whether or not the guitool will be selected. This change is not completely backwards compatible as there may be external users of git-mergetool--lib. However, only one user, git-diffall[1], was found from searching GitHub and Google, and this tool is superseded by `git difftool --dir-diff` anyway. It seems very unlikely that there exists an external caller that would take into account the return code of `get_merge_tool` as it would always return 0 before this change so this change probably does not affect any external users. [1]: https://github.com/thenigan/git-diffall Signed-off-by: Denton Liu Signed-off-by: Junio C Hamano diff --git a/Documentation/git-mergetool--lib.txt b/Documentation/git-mergetool--lib.txt index 055550b..4da9d24 100644 --- a/Documentation/git-mergetool--lib.txt +++ b/Documentation/git-mergetool--lib.txt @@ -28,7 +28,9 @@ to define the operation mode for the functions listed below. FUNCTIONS --------- get_merge_tool:: - returns a merge tool. + returns a merge tool. the return code is 1 if we returned a guessed + merge tool, else 0. '$GIT_MERGETOOL_GUI' may be set to 'true' to + search for the appropriate guitool. get_merge_tool_cmd:: returns the custom command for a merge tool. diff --git a/git-difftool--helper.sh b/git-difftool--helper.sh index 7bfb673..46af3e6 100755 --- a/git-difftool--helper.sh +++ b/git-difftool--helper.sh @@ -71,7 +71,7 @@ then then merge_tool="$GIT_DIFF_TOOL" else - merge_tool="$(get_merge_tool)" || exit + merge_tool="$(get_merge_tool)" fi fi diff --git a/git-mergetool--lib.sh b/git-mergetool--lib.sh index 83bf524..b928179 100644 --- a/git-mergetool--lib.sh +++ b/git-mergetool--lib.sh @@ -403,14 +403,17 @@ get_merge_tool_path () { } get_merge_tool () { + is_guessed=false # Check if a merge tool has been configured - merge_tool=$(get_configured_merge_tool) + merge_tool=$(get_configured_merge_tool $GIT_MERGETOOL_GUI) # Try to guess an appropriate merge tool if no tool has been set. if test -z "$merge_tool" then merge_tool=$(guess_merge_tool) || exit + is_guessed=true fi echo "$merge_tool" + test "$is_guessed" = false } mergetool_find_win32_cmd () { diff --git a/git-mergetool.sh b/git-mergetool.sh index 01b9ad5..88fa6a9 100755 --- a/git-mergetool.sh +++ b/git-mergetool.sh @@ -389,7 +389,7 @@ print_noop_and_exit () { main () { prompt=$(git config --bool mergetool.prompt) - gui_tool=false + GIT_MERGETOOL_GUI=false guessed_merge_tool=false orderfile= @@ -416,10 +416,10 @@ main () { esac ;; --no-gui) - gui_tool=false + GIT_MERGETOOL_GUI=false ;; -g|--gui) - gui_tool=true + GIT_MERGETOOL_GUI=true ;; -y|--no-prompt) prompt=false @@ -449,12 +449,8 @@ main () { if test -z "$merge_tool" then - # Check if a merge tool has been configured - merge_tool=$(get_configured_merge_tool $gui_tool) - # Try to guess an appropriate merge tool if no tool has been set. - if test -z "$merge_tool" + if ! merge_tool=$(get_merge_tool) then - merge_tool=$(guess_merge_tool) || exit guessed_merge_tool=true fi fi -- cgit v0.10.2-6-g49f6 From 884630b2e2634969656faabc7f4e33cab2e32b35 Mon Sep 17 00:00:00 2001 From: Denton Liu Date: Mon, 29 Apr 2019 02:21:11 -0400 Subject: mergetool--lib: create gui_mode function Before, in `get_configured_merge_tool`, we would test the value of the first argument directly, which corresponded to whether we were using guitool. However, since `$GIT_MERGETOOL_GUI` is available as an environment variable, create the `gui_mode` function which increases the clarify of functions which use it. While we're at it, add a space before `()` in function definitions to fix the style. Signed-off-by: Denton Liu Signed-off-by: Junio C Hamano diff --git a/git-mergetool--lib.sh b/git-mergetool--lib.sh index b928179..4ca170c 100644 --- a/git-mergetool--lib.sh +++ b/git-mergetool--lib.sh @@ -80,14 +80,18 @@ show_tool_names () { } } -diff_mode() { +diff_mode () { test "$TOOL_MODE" = diff } -merge_mode() { +merge_mode () { test "$TOOL_MODE" = merge } +gui_mode () { + test "$GIT_MERGETOOL_GUI" = true +} + translate_merge_tool_path () { echo "$1" } @@ -350,8 +354,7 @@ guess_merge_tool () { } get_configured_merge_tool () { - # If first argument is true, find the guitool instead - if test "$1" = true + if gui_mode then gui_prefix=gui fi @@ -405,7 +408,7 @@ get_merge_tool_path () { get_merge_tool () { is_guessed=false # Check if a merge tool has been configured - merge_tool=$(get_configured_merge_tool $GIT_MERGETOOL_GUI) + merge_tool=$(get_configured_merge_tool) # Try to guess an appropriate merge tool if no tool has been set. if test -z "$merge_tool" then -- cgit v0.10.2-6-g49f6 From 60aced3dfa68df60952fed28c4ae63a5bbda0275 Mon Sep 17 00:00:00 2001 From: Denton Liu Date: Mon, 29 Apr 2019 02:21:14 -0400 Subject: mergetool: fallback to tool when guitool unavailable In git-difftool, if the tool is called with --gui but `diff.guitool` is not set, it falls back to `diff.tool`. Make git-mergetool also fallback from `merge.guitool` to `merge.tool` if the former is undefined. If git-difftool, when called with `--gui`, were to use `get_configured_mergetool` in a future patch, it would also get the fallback behavior in the following precedence: 1. diff.guitool 2. merge.guitool 3. diff.tool 4. merge.tool The behavior for when difftool or mergetool are called without `--gui` should be identical with or without this patch. Note that the search loop could be written as sections="merge" keys="tool" if diff_mode then sections="diff $sections" fi if gui_mode then keys="guitool $keys" fi merge_tool=$( IFS=' ' for key in $keys do for section in $sections do selected=$(git config $section.$key) if test -n "$selected" then echo "$selected" return fi done done) which would make adding a mode in the future much easier. However, adding a new mode will likely never happen as it is highly discouraged so, as a result, it is written in its current form so that it is more readable for future readers. Signed-off-by: Denton Liu Signed-off-by: Junio C Hamano diff --git a/Documentation/git-mergetool.txt b/Documentation/git-mergetool.txt index 0c7975a..6b14702 100644 --- a/Documentation/git-mergetool.txt +++ b/Documentation/git-mergetool.txt @@ -83,7 +83,9 @@ success of the resolution after the custom tool has exited. --gui:: When 'git-mergetool' is invoked with the `-g` or `--gui` option the default merge tool will be read from the configured - `merge.guitool` variable instead of `merge.tool`. + `merge.guitool` variable instead of `merge.tool`. If + `merge.guitool` is not set, we will fallback to the tool + configured under `merge.tool`. --no-gui:: This overrides a previous `-g` or `--gui` setting and reads the diff --git a/git-mergetool--lib.sh b/git-mergetool--lib.sh index 4ca170c..696eb49 100644 --- a/git-mergetool--lib.sh +++ b/git-mergetool--lib.sh @@ -354,19 +354,36 @@ guess_merge_tool () { } get_configured_merge_tool () { - if gui_mode - 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 + keys= if diff_mode then - merge_tool=$(git config diff.${gui_prefix}tool || git config merge.${gui_prefix}tool) + if gui_mode + then + keys="diff.guitool merge.guitool diff.tool merge.tool" + else + keys="diff.tool merge.tool" + fi else - merge_tool=$(git config merge.${gui_prefix}tool) + if gui_mode + then + keys="merge.guitool merge.tool" + else + keys="merge.tool" + fi fi + + merge_tool=$( + IFS=' ' + for key in $keys + do + selected=$(git config $key) + if test -n "$selected" + then + echo "$selected" + return + fi + done) + if test -n "$merge_tool" && ! valid_tool "$merge_tool" then echo >&2 "git config option $TOOL_MODE.${gui_prefix}tool set to unknown tool: $merge_tool" diff --git a/t/t7610-mergetool.sh b/t/t7610-mergetool.sh index dad607e..5b61c10 100755 --- a/t/t7610-mergetool.sh +++ b/t/t7610-mergetool.sh @@ -167,6 +167,25 @@ test_expect_success 'gui mergetool' ' git commit -m "branch1 resolved with mergetool" ' +test_expect_success 'gui mergetool without merge.guitool set falls back to merge.tool' ' + test_when_finished "git reset --hard" && + git checkout -b test$test_count branch1 && + git submodule update -N && + test_must_fail git merge master && + ( yes "" | git mergetool --gui both ) && + ( yes "" | git mergetool -g file1 file1 ) && + ( yes "" | git mergetool --gui file2 "spaced name" ) && + ( yes "" | git mergetool --gui subdir/file3 ) && + ( yes "d" | git mergetool --gui file11 ) && + ( yes "d" | git mergetool --gui file12 ) && + ( yes "l" | git mergetool --gui submod ) && + test "$(cat file1)" = "master updated" && + test "$(cat file2)" = "master new" && + test "$(cat subdir/file3)" = "master new sub" && + test "$(cat submod/bar)" = "branch1 submodule" && + git commit -m "branch1 resolved with mergetool" +' + test_expect_success 'mergetool crlf' ' test_when_finished "git reset --hard" && # This test_config line must go after the above reset line so that -- cgit v0.10.2-6-g49f6 From 7f978d7d10a87c4a56ea3101b936cddb25bbe2c6 Mon Sep 17 00:00:00 2001 From: Denton Liu Date: Mon, 29 Apr 2019 02:21:17 -0400 Subject: difftool: make --gui, --tool and --extcmd mutually exclusive In git-difftool, these options specify which tool to ultimately run. As a result, they are logically conflicting. Explicitly disallow these options from being used together. Signed-off-by: Denton Liu Signed-off-by: Junio C Hamano diff --git a/builtin/difftool.c b/builtin/difftool.c index 544b0e8..481aa64 100644 --- a/builtin/difftool.c +++ b/builtin/difftool.c @@ -730,6 +730,9 @@ int cmd_difftool(int argc, const char **argv, const char *prefix) setenv(GIT_DIR_ENVIRONMENT, absolute_path(get_git_dir()), 1); setenv(GIT_WORK_TREE_ENVIRONMENT, absolute_path(get_git_work_tree()), 1); + if (use_gui_tool + !!difftool_cmd + !!extcmd > 1) + die(_("--gui, --tool and --extcmd are mutually exclusive")); + if (use_gui_tool && diff_gui_tool && *diff_gui_tool) setenv("GIT_DIFF_TOOL", diff_gui_tool, 1); else if (difftool_cmd) { diff --git a/t/t7800-difftool.sh b/t/t7800-difftool.sh index 562bd21..833e175 100755 --- a/t/t7800-difftool.sh +++ b/t/t7800-difftool.sh @@ -705,4 +705,12 @@ test_expect_success SYMLINKS 'difftool --dir-diff handles modified symlinks' ' test_cmp expect actual ' +test_expect_success 'difftool --gui, --tool and --extcmd are mutually exclusive' ' + difftool_test_setup && + test_must_fail git difftool --gui --tool=test-tool && + test_must_fail git difftool --gui --extcmd=cat && + test_must_fail git difftool --tool=test-tool --extcmd=cat && + test_must_fail git difftool --gui --tool=test-tool --extcmd=cat +' + test_done -- cgit v0.10.2-6-g49f6 From 6c22d715e7b4067a6865ff3fbceab991d0042c12 Mon Sep 17 00:00:00 2001 From: Denton Liu Date: Mon, 29 Apr 2019 02:21:20 -0400 Subject: difftool: fallback on merge.guitool In git-difftool.txt, it says 'git difftool' falls back to 'git mergetool' config variables when the difftool equivalents have not been defined. However, when `diff.guitool` is missing, it doesn't fallback to anything. Make git-difftool fallback to `merge.guitool` when `diff.guitool` is missing. Signed-off-by: Denton Liu Signed-off-by: Junio C Hamano diff --git a/Documentation/git-difftool.txt b/Documentation/git-difftool.txt index 96c26e6..484c485 100644 --- a/Documentation/git-difftool.txt +++ b/Documentation/git-difftool.txt @@ -90,7 +90,9 @@ instead. `--no-symlinks` is the default on Windows. When 'git-difftool' is invoked with the `-g` or `--gui` option the default diff tool will be read from the configured `diff.guitool` variable instead of `diff.tool`. The `--no-gui` - option can be used to override this setting. + option can be used to override this setting. If `diff.guitool` + is not set, we will fallback in the order of `merge.guitool`, + `diff.tool`, `merge.tool` until a tool is found. --[no-]trust-exit-code:: 'git-difftool' invokes a diff tool individually on each file. diff --git a/builtin/difftool.c b/builtin/difftool.c index 481aa64..2205de1 100644 --- a/builtin/difftool.c +++ b/builtin/difftool.c @@ -23,7 +23,6 @@ #include "object-store.h" #include "dir.h" -static char *diff_gui_tool; static int trust_exit_code; static const char *const builtin_difftool_usage[] = { @@ -33,11 +32,6 @@ static const char *const builtin_difftool_usage[] = { static int difftool_config(const char *var, const char *value, void *cb) { - if (!strcmp(var, "diff.guitool")) { - diff_gui_tool = xstrdup(value); - return 0; - } - if (!strcmp(var, "difftool.trustexitcode")) { trust_exit_code = git_config_bool(var, value); return 0; @@ -733,8 +727,8 @@ int cmd_difftool(int argc, const char **argv, const char *prefix) if (use_gui_tool + !!difftool_cmd + !!extcmd > 1) die(_("--gui, --tool and --extcmd are mutually exclusive")); - if (use_gui_tool && diff_gui_tool && *diff_gui_tool) - setenv("GIT_DIFF_TOOL", diff_gui_tool, 1); + if (use_gui_tool) + setenv("GIT_MERGETOOL_GUI", "true", 1); else if (difftool_cmd) { if (*difftool_cmd) setenv("GIT_DIFF_TOOL", difftool_cmd, 1); diff --git a/t/t7800-difftool.sh b/t/t7800-difftool.sh index 833e175..48b74c0 100755 --- a/t/t7800-difftool.sh +++ b/t/t7800-difftool.sh @@ -279,11 +279,27 @@ test_expect_success 'difftool + mergetool config variables' ' echo branch >expect && git difftool --no-prompt branch >actual && test_cmp expect actual && + git difftool --gui --no-prompt branch >actual && + test_cmp expect actual && # set merge.tool to something bogus, diff.tool to test-tool test_config merge.tool bogus-tool && test_config diff.tool test-tool && git difftool --no-prompt branch >actual && + test_cmp expect actual && + git difftool --gui --no-prompt branch >actual && + test_cmp expect actual && + + # set merge.tool, diff.tool to something bogus, merge.guitool to test-tool + test_config diff.tool bogus-tool && + test_config merge.guitool test-tool && + git difftool --gui --no-prompt branch >actual && + test_cmp expect actual && + + # set merge.tool, diff.tool, merge.guitool to something bogus, diff.guitool to test-tool + test_config merge.guitool bogus-tool && + test_config diff.guitool test-tool && + git difftool --gui --no-prompt branch >actual && test_cmp expect actual ' -- cgit v0.10.2-6-g49f6