* [PATCH 0/5] difftool and mergetool improvements
@ 2019-04-22 5:07 Denton Liu
2019-04-22 5:07 ` [PATCH 1/5] t7610: add mergetool --gui tests Denton Liu
` (5 more replies)
0 siblings, 6 replies; 48+ messages in thread
From: Denton Liu @ 2019-04-22 5:07 UTC (permalink / raw)
To: Git Mailing List; +Cc: Johannes Schindelin, David Aguilar
I noticed earlier that when running 'git difftool --gui', we do not
fallback to 'merge.guitool' when 'diff.guitool' isn't set, even though
this behaviour exists for when we invoke 'git difftool' (i.e. it falls
back from diff.tool to merge.tool).
While fixing this bug up, I noticed a few other places where we could do
some code cleanup/add tests so I did that as well.
Denton Liu (5):
t7610: add mergetool --gui tests
mergetool: use get_merge_tool function
mergetool: fallback to tool when guitool unavailable
difftool: make --gui, --tool and --extcmd exclusive
difftool: fallback on merge.guitool
Documentation/git-difftool.txt | 4 ++-
Documentation/git-mergetool--lib.txt | 5 +++-
Documentation/git-mergetool.txt | 4 ++-
builtin/difftool.c | 21 ++++++++------
git-difftool--helper.sh | 2 +-
git-mergetool--lib.sh | 33 ++++++++++++++--------
git-mergetool.sh | 11 ++------
t/t7610-mergetool.sh | 41 ++++++++++++++++++++++++++++
t/t7800-difftool.sh | 24 ++++++++++++++++
9 files changed, 113 insertions(+), 32 deletions(-)
--
2.21.0.967.gf85e14fd49
^ permalink raw reply [flat|nested] 48+ messages in thread* [PATCH 1/5] t7610: add mergetool --gui tests 2019-04-22 5:07 [PATCH 0/5] difftool and mergetool improvements Denton Liu @ 2019-04-22 5:07 ` Denton Liu 2019-04-22 5:07 ` [PATCH 2/5] mergetool: use get_merge_tool function Denton Liu ` (4 subsequent siblings) 5 siblings, 0 replies; 48+ messages in thread From: Denton Liu @ 2019-04-22 5:07 UTC (permalink / raw) To: Git Mailing List; +Cc: Johannes Schindelin, David Aguilar 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 <liu.denton@gmail.com> --- t/t7610-mergetool.sh | 22 ++++++++++++++++++++++ 1 file changed, 22 insertions(+) diff --git a/t/t7610-mergetool.sh b/t/t7610-mergetool.sh index a9fb971615..5f37d7a1ff 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 >/dev/null 2>&1 && + ( yes "" | git mergetool --gui 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 ) && + ( yes "d" | git mergetool --gui file11 >/dev/null 2>&1 ) && + ( yes "d" | git mergetool --gui file12 >/dev/null 2>&1 ) && + ( yes "l" | git mergetool --gui submod >/dev/null 2>&1 ) && + 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 -- 2.21.0.967.gf85e14fd49 ^ permalink raw reply related [flat|nested] 48+ messages in thread
* [PATCH 2/5] mergetool: use get_merge_tool function 2019-04-22 5:07 [PATCH 0/5] difftool and mergetool improvements Denton Liu 2019-04-22 5:07 ` [PATCH 1/5] t7610: add mergetool --gui tests Denton Liu @ 2019-04-22 5:07 ` Denton Liu 2019-04-22 7:07 ` Eric Sunshine 2019-04-22 5:07 ` [PATCH 3/5] mergetool: fallback to tool when guitool unavailable Denton Liu ` (3 subsequent siblings) 5 siblings, 1 reply; 48+ messages in thread From: Denton Liu @ 2019-04-22 5:07 UTC (permalink / raw) To: Git Mailing List; +Cc: Johannes Schindelin, David Aguilar 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 and make git-mergetool call this function instead of duplicating the logic. Also, let `$GIT_MERGETOOL_GUI` be set to determine whether or not the guitool will be selected. Signed-off-by: Denton Liu <liu.denton@gmail.com> --- Documentation/git-mergetool--lib.txt | 5 ++++- git-difftool--helper.sh | 2 +- git-mergetool--lib.sh | 6 ++++-- git-mergetool.sh | 11 +++-------- 4 files changed, 12 insertions(+), 12 deletions(-) diff --git a/Documentation/git-mergetool--lib.txt b/Documentation/git-mergetool--lib.txt index 055550b2bc..c4f10209e0 100644 --- a/Documentation/git-mergetool--lib.txt +++ b/Documentation/git-mergetool--lib.txt @@ -28,7 +28,10 @@ to define the operation mode for the functions listed below. FUNCTIONS --------- get_merge_tool:: - returns a merge tool. + returns '$is_guessed:$merge_tool'. '$is_guessed' is 'true' if + the tool was guessed, else 'false'. '$merge_tool' is the merge + tool to use. '$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 7bfb6737df..78a0446668 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 | sed -e 's/^[a-z]*://')" || exit fi fi diff --git a/git-mergetool--lib.sh b/git-mergetool--lib.sh index 83bf52494c..d5e2c6c5c6 100644 --- a/git-mergetool--lib.sh +++ b/git-mergetool--lib.sh @@ -403,14 +403,16 @@ 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" + echo "$is_guessed:$merge_tool" } mergetool_find_win32_cmd () { diff --git a/git-mergetool.sh b/git-mergetool.sh index 01b9ad59b2..6ad8024e46 100755 --- a/git-mergetool.sh +++ b/git-mergetool.sh @@ -449,14 +449,9 @@ 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" - then - merge_tool=$(guess_merge_tool) || exit - guessed_merge_tool=true - fi + IFS=':' read guessed_merge_tool merge_tool <<-EOF + $(GIT_MERGETOOL_GUI=$gui_tool get_merge_tool) + EOF fi merge_keep_backup="$(git config --bool mergetool.keepBackup || echo true)" merge_keep_temporaries="$(git config --bool mergetool.keepTemporaries || echo false)" -- 2.21.0.967.gf85e14fd49 ^ permalink raw reply related [flat|nested] 48+ messages in thread
* Re: [PATCH 2/5] mergetool: use get_merge_tool function 2019-04-22 5:07 ` [PATCH 2/5] mergetool: use get_merge_tool function Denton Liu @ 2019-04-22 7:07 ` Eric Sunshine 2019-04-22 8:35 ` Denton Liu 0 siblings, 1 reply; 48+ messages in thread From: Eric Sunshine @ 2019-04-22 7:07 UTC (permalink / raw) To: Denton Liu; +Cc: Git Mailing List, Johannes Schindelin, David Aguilar On Mon, Apr 22, 2019 at 1:07 AM Denton Liu <liu.denton@gmail.com> wrote: > [...] > Rewrite `get_merge_tool` to return whether or not the tool was guessed > and make git-mergetool call this function instead of duplicating the > logic. Also, let `$GIT_MERGETOOL_GUI` be set to determine whether or not > the guitool will be selected. > > Signed-off-by: Denton Liu <liu.denton@gmail.com> > --- > diff --git a/Documentation/git-mergetool--lib.txt b/Documentation/git-mergetool--lib.txt > @@ -28,7 +28,10 @@ to define the operation mode for the functions listed below. > get_merge_tool:: > - returns a merge tool. > + returns '$is_guessed:$merge_tool'. '$is_guessed' is 'true' if > + the tool was guessed, else 'false'. '$merge_tool' is the merge > + tool to use. '$GIT_MERGETOOL_GUI' may be set to 'true' to search > + for the appropriate guitool. What is the likelihood that code outside of our control is using this function? If there is such code, this backward-incompatible change will break that code. If the likelihood is excessively small, perhaps it is not worth worrying about, otherwise, perhaps this warrants a new function with a distinct name. ^ permalink raw reply [flat|nested] 48+ messages in thread
* Re: [PATCH 2/5] mergetool: use get_merge_tool function 2019-04-22 7:07 ` Eric Sunshine @ 2019-04-22 8:35 ` Denton Liu 0 siblings, 0 replies; 48+ messages in thread From: Denton Liu @ 2019-04-22 8:35 UTC (permalink / raw) To: Eric Sunshine; +Cc: Git Mailing List, Johannes Schindelin, David Aguilar Hi Eric, On Mon, Apr 22, 2019 at 03:07:25AM -0400, Eric Sunshine wrote: > On Mon, Apr 22, 2019 at 1:07 AM Denton Liu <liu.denton@gmail.com> wrote: > > [...] > > Rewrite `get_merge_tool` to return whether or not the tool was guessed > > and make git-mergetool call this function instead of duplicating the > > logic. Also, let `$GIT_MERGETOOL_GUI` be set to determine whether or not > > the guitool will be selected. > > > > Signed-off-by: Denton Liu <liu.denton@gmail.com> > > --- > > diff --git a/Documentation/git-mergetool--lib.txt b/Documentation/git-mergetool--lib.txt > > @@ -28,7 +28,10 @@ to define the operation mode for the functions listed below. > > get_merge_tool:: > > - returns a merge tool. > > + returns '$is_guessed:$merge_tool'. '$is_guessed' is 'true' if > > + the tool was guessed, else 'false'. '$merge_tool' is the merge > > + tool to use. '$GIT_MERGETOOL_GUI' may be set to 'true' to search > > + for the appropriate guitool. > > What is the likelihood that code outside of our control is using this > function? If there is such code, this backward-incompatible change > will break that code. If the likelihood is excessively small, perhaps > it is not worth worrying about, otherwise, perhaps this warrants a new > function with a distinct name. Thanks for considering this, I hadn't thought about it myself. I assumed this was an internal function but I guess I was wrong. I did a bit of digging on GitHub and Google and I found that git-diffall[1] uses it, although it seems quite old and unmaintained. Aside from this, I can't find any other open-source programs which use git-mergetool--lib (and in particular, get_merge_tool) so I believe that it is pretty rare. That being said, I'm open to writing a new function so that the change will be backwards compatible. I'll see what the list has to say. Thanks, Denton [1]: https://github.com/thenigan/git-diffall ^ permalink raw reply [flat|nested] 48+ messages in thread
* [PATCH 3/5] mergetool: fallback to tool when guitool unavailable 2019-04-22 5:07 [PATCH 0/5] difftool and mergetool improvements Denton Liu 2019-04-22 5:07 ` [PATCH 1/5] t7610: add mergetool --gui tests Denton Liu 2019-04-22 5:07 ` [PATCH 2/5] mergetool: use get_merge_tool function Denton Liu @ 2019-04-22 5:07 ` Denton Liu 2019-04-22 5:07 ` [PATCH 4/5] difftool: make --gui, --tool and --extcmd exclusive Denton Liu ` (2 subsequent siblings) 5 siblings, 0 replies; 48+ messages in thread From: Denton Liu @ 2019-04-22 5:07 UTC (permalink / raw) To: Git Mailing List; +Cc: Johannes Schindelin, David Aguilar 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 were to use `get_configured_mergetool`, it would also get the fallback behaviour in the following precedence: 1. diff.guitool 2. merge.guitool 3. diff.tool 4. merge.tool Signed-off-by: Denton Liu <liu.denton@gmail.com> --- Documentation/git-mergetool.txt | 4 +++- git-mergetool--lib.sh | 27 ++++++++++++++++++--------- t/t7610-mergetool.sh | 19 +++++++++++++++++++ 3 files changed, 40 insertions(+), 10 deletions(-) diff --git a/Documentation/git-mergetool.txt b/Documentation/git-mergetool.txt index 0c7975a050..6b14702e78 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 d5e2c6c5c6..68a85f4a7b 100644 --- a/git-mergetool--lib.sh +++ b/git-mergetool--lib.sh @@ -350,20 +350,29 @@ guess_merge_tool () { } get_configured_merge_tool () { - # If first argument is true, find the guitool instead - if test "$1" = true + is_gui="$1" + sections="merge" + keys="tool" + + if diff_mode then - gui_prefix=gui + sections="diff $sections" 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 + if "$is_gui" = true then - merge_tool=$(git config diff.${gui_prefix}tool || git config merge.${gui_prefix}tool) - else - merge_tool=$(git config merge.${gui_prefix}tool) + keys="guitool $keys" fi + + IFS=' ' + for key in $keys + do + for section in $sections + do + merge_tool=$(git config $section.$key) && break 2 + done + 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 5f37d7a1ff..bc2c9eaa30 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 fallsback 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 >/dev/null 2>&1 && + ( yes "" | git mergetool --gui 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 ) && + ( yes "d" | git mergetool --gui file11 >/dev/null 2>&1 ) && + ( yes "d" | git mergetool --gui file12 >/dev/null 2>&1 ) && + ( yes "l" | git mergetool --gui submod >/dev/null 2>&1 ) && + 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 -- 2.21.0.967.gf85e14fd49 ^ permalink raw reply related [flat|nested] 48+ messages in thread
* [PATCH 4/5] difftool: make --gui, --tool and --extcmd exclusive 2019-04-22 5:07 [PATCH 0/5] difftool and mergetool improvements Denton Liu ` (2 preceding siblings ...) 2019-04-22 5:07 ` [PATCH 3/5] mergetool: fallback to tool when guitool unavailable Denton Liu @ 2019-04-22 5:07 ` Denton Liu 2019-04-22 7:03 ` Eric Sunshine 2019-04-22 5:07 ` [PATCH 5/5] difftool: fallback on merge.guitool Denton Liu 2019-04-23 8:53 ` [PATCH v2 0/5] difftool and mergetool improvements Denton Liu 5 siblings, 1 reply; 48+ messages in thread From: Denton Liu @ 2019-04-22 5:07 UTC (permalink / raw) To: Git Mailing List; +Cc: Johannes Schindelin, David Aguilar 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 <liu.denton@gmail.com> --- builtin/difftool.c | 11 ++++++++++- t/t7800-difftool.sh | 8 ++++++++ 2 files changed, 18 insertions(+), 1 deletion(-) diff --git a/builtin/difftool.c b/builtin/difftool.c index a3ea60ea71..5ad39c9172 100644 --- a/builtin/difftool.c +++ b/builtin/difftool.c @@ -690,7 +690,7 @@ static int run_file_diff(int prompt, const char *prefix, int cmd_difftool(int argc, const char **argv, const char *prefix) { int use_gui_tool = 0, dir_diff = 0, prompt = -1, symlinks = 0, - tool_help = 0; + tool_help = 0, count = 0; static char *difftool_cmd = NULL, *extcmd = NULL; struct option builtin_difftool_options[] = { OPT_BOOL('g', "gui", &use_gui_tool, @@ -731,6 +731,15 @@ 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) + count++; + if (difftool_cmd) + count++; + if (extcmd) + count++; + if (count > 1) + die(_("--gui, --tool and --extcmd are 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 bb9a7f4ff9..107f31213d 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 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 -- 2.21.0.967.gf85e14fd49 ^ permalink raw reply related [flat|nested] 48+ messages in thread
* Re: [PATCH 4/5] difftool: make --gui, --tool and --extcmd exclusive 2019-04-22 5:07 ` [PATCH 4/5] difftool: make --gui, --tool and --extcmd exclusive Denton Liu @ 2019-04-22 7:03 ` Eric Sunshine 0 siblings, 0 replies; 48+ messages in thread From: Eric Sunshine @ 2019-04-22 7:03 UTC (permalink / raw) To: Denton Liu; +Cc: Git Mailing List, Johannes Schindelin, David Aguilar On Mon, Apr 22, 2019 at 1:07 AM Denton Liu <liu.denton@gmail.com> wrote: > 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 <liu.denton@gmail.com> > --- > diff --git a/builtin/difftool.c b/builtin/difftool.c > @@ -731,6 +731,15 @@ int cmd_difftool(int argc, const char **argv, const char *prefix) > + if (use_gui_tool) > + count++; > + if (difftool_cmd) > + count++; > + if (extcmd) > + count++; > + if (count > 1) > + die(_("--gui, --tool and --extcmd are exclusive")); The more typical way to check this condition in this codebase is: if (use_gui_tool + !!difftool_cmd + !!extcmd > 1) die(...); Also, I think you might want: s/exclusive/mutually &/ ^ permalink raw reply [flat|nested] 48+ messages in thread
* [PATCH 5/5] difftool: fallback on merge.guitool 2019-04-22 5:07 [PATCH 0/5] difftool and mergetool improvements Denton Liu ` (3 preceding siblings ...) 2019-04-22 5:07 ` [PATCH 4/5] difftool: make --gui, --tool and --extcmd exclusive Denton Liu @ 2019-04-22 5:07 ` Denton Liu 2019-04-22 18:18 ` Jeff Hostetler 2019-04-23 8:53 ` [PATCH v2 0/5] difftool and mergetool improvements Denton Liu 5 siblings, 1 reply; 48+ messages in thread From: Denton Liu @ 2019-04-22 5:07 UTC (permalink / raw) To: Git Mailing List; +Cc: Johannes Schindelin, David Aguilar 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 <liu.denton@gmail.com> --- Documentation/git-difftool.txt | 4 +++- builtin/difftool.c | 10 ++-------- t/t7800-difftool.sh | 16 ++++++++++++++++ 3 files changed, 21 insertions(+), 9 deletions(-) diff --git a/Documentation/git-difftool.txt b/Documentation/git-difftool.txt index 96c26e6aa8..484c485fd0 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 5ad39c9172..67f26502c5 100644 --- a/builtin/difftool.c +++ b/builtin/difftool.c @@ -24,7 +24,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[] = { @@ -34,11 +33,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; @@ -740,8 +734,8 @@ int cmd_difftool(int argc, const char **argv, const char *prefix) if (count > 1) die(_("--gui, --tool and --extcmd are 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 107f31213d..ae90701a12 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 ' -- 2.21.0.967.gf85e14fd49 ^ permalink raw reply related [flat|nested] 48+ messages in thread
* Re: [PATCH 5/5] difftool: fallback on merge.guitool 2019-04-22 5:07 ` [PATCH 5/5] difftool: fallback on merge.guitool Denton Liu @ 2019-04-22 18:18 ` Jeff Hostetler 2019-04-22 18:33 ` Denton Liu 0 siblings, 1 reply; 48+ messages in thread From: Jeff Hostetler @ 2019-04-22 18:18 UTC (permalink / raw) To: Denton Liu, Git Mailing List; +Cc: Johannes Schindelin, David Aguilar On 4/22/2019 1:07 AM, Denton Liu wrote: > 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. > Is this a well-defined operation? I mean, we're assuming here that a 3-way gui merge tool (that probably expects 3 input pathnames and maybe a 4th merge-result pathname (and associated titles and etc)) can function sanely when only given the pair that a diff would have. That is, we're assuming that the selected merge tool has a 2-way diff mode and that the command line args for the 2- and 3-way views are compatible. Just a thought Jeff ^ permalink raw reply [flat|nested] 48+ messages in thread
* Re: [PATCH 5/5] difftool: fallback on merge.guitool 2019-04-22 18:18 ` Jeff Hostetler @ 2019-04-22 18:33 ` Denton Liu 0 siblings, 0 replies; 48+ messages in thread From: Denton Liu @ 2019-04-22 18:33 UTC (permalink / raw) To: Jeff Hostetler; +Cc: Git Mailing List, Johannes Schindelin, David Aguilar Hi Jeff, On Mon, Apr 22, 2019 at 02:18:36PM -0400, Jeff Hostetler wrote: > > > On 4/22/2019 1:07 AM, Denton Liu wrote: > >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. > > > > Is this a well-defined operation? I believe this is a yes. > > I mean, we're assuming here that a 3-way gui merge tool (that probably > expects 3 input pathnames and maybe a 4th merge-result pathname (and > associated titles and etc)) can function sanely when only given the > pair that a diff would have. > > That is, we're assuming that the selected merge tool has a 2-way diff > mode and that the command line args for the 2- and 3-way views are > compatible. If I read the code correctly, it seems like the only tool that is strictly "merge-only" is tortoisemerge. In mergetools/tortoisemerge, we have can_diff () { return 1 } which means it will refuse to run as a difftool. In the case where it fails like this, it'll loudly complain to the user, which'll give them a chance to fix their config. I believe that this is desired behaviour and the patch adds on top of that. Thanks, Denton > > Just a thought > Jeff > > ^ permalink raw reply [flat|nested] 48+ messages in thread
* [PATCH v2 0/5] difftool and mergetool improvements 2019-04-22 5:07 [PATCH 0/5] difftool and mergetool improvements Denton Liu ` (4 preceding siblings ...) 2019-04-22 5:07 ` [PATCH 5/5] difftool: fallback on merge.guitool Denton Liu @ 2019-04-23 8:53 ` Denton Liu 2019-04-23 8:53 ` [PATCH v2 1/5] t7610: add mergetool --gui tests Denton Liu ` (5 more replies) 5 siblings, 6 replies; 48+ messages in thread From: Denton Liu @ 2019-04-23 8:53 UTC (permalink / raw) To: Git Mailing List Cc: Johannes Schindelin, David Aguilar, Jeff Hostetler, Eric Sunshine Thanks for the review, Eric. Hopefully, these changes have addressed the concerns that you've raised. --- Changes since v1: * Introduce get_merge_tool_guessed function instead of changing get_merge_tool * Remove unnecessary if-tower in mutual exclusivity logic Denton Liu (5): t7610: add mergetool --gui tests mergetool: use get_merge_tool_guessed function mergetool: fallback to tool when guitool unavailable difftool: make --gui, --tool and --extcmd mutually exclusive difftool: fallback on merge.guitool Documentation/git-difftool.txt | 4 ++- Documentation/git-mergetool--lib.txt | 9 +++++- Documentation/git-mergetool.txt | 4 ++- builtin/difftool.c | 13 ++++----- git-mergetool--lib.sh | 39 ++++++++++++++++++-------- git-mergetool.sh | 11 ++------ t/t7610-mergetool.sh | 41 ++++++++++++++++++++++++++++ t/t7800-difftool.sh | 24 ++++++++++++++++ 8 files changed, 114 insertions(+), 31 deletions(-) Range-diff against v1: 1: 678f9b11fc = 1: 678f9b11fc t7610: add mergetool --gui tests 2: 692875cf4b ! 2: e928db892e mergetool: use get_merge_tool function @@ -1,15 +1,19 @@ Author: Denton Liu <liu.denton@gmail.com> - mergetool: use get_merge_tool function + mergetool: use get_merge_tool_guessed 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 - and make git-mergetool call this function instead of duplicating the - logic. Also, let `$GIT_MERGETOOL_GUI` be set to determine whether or not - the guitool will be selected. + Write `get_merge_tool_guessed` to return whether or not the tool was + guessed in addition to the actual tool and make git-mergetool call this + function instead of duplicating the logic. Also, let + `$GIT_MERGETOOL_GUI` be set to determine whether or not the guitool will + be selected. + + Make `get_merge_tool` use this function internally so that code + duplication is reduced. Signed-off-by: Denton Liu <liu.denton@gmail.com> @@ -17,38 +21,32 @@ --- a/Documentation/git-mergetool--lib.txt +++ b/Documentation/git-mergetool--lib.txt @@ + FUNCTIONS --------- - get_merge_tool:: -- returns a merge tool. ++get_merge_tool_guessed:: + returns '$is_guessed:$merge_tool'. '$is_guessed' is 'true' if + the tool was guessed, else 'false'. '$merge_tool' is the merge + tool to use. '$GIT_MERGETOOL_GUI' may be set to 'true' to search + for the appropriate guitool. ++ + get_merge_tool:: +- returns a merge tool. ++ returns a merge tool. '$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 - --- a/git-difftool--helper.sh - +++ b/git-difftool--helper.sh -@@ - then - merge_tool="$GIT_DIFF_TOOL" - else -- merge_tool="$(get_merge_tool)" || exit -+ merge_tool="$(get_merge_tool | sed -e 's/^[a-z]*://')" || exit - fi - fi - - diff --git a/git-mergetool--lib.sh b/git-mergetool--lib.sh --- a/git-mergetool--lib.sh +++ b/git-mergetool--lib.sh @@ + echo "$merge_tool_path" } - get_merge_tool () { +-get_merge_tool () { ++get_merge_tool_guessed () { + is_guessed=false # Check if a merge tool has been configured - merge_tool=$(get_configured_merge_tool) @@ -61,6 +59,10 @@ fi - echo "$merge_tool" + echo "$is_guessed:$merge_tool" ++} ++ ++get_merge_tool () { ++ get_merge_tool_guessed | sed -e 's/^[a-z]*://' } mergetool_find_win32_cmd () { @@ -81,7 +83,7 @@ - guessed_merge_tool=true - fi + IFS=':' read guessed_merge_tool merge_tool <<-EOF -+ $(GIT_MERGETOOL_GUI=$gui_tool get_merge_tool) ++ $(GIT_MERGETOOL_GUI=$gui_tool get_merge_tool_guessed) + EOF fi merge_keep_backup="$(git config --bool mergetool.keepBackup || echo true)" 3: de1b897a11 = 3: 24db1afeee mergetool: fallback to tool when guitool unavailable 4: a272594bd2 = 4: 6f65b5c913 difftool: make --gui, --tool and --extcmd mutually exclusive 5: 4fc3f84bad = 5: 5a24772219 difftool: fallback on merge.guitool -- 2.21.0.1000.g11cd861522 ^ permalink raw reply [flat|nested] 48+ messages in thread
* [PATCH v2 1/5] t7610: add mergetool --gui tests 2019-04-23 8:53 ` [PATCH v2 0/5] difftool and mergetool improvements Denton Liu @ 2019-04-23 8:53 ` Denton Liu 2019-04-24 7:07 ` Junio C Hamano 2019-04-23 8:54 ` [PATCH v2 2/5] mergetool: use get_merge_tool_guessed function Denton Liu ` (4 subsequent siblings) 5 siblings, 1 reply; 48+ messages in thread From: Denton Liu @ 2019-04-23 8:53 UTC (permalink / raw) To: Git Mailing List Cc: Johannes Schindelin, David Aguilar, Jeff Hostetler, Eric Sunshine 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 <liu.denton@gmail.com> --- t/t7610-mergetool.sh | 22 ++++++++++++++++++++++ 1 file changed, 22 insertions(+) diff --git a/t/t7610-mergetool.sh b/t/t7610-mergetool.sh index a9fb971615..5f37d7a1ff 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 >/dev/null 2>&1 && + ( yes "" | git mergetool --gui 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 ) && + ( yes "d" | git mergetool --gui file11 >/dev/null 2>&1 ) && + ( yes "d" | git mergetool --gui file12 >/dev/null 2>&1 ) && + ( yes "l" | git mergetool --gui submod >/dev/null 2>&1 ) && + 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 -- 2.21.0.1000.g11cd861522 ^ permalink raw reply related [flat|nested] 48+ messages in thread
* Re: [PATCH v2 1/5] t7610: add mergetool --gui tests 2019-04-23 8:53 ` [PATCH v2 1/5] t7610: add mergetool --gui tests Denton Liu @ 2019-04-24 7:07 ` Junio C Hamano 0 siblings, 0 replies; 48+ messages in thread From: Junio C Hamano @ 2019-04-24 7:07 UTC (permalink / raw) To: Denton Liu Cc: Git Mailing List, Johannes Schindelin, David Aguilar, Jeff Hostetler, Eric Sunshine Denton Liu <liu.denton@gmail.com> writes: > 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 <liu.denton@gmail.com> > --- > t/t7610-mergetool.sh | 22 ++++++++++++++++++++++ > 1 file changed, 22 insertions(+) > > diff --git a/t/t7610-mergetool.sh b/t/t7610-mergetool.sh > index a9fb971615..5f37d7a1ff 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 >/dev/null 2>&1 && > + ( yes "" | git mergetool --gui 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 ) && > + ( yes "d" | git mergetool --gui file11 >/dev/null 2>&1 ) && > + ( yes "d" | git mergetool --gui file12 >/dev/null 2>&1 ) && > + ( yes "l" | git mergetool --gui submod >/dev/null 2>&1 ) && We usually discourage suppressing the output from git commands being tested like the above via redirection. This new testlet seems to mimick the way some of the existing ones are written, but it seems that not all invocations of mergetool in this file discard the output. Is there a particular reason why there are two styles? If not, I think we would want to standardize on *not* discarding. Thanks. ^ permalink raw reply [flat|nested] 48+ messages in thread
* [PATCH v2 2/5] mergetool: use get_merge_tool_guessed function 2019-04-23 8:53 ` [PATCH v2 0/5] difftool and mergetool improvements Denton Liu 2019-04-23 8:53 ` [PATCH v2 1/5] t7610: add mergetool --gui tests Denton Liu @ 2019-04-23 8:54 ` Denton Liu 2019-04-24 7:27 ` Junio C Hamano 2019-04-23 8:54 ` [PATCH v2 3/5] mergetool: fallback to tool when guitool unavailable Denton Liu ` (3 subsequent siblings) 5 siblings, 1 reply; 48+ messages in thread From: Denton Liu @ 2019-04-23 8:54 UTC (permalink / raw) To: Git Mailing List Cc: Johannes Schindelin, David Aguilar, Jeff Hostetler, Eric Sunshine 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. Write `get_merge_tool_guessed` to return whether or not the tool was guessed in addition to the actual tool and make git-mergetool call this function instead of duplicating the logic. Also, let `$GIT_MERGETOOL_GUI` be set to determine whether or not the guitool will be selected. Make `get_merge_tool` use this function internally so that code duplication is reduced. Signed-off-by: Denton Liu <liu.denton@gmail.com> --- After thinking about it for a while, I realised that if it was easy to find one (albeit old) public project using our code, there should be many others who we don't know about that will also be using our code. Let's save them the trouble and just introduce a new function instead of changing the behaviour of the old one. --- Documentation/git-mergetool--lib.txt | 9 ++++++++- git-mergetool--lib.sh | 12 +++++++++--- git-mergetool.sh | 11 +++-------- 3 files changed, 20 insertions(+), 12 deletions(-) diff --git a/Documentation/git-mergetool--lib.txt b/Documentation/git-mergetool--lib.txt index 055550b2bc..343268d885 100644 --- a/Documentation/git-mergetool--lib.txt +++ b/Documentation/git-mergetool--lib.txt @@ -27,8 +27,15 @@ to define the operation mode for the functions listed below. FUNCTIONS --------- +get_merge_tool_guessed:: + returns '$is_guessed:$merge_tool'. '$is_guessed' is 'true' if + the tool was guessed, else 'false'. '$merge_tool' is the merge + tool to use. '$GIT_MERGETOOL_GUI' may be set to 'true' to search + for the appropriate guitool. + get_merge_tool:: - returns a merge tool. + returns a merge tool. '$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-mergetool--lib.sh b/git-mergetool--lib.sh index 83bf52494c..5eedb1a08a 100644 --- a/git-mergetool--lib.sh +++ b/git-mergetool--lib.sh @@ -402,15 +402,21 @@ get_merge_tool_path () { echo "$merge_tool_path" } -get_merge_tool () { +get_merge_tool_guessed () { + 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" + echo "$is_guessed:$merge_tool" +} + +get_merge_tool () { + get_merge_tool_guessed | sed -e 's/^[a-z]*://' } mergetool_find_win32_cmd () { diff --git a/git-mergetool.sh b/git-mergetool.sh index 01b9ad59b2..63e4da1b2f 100755 --- a/git-mergetool.sh +++ b/git-mergetool.sh @@ -449,14 +449,9 @@ 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" - then - merge_tool=$(guess_merge_tool) || exit - guessed_merge_tool=true - fi + IFS=':' read guessed_merge_tool merge_tool <<-EOF + $(GIT_MERGETOOL_GUI=$gui_tool get_merge_tool_guessed) + EOF fi merge_keep_backup="$(git config --bool mergetool.keepBackup || echo true)" merge_keep_temporaries="$(git config --bool mergetool.keepTemporaries || echo false)" -- 2.21.0.1000.g11cd861522 ^ permalink raw reply related [flat|nested] 48+ messages in thread
* Re: [PATCH v2 2/5] mergetool: use get_merge_tool_guessed function 2019-04-23 8:54 ` [PATCH v2 2/5] mergetool: use get_merge_tool_guessed function Denton Liu @ 2019-04-24 7:27 ` Junio C Hamano 0 siblings, 0 replies; 48+ messages in thread From: Junio C Hamano @ 2019-04-24 7:27 UTC (permalink / raw) To: Denton Liu Cc: Git Mailing List, Johannes Schindelin, David Aguilar, Jeff Hostetler, Eric Sunshine Denton Liu <liu.denton@gmail.com> writes: > +get_merge_tool_guessed () { > + 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" > + echo "$is_guessed:$merge_tool" > +} > + > +get_merge_tool () { > + get_merge_tool_guessed | sed -e 's/^[a-z]*://' > } Yuck. Returning a:b is fine if the main use is to match that string using shell builtins like "test" and "case", but piping to "sed" feels a bit too much overhead. Especially given that the other reader in git-emrgetool.sh is not protected for $merge_tool that has a colon in it. Do not try to be too cute and end up with a hack that is both inefficient and brittle at the same time. Possible alternatives: - Because variables in bourne family of shells are global, the caller can easily peek at $is_guessed; or - One bit "did we guess, or did we get from the user?" boolean choice can sufficiently be conveyed by ending the fuction like so instead: ... fi echo "$merge_tool" test "$is_guessed" = true } > diff --git a/git-mergetool.sh b/git-mergetool.sh > index 01b9ad59b2..63e4da1b2f 100755 > --- a/git-mergetool.sh > +++ b/git-mergetool.sh > @@ -449,14 +449,9 @@ 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" > - then > - merge_tool=$(guess_merge_tool) || exit > - guessed_merge_tool=true > - fi > + IFS=':' read guessed_merge_tool merge_tool <<-EOF > + $(GIT_MERGETOOL_GUI=$gui_tool get_merge_tool_guessed) > + EOF With the "let the return code speak" alternative, this would become something like if merge_tool=$(GIT_MERGETOOL_GUI=$gui_tool; get_merge_tool_guessed) then guessed_merge_tool=true else guessed_merge_tool=false fi I do not know what you are trying with GIT_MERGETOOL_GUI=$gui_tool before the shell function, though. It does not work as one-shot assignment to an environment variable. I _think_ it is to feed the all-caps variable to get_configured_merge_tool that is invoked by the get_merge_tool_guessed function, so it does not have to be exported as an environment in the first place, so in the above illustration, I simply wrote an assignment statement, followed by a separate statement that is a parameterless call of a shell function, separated by a semicolon. > fi > merge_keep_backup="$(git config --bool mergetool.keepBackup || echo true)" > merge_keep_temporaries="$(git config --bool mergetool.keepTemporaries || echo false)" ^ permalink raw reply [flat|nested] 48+ messages in thread
* [PATCH v2 3/5] mergetool: fallback to tool when guitool unavailable 2019-04-23 8:53 ` [PATCH v2 0/5] difftool and mergetool improvements Denton Liu 2019-04-23 8:53 ` [PATCH v2 1/5] t7610: add mergetool --gui tests Denton Liu 2019-04-23 8:54 ` [PATCH v2 2/5] mergetool: use get_merge_tool_guessed function Denton Liu @ 2019-04-23 8:54 ` Denton Liu 2019-04-23 8:54 ` [PATCH v2 4/5] difftool: make --gui, --tool and --extcmd mutually exclusive Denton Liu ` (2 subsequent siblings) 5 siblings, 0 replies; 48+ messages in thread From: Denton Liu @ 2019-04-23 8:54 UTC (permalink / raw) To: Git Mailing List Cc: Johannes Schindelin, David Aguilar, Jeff Hostetler, Eric Sunshine 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 were to use `get_configured_mergetool`, it would also get the fallback behaviour in the following precedence: 1. diff.guitool 2. merge.guitool 3. diff.tool 4. merge.tool Signed-off-by: Denton Liu <liu.denton@gmail.com> --- Documentation/git-mergetool.txt | 4 +++- git-mergetool--lib.sh | 27 ++++++++++++++++++--------- t/t7610-mergetool.sh | 19 +++++++++++++++++++ 3 files changed, 40 insertions(+), 10 deletions(-) diff --git a/Documentation/git-mergetool.txt b/Documentation/git-mergetool.txt index 0c7975a050..6b14702e78 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 5eedb1a08a..6f477ae77d 100644 --- a/git-mergetool--lib.sh +++ b/git-mergetool--lib.sh @@ -350,20 +350,29 @@ guess_merge_tool () { } get_configured_merge_tool () { - # If first argument is true, find the guitool instead - if test "$1" = true + is_gui="$1" + sections="merge" + keys="tool" + + if diff_mode then - gui_prefix=gui + sections="diff $sections" 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 + if "$is_gui" = true then - merge_tool=$(git config diff.${gui_prefix}tool || git config merge.${gui_prefix}tool) - else - merge_tool=$(git config merge.${gui_prefix}tool) + keys="guitool $keys" fi + + IFS=' ' + for key in $keys + do + for section in $sections + do + merge_tool=$(git config $section.$key) && break 2 + done + 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 5f37d7a1ff..bc2c9eaa30 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 fallsback 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 >/dev/null 2>&1 && + ( yes "" | git mergetool --gui 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 ) && + ( yes "d" | git mergetool --gui file11 >/dev/null 2>&1 ) && + ( yes "d" | git mergetool --gui file12 >/dev/null 2>&1 ) && + ( yes "l" | git mergetool --gui submod >/dev/null 2>&1 ) && + 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 -- 2.21.0.1000.g11cd861522 ^ permalink raw reply related [flat|nested] 48+ messages in thread
* [PATCH v2 4/5] difftool: make --gui, --tool and --extcmd mutually exclusive 2019-04-23 8:53 ` [PATCH v2 0/5] difftool and mergetool improvements Denton Liu ` (2 preceding siblings ...) 2019-04-23 8:54 ` [PATCH v2 3/5] mergetool: fallback to tool when guitool unavailable Denton Liu @ 2019-04-23 8:54 ` Denton Liu 2019-04-23 8:54 ` [PATCH v2 5/5] difftool: fallback on merge.guitool Denton Liu 2019-04-24 22:46 ` [PATCH v3 0/6] difftool and mergetool improvements Denton Liu 5 siblings, 0 replies; 48+ messages in thread From: Denton Liu @ 2019-04-23 8:54 UTC (permalink / raw) To: Git Mailing List Cc: Johannes Schindelin, David Aguilar, Jeff Hostetler, Eric Sunshine 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 <liu.denton@gmail.com> --- builtin/difftool.c | 3 +++ t/t7800-difftool.sh | 8 ++++++++ 2 files changed, 11 insertions(+) diff --git a/builtin/difftool.c b/builtin/difftool.c index a3ea60ea71..65bba90338 100644 --- a/builtin/difftool.c +++ b/builtin/difftool.c @@ -731,6 +731,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 bb9a7f4ff9..107f31213d 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 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 -- 2.21.0.1000.g11cd861522 ^ permalink raw reply related [flat|nested] 48+ messages in thread
* [PATCH v2 5/5] difftool: fallback on merge.guitool 2019-04-23 8:53 ` [PATCH v2 0/5] difftool and mergetool improvements Denton Liu ` (3 preceding siblings ...) 2019-04-23 8:54 ` [PATCH v2 4/5] difftool: make --gui, --tool and --extcmd mutually exclusive Denton Liu @ 2019-04-23 8:54 ` Denton Liu 2019-04-24 22:46 ` [PATCH v3 0/6] difftool and mergetool improvements Denton Liu 5 siblings, 0 replies; 48+ messages in thread From: Denton Liu @ 2019-04-23 8:54 UTC (permalink / raw) To: Git Mailing List Cc: Johannes Schindelin, David Aguilar, Jeff Hostetler, Eric Sunshine 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 <liu.denton@gmail.com> --- Documentation/git-difftool.txt | 4 +++- builtin/difftool.c | 10 ++-------- t/t7800-difftool.sh | 16 ++++++++++++++++ 3 files changed, 21 insertions(+), 9 deletions(-) diff --git a/Documentation/git-difftool.txt b/Documentation/git-difftool.txt index 96c26e6aa8..484c485fd0 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 65bba90338..10660639c0 100644 --- a/builtin/difftool.c +++ b/builtin/difftool.c @@ -24,7 +24,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[] = { @@ -34,11 +33,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; @@ -734,8 +728,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 107f31213d..ae90701a12 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 ' -- 2.21.0.1000.g11cd861522 ^ permalink raw reply related [flat|nested] 48+ messages in thread
* [PATCH v3 0/6] difftool and mergetool improvements 2019-04-23 8:53 ` [PATCH v2 0/5] difftool and mergetool improvements Denton Liu ` (4 preceding siblings ...) 2019-04-23 8:54 ` [PATCH v2 5/5] difftool: fallback on merge.guitool Denton Liu @ 2019-04-24 22:46 ` Denton Liu 2019-04-24 22:46 ` [PATCH v3 1/6] t7610: unsuppress output Denton Liu ` (6 more replies) 5 siblings, 7 replies; 48+ messages in thread From: Denton Liu @ 2019-04-24 22:46 UTC (permalink / raw) To: Git Mailing List Cc: Johannes Schindelin, David Aguilar, Jeff Hostetler, Eric Sunshine, Junio C Hamano Thanks for the review, Junio. I hadn't thought about using the return code for `get_merge_tool` so thanks for that suggestion. It should be a lot cleaner now. --- Changes since v2: * Unsuppress output in t7610 * Make `get_merge_tool` return 1 on guessed so we don't have to deal with parsing '$guessed:$merge_tool' Changes since v1: * Introduce get_merge_tool_guessed function instead of changing get_merge_tool * Remove unnecessary if-tower in mutual exclusivity logic Denton Liu (6): t7610: unsuppress output t7610: add mergetool --gui tests mergetool: use get_merge_tool function mergetool: fallback to tool when guitool unavailable difftool: make --gui, --tool and --extcmd mutually exclusive difftool: fallback on merge.guitool Documentation/git-difftool.txt | 4 +- Documentation/git-mergetool--lib.txt | 4 +- Documentation/git-mergetool.txt | 4 +- builtin/difftool.c | 13 +-- git-difftool--helper.sh | 2 +- git-mergetool--lib.sh | 32 ++++-- git-mergetool.sh | 12 +- t/t7610-mergetool.sh | 163 +++++++++++++++++---------- t/t7800-difftool.sh | 24 ++++ 9 files changed, 167 insertions(+), 91 deletions(-) Range-diff against v2: -: ---------- > 1: 9f9922cab3 t7610: unsuppress output 1: c436765684 ! 2: 0f632ca6bf t7610: add mergetool --gui tests @@ -23,14 +23,14 @@ + 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 --gui both >/dev/null 2>&1 ) && ++ test_must_fail git merge master && ++ ( yes "" | git mergetool --gui both ) && + ( 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 ) && -+ ( yes "d" | git mergetool --gui file11 >/dev/null 2>&1 ) && -+ ( yes "d" | git mergetool --gui file12 >/dev/null 2>&1 ) && -+ ( yes "l" | git mergetool --gui submod >/dev/null 2>&1 ) && ++ ( 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" && 2: 9f8e76a421 < -: ---------- mergetool: use get_merge_tool_guessed function -: ---------- > 3: ff83d25ff2 mergetool: use get_merge_tool function 3: 4847e64e46 ! 4: e975fe4a8b mergetool: fallback to tool when guitool unavailable @@ -81,18 +81,18 @@ git commit -m "branch1 resolved with mergetool" ' -+test_expect_success 'gui mergetool without merge.guitool set fallsback to merge.tool' ' ++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 >/dev/null 2>&1 && -+ ( yes "" | git mergetool --gui both >/dev/null 2>&1 ) && ++ test_must_fail git merge master && ++ ( yes "" | git mergetool --gui both ) && + ( 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 ) && -+ ( yes "d" | git mergetool --gui file11 >/dev/null 2>&1 ) && -+ ( yes "d" | git mergetool --gui file12 >/dev/null 2>&1 ) && -+ ( yes "l" | git mergetool --gui submod >/dev/null 2>&1 ) && ++ ( 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" && 4: bc8cadaf55 = 5: bc3e229171 difftool: make --gui, --tool and --extcmd mutually exclusive 5: e5e4dc3dd2 = 6: f39b15efbd difftool: fallback on merge.guitool -- 2.21.0.1000.g7817e26e80 ^ permalink raw reply [flat|nested] 48+ messages in thread
* [PATCH v3 1/6] t7610: unsuppress output 2019-04-24 22:46 ` [PATCH v3 0/6] difftool and mergetool improvements Denton Liu @ 2019-04-24 22:46 ` Denton Liu 2019-04-25 2:31 ` Junio C Hamano 2019-04-24 22:46 ` [PATCH v3 2/6] t7610: add mergetool --gui tests Denton Liu ` (5 subsequent siblings) 6 siblings, 1 reply; 48+ messages in thread From: Denton Liu @ 2019-04-24 22:46 UTC (permalink / raw) To: Git Mailing List Cc: Johannes Schindelin, David Aguilar, Jeff Hostetler, Eric Sunshine, Junio C Hamano 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 <liu.denton@gmail.com> --- t/t7610-mergetool.sh | 122 +++++++++++++++++++++---------------------- 1 file changed, 61 insertions(+), 61 deletions(-) diff --git a/t/t7610-mergetool.sh b/t/t7610-mergetool.sh index a9fb971615..69711487dd 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' ' -- 2.21.0.1000.g7817e26e80 ^ permalink raw reply related [flat|nested] 48+ messages in thread
* Re: [PATCH v3 1/6] t7610: unsuppress output 2019-04-24 22:46 ` [PATCH v3 1/6] t7610: unsuppress output Denton Liu @ 2019-04-25 2:31 ` Junio C Hamano 0 siblings, 0 replies; 48+ messages in thread From: Junio C Hamano @ 2019-04-25 2:31 UTC (permalink / raw) To: Denton Liu Cc: Git Mailing List, Johannes Schindelin, David Aguilar, Jeff Hostetler, Eric Sunshine Denton Liu <liu.denton@gmail.com> writes: > 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. Thanks for clearly justifying the clean-up. I take it that with this there is no behaviour changes (e.g. a command that may change its behaviour depending on where its standard output goes does not change the outcome of the tests)? I am not worried about the invocations of "git mergetool" downstream of the pipe that is fed by "yes", and the remainder are a handful of "git merge" invocations, which looked OK to me. Thanks. > Signed-off-by: Denton Liu <liu.denton@gmail.com> > --- > t/t7610-mergetool.sh | 122 +++++++++++++++++++++---------------------- > 1 file changed, 61 insertions(+), 61 deletions(-) > > diff --git a/t/t7610-mergetool.sh b/t/t7610-mergetool.sh > index a9fb971615..69711487dd 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' ' ^ permalink raw reply [flat|nested] 48+ messages in thread
* [PATCH v3 2/6] t7610: add mergetool --gui tests 2019-04-24 22:46 ` [PATCH v3 0/6] difftool and mergetool improvements Denton Liu 2019-04-24 22:46 ` [PATCH v3 1/6] t7610: unsuppress output Denton Liu @ 2019-04-24 22:46 ` Denton Liu 2019-04-24 22:47 ` [PATCH v3 3/6] mergetool: use get_merge_tool function Denton Liu ` (4 subsequent siblings) 6 siblings, 0 replies; 48+ messages in thread From: Denton Liu @ 2019-04-24 22:46 UTC (permalink / raw) To: Git Mailing List Cc: Johannes Schindelin, David Aguilar, Jeff Hostetler, Eric Sunshine, Junio C Hamano 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 <liu.denton@gmail.com> --- t/t7610-mergetool.sh | 22 ++++++++++++++++++++++ 1 file changed, 22 insertions(+) diff --git a/t/t7610-mergetool.sh b/t/t7610-mergetool.sh index 69711487dd..dad607e186 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 -- 2.21.0.1000.g7817e26e80 ^ permalink raw reply related [flat|nested] 48+ messages in thread
* [PATCH v3 3/6] mergetool: use get_merge_tool function 2019-04-24 22:46 ` [PATCH v3 0/6] difftool and mergetool improvements Denton Liu 2019-04-24 22:46 ` [PATCH v3 1/6] t7610: unsuppress output Denton Liu 2019-04-24 22:46 ` [PATCH v3 2/6] t7610: add mergetool --gui tests Denton Liu @ 2019-04-24 22:47 ` Denton Liu 2019-04-25 2:36 ` Junio C Hamano 2019-04-24 22:47 ` [PATCH v3 4/6] mergetool: fallback to tool when guitool unavailable Denton Liu ` (3 subsequent siblings) 6 siblings, 1 reply; 48+ messages in thread From: Denton Liu @ 2019-04-24 22:47 UTC (permalink / raw) To: Git Mailing List Cc: Johannes Schindelin, David Aguilar, Jeff Hostetler, Eric Sunshine, Junio C Hamano 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. 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 <liu.denton@gmail.com> --- Documentation/git-mergetool--lib.txt | 4 +++- git-difftool--helper.sh | 2 +- git-mergetool--lib.sh | 5 ++++- git-mergetool.sh | 12 ++++-------- 4 files changed, 12 insertions(+), 11 deletions(-) diff --git a/Documentation/git-mergetool--lib.txt b/Documentation/git-mergetool--lib.txt index 055550b2bc..4da9d24096 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 7bfb6737df..46af3e60b7 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 83bf52494c..68ff26a0f7 100644 --- a/git-mergetool--lib.sh +++ b/git-mergetool--lib.sh @@ -403,14 +403,17 @@ get_merge_tool_path () { } get_merge_tool () { + not_guessed=true # 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 + not_guessed=false fi echo "$merge_tool" + test "$not_guessed" = true } mergetool_find_win32_cmd () { diff --git a/git-mergetool.sh b/git-mergetool.sh index 01b9ad59b2..88fa6a914a 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 -- 2.21.0.1000.g7817e26e80 ^ permalink raw reply related [flat|nested] 48+ messages in thread
* Re: [PATCH v3 3/6] mergetool: use get_merge_tool function 2019-04-24 22:47 ` [PATCH v3 3/6] mergetool: use get_merge_tool function Denton Liu @ 2019-04-25 2:36 ` Junio C Hamano 0 siblings, 0 replies; 48+ messages in thread From: Junio C Hamano @ 2019-04-25 2:36 UTC (permalink / raw) To: Denton Liu Cc: Git Mailing List, Johannes Schindelin, David Aguilar, Jeff Hostetler, Eric Sunshine Denton Liu <liu.denton@gmail.com> writes: > 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. 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. Sounds like a risk that is OK to accept, as it is easy enough to reintroduce the middle layer function like you did in the previous round when it becomes an issue. Will queue; thanks. ^ permalink raw reply [flat|nested] 48+ messages in thread
* [PATCH v3 4/6] mergetool: fallback to tool when guitool unavailable 2019-04-24 22:46 ` [PATCH v3 0/6] difftool and mergetool improvements Denton Liu ` (2 preceding siblings ...) 2019-04-24 22:47 ` [PATCH v3 3/6] mergetool: use get_merge_tool function Denton Liu @ 2019-04-24 22:47 ` Denton Liu 2019-04-25 3:02 ` Junio C Hamano 2019-04-24 22:47 ` [PATCH v3 5/6] difftool: make --gui, --tool and --extcmd mutually exclusive Denton Liu ` (2 subsequent siblings) 6 siblings, 1 reply; 48+ messages in thread From: Denton Liu @ 2019-04-24 22:47 UTC (permalink / raw) To: Git Mailing List Cc: Johannes Schindelin, David Aguilar, Jeff Hostetler, Eric Sunshine, Junio C Hamano 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 were to use `get_configured_mergetool`, it would also get the fallback behaviour in the following precedence: 1. diff.guitool 2. merge.guitool 3. diff.tool 4. merge.tool Signed-off-by: Denton Liu <liu.denton@gmail.com> --- Documentation/git-mergetool.txt | 4 +++- git-mergetool--lib.sh | 27 ++++++++++++++++++--------- t/t7610-mergetool.sh | 19 +++++++++++++++++++ 3 files changed, 40 insertions(+), 10 deletions(-) diff --git a/Documentation/git-mergetool.txt b/Documentation/git-mergetool.txt index 0c7975a050..6b14702e78 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 68ff26a0f7..ada16acffc 100644 --- a/git-mergetool--lib.sh +++ b/git-mergetool--lib.sh @@ -350,20 +350,29 @@ guess_merge_tool () { } get_configured_merge_tool () { - # If first argument is true, find the guitool instead - if test "$1" = true + is_gui="$1" + sections="merge" + keys="tool" + + if diff_mode then - gui_prefix=gui + sections="diff $sections" 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 + if "$is_gui" = true then - merge_tool=$(git config diff.${gui_prefix}tool || git config merge.${gui_prefix}tool) - else - merge_tool=$(git config merge.${gui_prefix}tool) + keys="guitool $keys" fi + + IFS=' ' + for key in $keys + do + for section in $sections + do + merge_tool=$(git config $section.$key) && break 2 + done + 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 dad607e186..5b61c10a9c 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 -- 2.21.0.1000.g7817e26e80 ^ permalink raw reply related [flat|nested] 48+ messages in thread
* Re: [PATCH v3 4/6] mergetool: fallback to tool when guitool unavailable 2019-04-24 22:47 ` [PATCH v3 4/6] mergetool: fallback to tool when guitool unavailable Denton Liu @ 2019-04-25 3:02 ` Junio C Hamano 2019-04-25 5:16 ` Denton Liu 0 siblings, 1 reply; 48+ messages in thread From: Junio C Hamano @ 2019-04-25 3:02 UTC (permalink / raw) To: Denton Liu Cc: Git Mailing List, Johannes Schindelin, David Aguilar, Jeff Hostetler, Eric Sunshine Denton Liu <liu.denton@gmail.com> writes: > 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 were to use `get_configured_mergetool`, it would also I agree that the precedence order below makes sense, but I am a bit confused by "were to use" here. Do you mean you'll make the change to make difftool to look at mergetool configuraiton in a later step in the series? Or is there a way for the user to say "I want my difftool to also pay attention to the mergetool configurations" (and another "I do not want that" option)? I'll come back to this later. > get the fallback behaviour in the following precedence: > > 1. diff.guitool > 2. merge.guitool > 3. diff.tool > 4. merge.tool > > Signed-off-by: Denton Liu <liu.denton@gmail.com> > --- > Documentation/git-mergetool.txt | 4 +++- > git-mergetool--lib.sh | 27 ++++++++++++++++++--------- > t/t7610-mergetool.sh | 19 +++++++++++++++++++ > 3 files changed, 40 insertions(+), 10 deletions(-) > > diff --git a/Documentation/git-mergetool.txt b/Documentation/git-mergetool.txt > index 0c7975a050..6b14702e78 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`. Makes sense. So "mergetool --gui" from the command line would look at guitool and then tool in the merge section. > diff --git a/git-mergetool--lib.sh b/git-mergetool--lib.sh > index 68ff26a0f7..ada16acffc 100644 > --- a/git-mergetool--lib.sh > +++ b/git-mergetool--lib.sh > @@ -350,20 +350,29 @@ guess_merge_tool () { > } > > get_configured_merge_tool () { > - # If first argument is true, find the guitool instead > - if test "$1" = true > + is_gui="$1" > + sections="merge" > + keys="tool" > + > + if diff_mode > then > - gui_prefix=gui > + sections="diff $sections" > 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 > + if "$is_gui" = true > then > - merge_tool=$(git config diff.${gui_prefix}tool || git config merge.${gui_prefix}tool) > - else > - merge_tool=$(git config merge.${gui_prefix}tool) > + keys="guitool $keys" > fi OK, so $sections is "diff merge" for difftool and just "merge" for mergetool. $tool is "guitool tool" under "--gui", and just "tool" otherwise. > + IFS=' ' > + for key in $keys > + do > + for section in $sections > + do > + merge_tool=$(git config $section.$key) && break 2 > + done > + done And you do up to four iterations to cover the combination in the precedence order. Which makes sense. I am not sure about the wisdom of setting IFS here, though. As far as I can see, both $keys and $sections do not take any arbitrary values, but just the two (for each) values you know that do not have any funny characters, so I am not sure what you are trying to achieve by that (i.e. benefit is unclear). As long as the get_configured_merge_tool function is called always for string_emitted_to_stdout=$(that function), the updated setting will not leak to the outside world so there is no risk to break its callers, but it is not immediately obvious if helper functions called in the remainder of this function are OK with the modified value of IFS (i.e. safety is not obvious). Now for the promised "come back to this later", I think you meant "the get_configured_merge_tool function is already prepared to be used from difftool in this step and when difftool starts to call it here is what happens". But I wonder if it makes the evolution of the topic easier to follow if you defer it to a later step when you actually make difftool to start calling it? In other words, in this step, your get_configured_merge_tool would look like sections=merge case "$1" in true) keys="guitool tool" ;; *) keys="tool" ;; esac for key in $keys do for section in $sections do merge_tool=$(git config ...) && break 2 done done ... and then in a later step (6/6?), the unconditional setting of sections to 'merge' would be updated so that in diff_mode, you'll iterate over two sections. I dunno. ^ permalink raw reply [flat|nested] 48+ messages in thread
* Re: [PATCH v3 4/6] mergetool: fallback to tool when guitool unavailable 2019-04-25 3:02 ` Junio C Hamano @ 2019-04-25 5:16 ` Denton Liu 0 siblings, 0 replies; 48+ messages in thread From: Denton Liu @ 2019-04-25 5:16 UTC (permalink / raw) To: Junio C Hamano Cc: Git Mailing List, Johannes Schindelin, David Aguilar, Jeff Hostetler, Eric Sunshine Hi Junio, On Thu, Apr 25, 2019 at 12:02:23PM +0900, Junio C Hamano wrote: > Denton Liu <liu.denton@gmail.com> writes: > > > 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 were to use `get_configured_mergetool`, it would also > > I agree that the precedence order below makes sense, but I am a bit > confused by "were to use" here. Do you mean you'll make the change > to make difftool to look at mergetool configuraiton in a later step > in the series? Or is there a way for the user to say "I want my > difftool to also pay attention to the mergetool configurations" (and > another "I do not want that" option)? I'll come back to this later. Correct, it means it'll be done in a future patch (i.e. 6/6). I guess I wasn't fully clear in the message. I meant something like, "If `git difftool --gui` were to use..." because difftool currently already uses this function in the non-gui case. [snip] > > > + IFS=' ' > > + for key in $keys > > + do > > + for section in $sections > > + do > > + merge_tool=$(git config $section.$key) && break 2 > > + done > > + done > > And you do up to four iterations to cover the combination in the > precedence order. Which makes sense. > > I am not sure about the wisdom of setting IFS here, though. > > As far as I can see, both $keys and $sections do not take any > arbitrary values, but just the two (for each) values you know that > do not have any funny characters, so I am not sure what you are > trying to achieve by that (i.e. benefit is unclear). The reason why IFS is being set is because at the top of mergetool--lib, we set IFS to '\n'. As a result, without setting IFS, the strings won't parse properly into the for loop. > > As long as the get_configured_merge_tool function is called always > for string_emitted_to_stdout=$(that function), the updated setting > will not leak to the outside world so there is no risk to break its > callers, but it is not immediately obvious if helper functions > called in the remainder of this function are OK with the modified > value of IFS (i.e. safety is not obvious). When I was writing this, I didn't realise that the value of IFS bleeds out of this function. I'll reroll this to use a helper function just in case. > > Now for the promised "come back to this later", I think you meant > "the get_configured_merge_tool function is already prepared to be > used from difftool in this step and when difftool starts to call it > here is what happens". But I wonder if it makes the evolution of > the topic easier to follow if you defer it to a later step when you > actually make difftool to start calling it? In other words, in this > step, your get_configured_merge_tool would look like > > sections=merge > > case "$1" in > true) > keys="guitool tool" ;; > *) > keys="tool" ;; > esac > > for key in $keys > do > for section in $sections > do > merge_tool=$(git config ...) && break 2 > done > done > ... > > and then in a later step (6/6?), the unconditional setting of > sections to 'merge' would be updated so that in diff_mode, you'll > iterate over two sections. > > I dunno. As stated above, difftool currently uses this function in the non-gui case. I think that clarifying the log message on my part should make it easier to understand the evolution of this topic. Thanks for the careful review, Denton ^ permalink raw reply [flat|nested] 48+ messages in thread
* [PATCH v3 5/6] difftool: make --gui, --tool and --extcmd mutually exclusive 2019-04-24 22:46 ` [PATCH v3 0/6] difftool and mergetool improvements Denton Liu ` (3 preceding siblings ...) 2019-04-24 22:47 ` [PATCH v3 4/6] mergetool: fallback to tool when guitool unavailable Denton Liu @ 2019-04-24 22:47 ` Denton Liu 2019-04-24 22:47 ` [PATCH v3 6/6] difftool: fallback on merge.guitool Denton Liu 2019-04-25 9:54 ` [PATCH v4 0/6] difftool and mergetool improvements Denton Liu 6 siblings, 0 replies; 48+ messages in thread From: Denton Liu @ 2019-04-24 22:47 UTC (permalink / raw) To: Git Mailing List Cc: Johannes Schindelin, David Aguilar, Jeff Hostetler, Eric Sunshine, Junio C Hamano 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 <liu.denton@gmail.com> --- builtin/difftool.c | 3 +++ t/t7800-difftool.sh | 8 ++++++++ 2 files changed, 11 insertions(+) diff --git a/builtin/difftool.c b/builtin/difftool.c index a3ea60ea71..65bba90338 100644 --- a/builtin/difftool.c +++ b/builtin/difftool.c @@ -731,6 +731,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 bb9a7f4ff9..107f31213d 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 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 -- 2.21.0.1000.g7817e26e80 ^ permalink raw reply related [flat|nested] 48+ messages in thread
* [PATCH v3 6/6] difftool: fallback on merge.guitool 2019-04-24 22:46 ` [PATCH v3 0/6] difftool and mergetool improvements Denton Liu ` (4 preceding siblings ...) 2019-04-24 22:47 ` [PATCH v3 5/6] difftool: make --gui, --tool and --extcmd mutually exclusive Denton Liu @ 2019-04-24 22:47 ` Denton Liu 2019-04-25 3:10 ` Junio C Hamano 2019-04-25 9:54 ` [PATCH v4 0/6] difftool and mergetool improvements Denton Liu 6 siblings, 1 reply; 48+ messages in thread From: Denton Liu @ 2019-04-24 22:47 UTC (permalink / raw) To: Git Mailing List Cc: Johannes Schindelin, David Aguilar, Jeff Hostetler, Eric Sunshine, Junio C Hamano 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 <liu.denton@gmail.com> --- Documentation/git-difftool.txt | 4 +++- builtin/difftool.c | 10 ++-------- t/t7800-difftool.sh | 16 ++++++++++++++++ 3 files changed, 21 insertions(+), 9 deletions(-) diff --git a/Documentation/git-difftool.txt b/Documentation/git-difftool.txt index 96c26e6aa8..484c485fd0 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 65bba90338..10660639c0 100644 --- a/builtin/difftool.c +++ b/builtin/difftool.c @@ -24,7 +24,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[] = { @@ -34,11 +33,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; @@ -734,8 +728,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 107f31213d..ae90701a12 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 ' -- 2.21.0.1000.g7817e26e80 ^ permalink raw reply related [flat|nested] 48+ messages in thread
* Re: [PATCH v3 6/6] difftool: fallback on merge.guitool 2019-04-24 22:47 ` [PATCH v3 6/6] difftool: fallback on merge.guitool Denton Liu @ 2019-04-25 3:10 ` Junio C Hamano 0 siblings, 0 replies; 48+ messages in thread From: Junio C Hamano @ 2019-04-25 3:10 UTC (permalink / raw) To: Denton Liu Cc: Git Mailing List, Johannes Schindelin, David Aguilar, Jeff Hostetler, Eric Sunshine Denton Liu <liu.denton@gmail.com> writes: > @@ -734,8 +728,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); So unless difftool_cmd is given explicitly, we'll let the scripted difftool--helper to let merge_tool=$(get_merge_tool) to pick the tool, which will use the same logic you wrote in the step 2/6. OK, that makes sense. What was preventing the get_configured_merge_tool updated in step 2/6 from getting called in difftool was the exporting of GIT_DIFF_TOOL we see abovethat was removed by this step. Which also makes sense. > diff --git a/t/t7800-difftool.sh b/t/t7800-difftool.sh > index 107f31213d..ae90701a12 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 > ' ^ permalink raw reply [flat|nested] 48+ messages in thread
* [PATCH v4 0/6] difftool and mergetool improvements 2019-04-24 22:46 ` [PATCH v3 0/6] difftool and mergetool improvements Denton Liu ` (5 preceding siblings ...) 2019-04-24 22:47 ` [PATCH v3 6/6] difftool: fallback on merge.guitool Denton Liu @ 2019-04-25 9:54 ` Denton Liu 2019-04-25 9:54 ` [PATCH v4 1/6] t7610: unsuppress output Denton Liu ` (6 more replies) 6 siblings, 7 replies; 48+ messages in thread From: Denton Liu @ 2019-04-25 9:54 UTC (permalink / raw) To: Git Mailing List Cc: Johannes Schindelin, David Aguilar, Jeff Hostetler, Eric Sunshine, Junio C Hamano Thanks for another review, Junio. This should basically be the same as v3 except I've moved the IFS assignment into a subshell so its value does not leak out. I'd like the change in 4/6 to be reviewed carefully. t7610 and t7800 run properly with dash on My Machineة, but I couldn't find a reference on whether my usage is POSIX legal or not. We do have precedent for using fors within a subshell, though, from a quick git grep: t/t3202-show-branch-octopus.sh: git show-branch $(for i in $numbers; do echo branch$i; done) > out && Thanks, Denton --- Changes since v3: * Move nested for into a subshell so that IFS does not leak out of function Changes since v2: * Unsuppress output in t7610 * Make `get_merge_tool` return 1 on guessed so we don't have to deal with parsing '$guessed:$merge_tool' Changes since v1: * Introduce get_merge_tool_guessed function instead of changing get_merge_tool * Remove unnecessary if-tower in mutual exclusivity logic Denton Liu (6): t7610: unsuppress output t7610: add mergetool --gui tests mergetool: use get_merge_tool function mergetool: fallback to tool when guitool unavailable difftool: make --gui, --tool and --extcmd mutually exclusive difftool: fallback on merge.guitool Documentation/git-difftool.txt | 4 +- Documentation/git-mergetool--lib.txt | 4 +- Documentation/git-mergetool.txt | 4 +- builtin/difftool.c | 13 +-- git-difftool--helper.sh | 2 +- git-mergetool--lib.sh | 37 ++++-- git-mergetool.sh | 12 +- t/t7610-mergetool.sh | 163 +++++++++++++++++---------- t/t7800-difftool.sh | 24 ++++ 9 files changed, 172 insertions(+), 91 deletions(-) Range-diff against v3: 1: 9f9922cab3 = 1: 9f9922cab3 t7610: unsuppress output 2: 0f632ca6bf = 2: 0f632ca6bf t7610: add mergetool --gui tests 3: ff83d25ff2 = 3: ff83d25ff2 mergetool: use get_merge_tool function 4: e975fe4a8b ! 4: c799e871e2 mergetool: fallback to tool when guitool unavailable @@ -6,14 +6,18 @@ 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 were to use `get_configured_mergetool`, it would also - get the fallback behaviour in the following precedence: + 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 + Note that the behavior for when difftool or mergetool are called without + `--gui` should be identical with or without this patch. + Signed-off-by: Denton Liu <liu.denton@gmail.com> diff --git a/Documentation/git-mergetool.txt b/Documentation/git-mergetool.txt @@ -61,14 +65,19 @@ + keys="guitool $keys" fi + -+ IFS=' ' -+ for key in $keys -+ do -+ for section in $sections ++ merge_tool=$( ++ IFS=' ' ++ for key in $keys + do -+ merge_tool=$(git config $section.$key) && break 2 -+ done -+ done ++ for section in $sections ++ do ++ if selected=$(git config $section.$key) ++ then ++ echo "$selected" ++ return ++ fi ++ done ++ done) + if test -n "$merge_tool" && ! valid_tool "$merge_tool" then 5: bc3e229171 = 5: 7d7c936cd3 difftool: make --gui, --tool and --extcmd mutually exclusive 6: f39b15efbd = 6: b642ed3b1e difftool: fallback on merge.guitool -- 2.21.0.1000.g11cd861522 ^ permalink raw reply [flat|nested] 48+ messages in thread
* [PATCH v4 1/6] t7610: unsuppress output 2019-04-25 9:54 ` [PATCH v4 0/6] difftool and mergetool improvements Denton Liu @ 2019-04-25 9:54 ` Denton Liu 2019-04-25 9:54 ` [PATCH v4 2/6] t7610: add mergetool --gui tests Denton Liu ` (5 subsequent siblings) 6 siblings, 0 replies; 48+ messages in thread From: Denton Liu @ 2019-04-25 9:54 UTC (permalink / raw) To: Git Mailing List Cc: Johannes Schindelin, David Aguilar, Jeff Hostetler, Eric Sunshine, Junio C Hamano 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 <liu.denton@gmail.com> --- t/t7610-mergetool.sh | 122 +++++++++++++++++++++---------------------- 1 file changed, 61 insertions(+), 61 deletions(-) diff --git a/t/t7610-mergetool.sh b/t/t7610-mergetool.sh index a9fb971615..69711487dd 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' ' -- 2.21.0.1000.g11cd861522 ^ permalink raw reply related [flat|nested] 48+ messages in thread
* [PATCH v4 2/6] t7610: add mergetool --gui tests 2019-04-25 9:54 ` [PATCH v4 0/6] difftool and mergetool improvements Denton Liu 2019-04-25 9:54 ` [PATCH v4 1/6] t7610: unsuppress output Denton Liu @ 2019-04-25 9:54 ` Denton Liu 2019-04-25 9:54 ` [PATCH v4 3/6] mergetool: use get_merge_tool function Denton Liu ` (4 subsequent siblings) 6 siblings, 0 replies; 48+ messages in thread From: Denton Liu @ 2019-04-25 9:54 UTC (permalink / raw) To: Git Mailing List Cc: Johannes Schindelin, David Aguilar, Jeff Hostetler, Eric Sunshine, Junio C Hamano 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 <liu.denton@gmail.com> --- t/t7610-mergetool.sh | 22 ++++++++++++++++++++++ 1 file changed, 22 insertions(+) diff --git a/t/t7610-mergetool.sh b/t/t7610-mergetool.sh index 69711487dd..dad607e186 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 -- 2.21.0.1000.g11cd861522 ^ permalink raw reply related [flat|nested] 48+ messages in thread
* [PATCH v4 3/6] mergetool: use get_merge_tool function 2019-04-25 9:54 ` [PATCH v4 0/6] difftool and mergetool improvements Denton Liu 2019-04-25 9:54 ` [PATCH v4 1/6] t7610: unsuppress output Denton Liu 2019-04-25 9:54 ` [PATCH v4 2/6] t7610: add mergetool --gui tests Denton Liu @ 2019-04-25 9:54 ` Denton Liu 2019-04-28 23:52 ` David Aguilar 2019-04-25 9:54 ` [PATCH v4 4/6] mergetool: fallback to tool when guitool unavailable Denton Liu ` (3 subsequent siblings) 6 siblings, 1 reply; 48+ messages in thread From: Denton Liu @ 2019-04-25 9:54 UTC (permalink / raw) To: Git Mailing List Cc: Johannes Schindelin, David Aguilar, Jeff Hostetler, Eric Sunshine, Junio C Hamano 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. 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 <liu.denton@gmail.com> --- Documentation/git-mergetool--lib.txt | 4 +++- git-difftool--helper.sh | 2 +- git-mergetool--lib.sh | 5 ++++- git-mergetool.sh | 12 ++++-------- 4 files changed, 12 insertions(+), 11 deletions(-) diff --git a/Documentation/git-mergetool--lib.txt b/Documentation/git-mergetool--lib.txt index 055550b2bc..4da9d24096 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 7bfb6737df..46af3e60b7 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 83bf52494c..68ff26a0f7 100644 --- a/git-mergetool--lib.sh +++ b/git-mergetool--lib.sh @@ -403,14 +403,17 @@ get_merge_tool_path () { } get_merge_tool () { + not_guessed=true # 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 + not_guessed=false fi echo "$merge_tool" + test "$not_guessed" = true } mergetool_find_win32_cmd () { diff --git a/git-mergetool.sh b/git-mergetool.sh index 01b9ad59b2..88fa6a914a 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 -- 2.21.0.1000.g11cd861522 ^ permalink raw reply related [flat|nested] 48+ messages in thread
* Re: [PATCH v4 3/6] mergetool: use get_merge_tool function 2019-04-25 9:54 ` [PATCH v4 3/6] mergetool: use get_merge_tool function Denton Liu @ 2019-04-28 23:52 ` David Aguilar 0 siblings, 0 replies; 48+ messages in thread From: David Aguilar @ 2019-04-28 23:52 UTC (permalink / raw) To: Denton Liu Cc: Git Mailing List, Johannes Schindelin, Jeff Hostetler, Eric Sunshine, Junio C Hamano On Thu, Apr 25, 2019 at 02:54:39AM -0700, Denton Liu wrote: > 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. 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 <liu.denton@gmail.com> The author of git-diffall approached the list some time ago and his contribution resulted in "git difftool --dir-diff". These days we would probably encourage users to use the difftool built-in feature rather than "diffall", but thank you for your careful consideration against breaking external scripts. Maybe we can submit a PR against the diffall repo mentioning the backstory so that new users are pointed to the main tool. > diff --git a/git-mergetool--lib.sh b/git-mergetool--lib.sh > index 83bf52494c..68ff26a0f7 100644 > --- a/git-mergetool--lib.sh > +++ b/git-mergetool--lib.sh > @@ -403,14 +403,17 @@ get_merge_tool_path () { > } > > get_merge_tool () { > + not_guessed=true Tiny nit; I find double-negatives hard to understand. Maybe rename this to "is_guessed" and flip the logic below so that we can keep the variable named as a positive flag? > # 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 > + not_guessed=false > fi > echo "$merge_tool" > + test "$not_guessed" = true > } -- David ^ permalink raw reply [flat|nested] 48+ messages in thread
* [PATCH v4 4/6] mergetool: fallback to tool when guitool unavailable 2019-04-25 9:54 ` [PATCH v4 0/6] difftool and mergetool improvements Denton Liu ` (2 preceding siblings ...) 2019-04-25 9:54 ` [PATCH v4 3/6] mergetool: use get_merge_tool function Denton Liu @ 2019-04-25 9:54 ` Denton Liu 2019-04-28 23:56 ` David Aguilar 2019-04-25 9:54 ` [PATCH v4 5/6] difftool: make --gui, --tool and --extcmd mutually exclusive Denton Liu ` (2 subsequent siblings) 6 siblings, 1 reply; 48+ messages in thread From: Denton Liu @ 2019-04-25 9:54 UTC (permalink / raw) To: Git Mailing List Cc: Johannes Schindelin, David Aguilar, Jeff Hostetler, Eric Sunshine, Junio C Hamano 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 Note that the behavior for when difftool or mergetool are called without `--gui` should be identical with or without this patch. Signed-off-by: Denton Liu <liu.denton@gmail.com> --- Documentation/git-mergetool.txt | 4 +++- git-mergetool--lib.sh | 32 +++++++++++++++++++++++--------- t/t7610-mergetool.sh | 19 +++++++++++++++++++ 3 files changed, 45 insertions(+), 10 deletions(-) diff --git a/Documentation/git-mergetool.txt b/Documentation/git-mergetool.txt index 0c7975a050..6b14702e78 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 68ff26a0f7..c4b16c5e59 100644 --- a/git-mergetool--lib.sh +++ b/git-mergetool--lib.sh @@ -350,20 +350,34 @@ guess_merge_tool () { } get_configured_merge_tool () { - # If first argument is true, find the guitool instead - if test "$1" = true + is_gui="$1" + sections="merge" + keys="tool" + + if diff_mode then - gui_prefix=gui + sections="diff $sections" 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 + if "$is_gui" = true then - merge_tool=$(git config diff.${gui_prefix}tool || git config merge.${gui_prefix}tool) - else - merge_tool=$(git config merge.${gui_prefix}tool) + keys="guitool $keys" fi + + merge_tool=$( + IFS=' ' + for key in $keys + do + for section in $sections + do + if selected=$(git config $section.$key) + then + echo "$selected" + return + fi + done + 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 dad607e186..5b61c10a9c 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 -- 2.21.0.1000.g11cd861522 ^ permalink raw reply related [flat|nested] 48+ messages in thread
* Re: [PATCH v4 4/6] mergetool: fallback to tool when guitool unavailable 2019-04-25 9:54 ` [PATCH v4 4/6] mergetool: fallback to tool when guitool unavailable Denton Liu @ 2019-04-28 23:56 ` David Aguilar 0 siblings, 0 replies; 48+ messages in thread From: David Aguilar @ 2019-04-28 23:56 UTC (permalink / raw) To: Denton Liu Cc: Git Mailing List, Johannes Schindelin, Jeff Hostetler, Eric Sunshine, Junio C Hamano On Thu, Apr 25, 2019 at 02:54:41AM -0700, Denton Liu wrote: > diff --git a/git-mergetool--lib.sh b/git-mergetool--lib.sh > index 68ff26a0f7..c4b16c5e59 100644 > --- a/git-mergetool--lib.sh > +++ b/git-mergetool--lib.sh > @@ -350,20 +350,34 @@ guess_merge_tool () { > } > > get_configured_merge_tool () { > - # If first argument is true, find the guitool instead > - if test "$1" = true > + is_gui="$1" > + sections="merge" > + keys="tool" > + > + if diff_mode > then > - gui_prefix=gui > + sections="diff $sections" > 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 > + if "$is_gui" = true This line looks suspect. How about, if test "$is_gui" = true instead? This expression could also be lifted out to an "is_gui" helper function. > then > - merge_tool=$(git config diff.${gui_prefix}tool || git config merge.${gui_prefix}tool) > - else > - merge_tool=$(git config merge.${gui_prefix}tool) > + keys="guitool $keys" > fi > + > + merge_tool=$( > + IFS=' ' > + for key in $keys > + do > + for section in $sections > + do > + if selected=$(git config $section.$key) Would it be simpler to split this conditional into two lines? selected=$(git config ...) if test -n "$selected" then ... fi Yes, it stops looking at the exit code, but it instead focuses on the result, which is slightly more bulletproof against a funky user configuration. Regarding the two loops above, what would it look like if we unrolled the logic and just spelled out the keys up front that it's a little easier to follow? I agree it is nicer from an implementation sense to use loops, but we really shouldn't be planning to extend to more permutations in the future beyond the diff/merge duality, so being explicit and spelling out each config lookup permutation is simpler to understand since we only have 4 states. We should be discouraged from adding any more ;-) Something like, keys= if merge_mode then if gui_mode # probably worth adding this function then keys="merge.guitool merge.tool" else keys="merge.tool" fi else if gui_mode then keys="diff.guitool merge.guitool diff.tool merge.tool" else keys="diff.tool merge.tool" fi fi .. and then just have a single loop over $keys. -- David ^ permalink raw reply [flat|nested] 48+ messages in thread
* [PATCH v4 5/6] difftool: make --gui, --tool and --extcmd mutually exclusive 2019-04-25 9:54 ` [PATCH v4 0/6] difftool and mergetool improvements Denton Liu ` (3 preceding siblings ...) 2019-04-25 9:54 ` [PATCH v4 4/6] mergetool: fallback to tool when guitool unavailable Denton Liu @ 2019-04-25 9:54 ` Denton Liu 2019-04-25 9:54 ` [PATCH v4 6/6] difftool: fallback on merge.guitool Denton Liu 2019-04-29 6:20 ` [PATCH v5 0/7] difftool and mergetool improvements Denton Liu 6 siblings, 0 replies; 48+ messages in thread From: Denton Liu @ 2019-04-25 9:54 UTC (permalink / raw) To: Git Mailing List Cc: Johannes Schindelin, David Aguilar, Jeff Hostetler, Eric Sunshine, Junio C Hamano 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 <liu.denton@gmail.com> --- builtin/difftool.c | 3 +++ t/t7800-difftool.sh | 8 ++++++++ 2 files changed, 11 insertions(+) diff --git a/builtin/difftool.c b/builtin/difftool.c index a3ea60ea71..65bba90338 100644 --- a/builtin/difftool.c +++ b/builtin/difftool.c @@ -731,6 +731,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 bb9a7f4ff9..107f31213d 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 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 -- 2.21.0.1000.g11cd861522 ^ permalink raw reply related [flat|nested] 48+ messages in thread
* [PATCH v4 6/6] difftool: fallback on merge.guitool 2019-04-25 9:54 ` [PATCH v4 0/6] difftool and mergetool improvements Denton Liu ` (4 preceding siblings ...) 2019-04-25 9:54 ` [PATCH v4 5/6] difftool: make --gui, --tool and --extcmd mutually exclusive Denton Liu @ 2019-04-25 9:54 ` Denton Liu 2019-04-29 6:20 ` [PATCH v5 0/7] difftool and mergetool improvements Denton Liu 6 siblings, 0 replies; 48+ messages in thread From: Denton Liu @ 2019-04-25 9:54 UTC (permalink / raw) To: Git Mailing List Cc: Johannes Schindelin, David Aguilar, Jeff Hostetler, Eric Sunshine, Junio C Hamano 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 <liu.denton@gmail.com> --- Documentation/git-difftool.txt | 4 +++- builtin/difftool.c | 10 ++-------- t/t7800-difftool.sh | 16 ++++++++++++++++ 3 files changed, 21 insertions(+), 9 deletions(-) diff --git a/Documentation/git-difftool.txt b/Documentation/git-difftool.txt index 96c26e6aa8..484c485fd0 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 65bba90338..10660639c0 100644 --- a/builtin/difftool.c +++ b/builtin/difftool.c @@ -24,7 +24,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[] = { @@ -34,11 +33,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; @@ -734,8 +728,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 107f31213d..ae90701a12 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 ' -- 2.21.0.1000.g11cd861522 ^ permalink raw reply related [flat|nested] 48+ messages in thread
* [PATCH v5 0/7] difftool and mergetool improvements 2019-04-25 9:54 ` [PATCH v4 0/6] difftool and mergetool improvements Denton Liu ` (5 preceding siblings ...) 2019-04-25 9:54 ` [PATCH v4 6/6] difftool: fallback on merge.guitool Denton Liu @ 2019-04-29 6:20 ` Denton Liu 2019-04-29 6:21 ` [PATCH v5 1/7] t7610: unsuppress output Denton Liu ` (6 more replies) 6 siblings, 7 replies; 48+ messages in thread From: Denton Liu @ 2019-04-29 6:20 UTC (permalink / raw) To: Git Mailing List Cc: Johannes Schindelin, David Aguilar, Jeff Hostetler, Eric Sunshine, Junio C Hamano Hi David, thanks for the comments. I'm not really sure why I did the double-negative thing... It seems obvious that it should be the other way around. I also unrolled the loops and wrote a gui_mode function. Good suggestions! --- Changes since v4: * Remove double-negative * Change double-nested search loop into one for-loop * Create gui_mode function * Change an instance of "exclusive" to "mutually exclusive" Changes since v3: * Move nested for into a subshell so that IFS does not leak out of function Changes since v2: * Unsuppress output in t7610 * Make `get_merge_tool` return 1 on guessed so we don't have to deal with parsing '$guessed:$merge_tool' Changes since v1: * Introduce get_merge_tool_guessed function instead of changing get_merge_tool * Remove unnecessary if-tower in mutual exclusivity logic Denton Liu (7): t7610: unsuppress output t7610: add mergetool --gui tests mergetool: use get_merge_tool function mergetool--lib: create gui_mode function mergetool: fallback to tool when guitool unavailable difftool: make --gui, --tool and --extcmd mutually exclusive difftool: fallback on merge.guitool Documentation/git-difftool.txt | 4 +- Documentation/git-mergetool--lib.txt | 4 +- Documentation/git-mergetool.txt | 4 +- builtin/difftool.c | 13 +-- git-difftool--helper.sh | 2 +- git-mergetool--lib.sh | 47 ++++++-- git-mergetool.sh | 12 +- t/t7610-mergetool.sh | 163 +++++++++++++++++---------- t/t7800-difftool.sh | 24 ++++ 9 files changed, 180 insertions(+), 93 deletions(-) Range-diff against v4: 1: 919aa32e20 = 1: 9f9922cab3 t7610: unsuppress output 2: 9a1bb60b20 = 2: 0f632ca6bf t7610: add mergetool --gui tests 3: a900ce2a6a ! 3: 81dd25d8e2 mergetool: use get_merge_tool function @@ -18,8 +18,9 @@ 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. It seems - very unlikely that there exists an external caller that would take into + 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. @@ -63,7 +64,7 @@ } get_merge_tool () { -+ not_guessed=true ++ 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) @@ -71,10 +72,10 @@ if test -z "$merge_tool" then merge_tool=$(guess_merge_tool) || exit -+ not_guessed=false ++ is_guessed=true fi echo "$merge_tool" -+ test "$not_guessed" = true ++ test "$is_guessed" = false } mergetool_find_win32_cmd () { -: ---------- > 4: 27a59e1e27 mergetool--lib: create gui_mode function 4: abcf91688a ! 5: 40413dbda1 mergetool: fallback to tool when guitool unavailable @@ -15,8 +15,41 @@ 3. diff.tool 4. merge.tool - Note that the behavior for when difftool or mergetool are called without - `--gui` should be identical with or without this patch. + 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's + immediately obvious for future readers. Signed-off-by: Denton Liu <liu.denton@gmail.com> @@ -42,41 +75,43 @@ } get_configured_merge_tool () { -- # If first argument is true, find the guitool instead -- if test "$1" = true -+ is_gui="$1" -+ sections="merge" -+ keys="tool" -+ -+ if diff_mode - then +- if gui_mode +- then - gui_prefix=gui -+ sections="diff $sections" - fi - +- 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 -+ if "$is_gui" = true ++ keys= + if diff_mode then - merge_tool=$(git config diff.${gui_prefix}tool || git config merge.${gui_prefix}tool) -- else ++ 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) -+ keys="guitool $keys" ++ if gui_mode ++ then ++ keys="merge.guitool merge.tool" ++ else ++ keys="merge.tool" ++ fi fi + + merge_tool=$( + IFS=' ' + for key in $keys + do -+ for section in $sections -+ do -+ if selected=$(git config $section.$key) -+ then -+ echo "$selected" -+ return -+ fi -+ done ++ selected=$(git config $key) ++ if test -n "$selected" ++ then ++ echo "$selected" ++ return ++ fi + done) + if test -n "$merge_tool" && ! valid_tool "$merge_tool" 5: 9ec39c5af0 ! 6: c70789b689 difftool: make --gui, --tool and --extcmd mutually exclusive @@ -29,7 +29,7 @@ test_cmp expect actual ' -+test_expect_success 'difftool --gui, --tool and --extcmd are exclusive' ' ++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 && 6: a72009fc3d = 7: 3fd4f46a7c difftool: fallback on merge.guitool -- 2.21.0.1033.g0e8cc1100c ^ permalink raw reply [flat|nested] 48+ messages in thread
* [PATCH v5 1/7] t7610: unsuppress output 2019-04-29 6:20 ` [PATCH v5 0/7] difftool and mergetool improvements Denton Liu @ 2019-04-29 6:21 ` Denton Liu 2019-04-29 6:21 ` [PATCH v5 2/7] t7610: add mergetool --gui tests Denton Liu ` (5 subsequent siblings) 6 siblings, 0 replies; 48+ messages in thread From: Denton Liu @ 2019-04-29 6:21 UTC (permalink / raw) To: Git Mailing List Cc: Johannes Schindelin, David Aguilar, Jeff Hostetler, Eric Sunshine, Junio C Hamano 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 <liu.denton@gmail.com> --- t/t7610-mergetool.sh | 122 +++++++++++++++++++++---------------------- 1 file changed, 61 insertions(+), 61 deletions(-) diff --git a/t/t7610-mergetool.sh b/t/t7610-mergetool.sh index a9fb971615..69711487dd 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' ' -- 2.21.0.1033.g0e8cc1100c ^ permalink raw reply related [flat|nested] 48+ messages in thread
* [PATCH v5 2/7] t7610: add mergetool --gui tests 2019-04-29 6:20 ` [PATCH v5 0/7] difftool and mergetool improvements Denton Liu 2019-04-29 6:21 ` [PATCH v5 1/7] t7610: unsuppress output Denton Liu @ 2019-04-29 6:21 ` Denton Liu 2019-04-29 6:21 ` [PATCH v5 3/7] mergetool: use get_merge_tool function Denton Liu ` (4 subsequent siblings) 6 siblings, 0 replies; 48+ messages in thread From: Denton Liu @ 2019-04-29 6:21 UTC (permalink / raw) To: Git Mailing List Cc: Johannes Schindelin, David Aguilar, Jeff Hostetler, Eric Sunshine, Junio C Hamano 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 <liu.denton@gmail.com> --- t/t7610-mergetool.sh | 22 ++++++++++++++++++++++ 1 file changed, 22 insertions(+) diff --git a/t/t7610-mergetool.sh b/t/t7610-mergetool.sh index 69711487dd..dad607e186 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 -- 2.21.0.1033.g0e8cc1100c ^ permalink raw reply related [flat|nested] 48+ messages in thread
* [PATCH v5 3/7] mergetool: use get_merge_tool function 2019-04-29 6:20 ` [PATCH v5 0/7] difftool and mergetool improvements Denton Liu 2019-04-29 6:21 ` [PATCH v5 1/7] t7610: unsuppress output Denton Liu 2019-04-29 6:21 ` [PATCH v5 2/7] t7610: add mergetool --gui tests Denton Liu @ 2019-04-29 6:21 ` Denton Liu 2019-04-29 6:21 ` [PATCH v5 4/7] mergetool--lib: create gui_mode function Denton Liu ` (3 subsequent siblings) 6 siblings, 0 replies; 48+ messages in thread From: Denton Liu @ 2019-04-29 6:21 UTC (permalink / raw) To: Git Mailing List Cc: Johannes Schindelin, David Aguilar, Jeff Hostetler, Eric Sunshine, Junio C Hamano 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 <liu.denton@gmail.com> --- Documentation/git-mergetool--lib.txt | 4 +++- git-difftool--helper.sh | 2 +- git-mergetool--lib.sh | 5 ++++- git-mergetool.sh | 12 ++++-------- 4 files changed, 12 insertions(+), 11 deletions(-) diff --git a/Documentation/git-mergetool--lib.txt b/Documentation/git-mergetool--lib.txt index 055550b2bc..4da9d24096 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 7bfb6737df..46af3e60b7 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 83bf52494c..b928179a2e 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 01b9ad59b2..88fa6a914a 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 -- 2.21.0.1033.g0e8cc1100c ^ permalink raw reply related [flat|nested] 48+ messages in thread
* [PATCH v5 4/7] mergetool--lib: create gui_mode function 2019-04-29 6:20 ` [PATCH v5 0/7] difftool and mergetool improvements Denton Liu ` (2 preceding siblings ...) 2019-04-29 6:21 ` [PATCH v5 3/7] mergetool: use get_merge_tool function Denton Liu @ 2019-04-29 6:21 ` Denton Liu 2019-04-29 6:21 ` [PATCH v5 5/7] mergetool: fallback to tool when guitool unavailable Denton Liu ` (2 subsequent siblings) 6 siblings, 0 replies; 48+ messages in thread From: Denton Liu @ 2019-04-29 6:21 UTC (permalink / raw) To: Git Mailing List Cc: Johannes Schindelin, David Aguilar, Jeff Hostetler, Eric Sunshine, Junio C Hamano 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 <liu.denton@gmail.com> --- git-mergetool--lib.sh | 13 ++++++++----- 1 file changed, 8 insertions(+), 5 deletions(-) diff --git a/git-mergetool--lib.sh b/git-mergetool--lib.sh index b928179a2e..4ca170c8a7 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 -- 2.21.0.1033.g0e8cc1100c ^ permalink raw reply related [flat|nested] 48+ messages in thread
* [PATCH v5 5/7] mergetool: fallback to tool when guitool unavailable 2019-04-29 6:20 ` [PATCH v5 0/7] difftool and mergetool improvements Denton Liu ` (3 preceding siblings ...) 2019-04-29 6:21 ` [PATCH v5 4/7] mergetool--lib: create gui_mode function Denton Liu @ 2019-04-29 6:21 ` Denton Liu 2019-04-29 6:21 ` [PATCH v5 6/7] difftool: make --gui, --tool and --extcmd mutually exclusive Denton Liu 2019-04-29 6:21 ` [PATCH v5 7/7] difftool: fallback on merge.guitool Denton Liu 6 siblings, 0 replies; 48+ messages in thread From: Denton Liu @ 2019-04-29 6:21 UTC (permalink / raw) To: Git Mailing List Cc: Johannes Schindelin, David Aguilar, Jeff Hostetler, Eric Sunshine, Junio C Hamano 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 <liu.denton@gmail.com> --- Documentation/git-mergetool.txt | 4 +++- git-mergetool--lib.sh | 35 ++++++++++++++++++++++++--------- t/t7610-mergetool.sh | 19 ++++++++++++++++++ 3 files changed, 48 insertions(+), 10 deletions(-) diff --git a/Documentation/git-mergetool.txt b/Documentation/git-mergetool.txt index 0c7975a050..6b14702e78 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 4ca170c8a7..696eb49160 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 dad607e186..5b61c10a9c 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 -- 2.21.0.1033.g0e8cc1100c ^ permalink raw reply related [flat|nested] 48+ messages in thread
* [PATCH v5 6/7] difftool: make --gui, --tool and --extcmd mutually exclusive 2019-04-29 6:20 ` [PATCH v5 0/7] difftool and mergetool improvements Denton Liu ` (4 preceding siblings ...) 2019-04-29 6:21 ` [PATCH v5 5/7] mergetool: fallback to tool when guitool unavailable Denton Liu @ 2019-04-29 6:21 ` Denton Liu 2019-04-29 6:21 ` [PATCH v5 7/7] difftool: fallback on merge.guitool Denton Liu 6 siblings, 0 replies; 48+ messages in thread From: Denton Liu @ 2019-04-29 6:21 UTC (permalink / raw) To: Git Mailing List Cc: Johannes Schindelin, David Aguilar, Jeff Hostetler, Eric Sunshine, Junio C Hamano 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 <liu.denton@gmail.com> --- builtin/difftool.c | 3 +++ t/t7800-difftool.sh | 8 ++++++++ 2 files changed, 11 insertions(+) diff --git a/builtin/difftool.c b/builtin/difftool.c index a3ea60ea71..65bba90338 100644 --- a/builtin/difftool.c +++ b/builtin/difftool.c @@ -731,6 +731,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 bb9a7f4ff9..fd4a2a93b6 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 -- 2.21.0.1033.g0e8cc1100c ^ permalink raw reply related [flat|nested] 48+ messages in thread
* [PATCH v5 7/7] difftool: fallback on merge.guitool 2019-04-29 6:20 ` [PATCH v5 0/7] difftool and mergetool improvements Denton Liu ` (5 preceding siblings ...) 2019-04-29 6:21 ` [PATCH v5 6/7] difftool: make --gui, --tool and --extcmd mutually exclusive Denton Liu @ 2019-04-29 6:21 ` Denton Liu 6 siblings, 0 replies; 48+ messages in thread From: Denton Liu @ 2019-04-29 6:21 UTC (permalink / raw) To: Git Mailing List Cc: Johannes Schindelin, David Aguilar, Jeff Hostetler, Eric Sunshine, Junio C Hamano 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 <liu.denton@gmail.com> --- Documentation/git-difftool.txt | 4 +++- builtin/difftool.c | 10 ++-------- t/t7800-difftool.sh | 16 ++++++++++++++++ 3 files changed, 21 insertions(+), 9 deletions(-) diff --git a/Documentation/git-difftool.txt b/Documentation/git-difftool.txt index 96c26e6aa8..484c485fd0 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 65bba90338..10660639c0 100644 --- a/builtin/difftool.c +++ b/builtin/difftool.c @@ -24,7 +24,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[] = { @@ -34,11 +33,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; @@ -734,8 +728,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 fd4a2a93b6..957ddf5dc6 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 ' -- 2.21.0.1033.g0e8cc1100c ^ permalink raw reply related [flat|nested] 48+ messages in thread
end of thread, other threads:[~2019-04-29 6:21 UTC | newest] Thread overview: 48+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2019-04-22 5:07 [PATCH 0/5] difftool and mergetool improvements Denton Liu 2019-04-22 5:07 ` [PATCH 1/5] t7610: add mergetool --gui tests Denton Liu 2019-04-22 5:07 ` [PATCH 2/5] mergetool: use get_merge_tool function Denton Liu 2019-04-22 7:07 ` Eric Sunshine 2019-04-22 8:35 ` Denton Liu 2019-04-22 5:07 ` [PATCH 3/5] mergetool: fallback to tool when guitool unavailable Denton Liu 2019-04-22 5:07 ` [PATCH 4/5] difftool: make --gui, --tool and --extcmd exclusive Denton Liu 2019-04-22 7:03 ` Eric Sunshine 2019-04-22 5:07 ` [PATCH 5/5] difftool: fallback on merge.guitool Denton Liu 2019-04-22 18:18 ` Jeff Hostetler 2019-04-22 18:33 ` Denton Liu 2019-04-23 8:53 ` [PATCH v2 0/5] difftool and mergetool improvements Denton Liu 2019-04-23 8:53 ` [PATCH v2 1/5] t7610: add mergetool --gui tests Denton Liu 2019-04-24 7:07 ` Junio C Hamano 2019-04-23 8:54 ` [PATCH v2 2/5] mergetool: use get_merge_tool_guessed function Denton Liu 2019-04-24 7:27 ` Junio C Hamano 2019-04-23 8:54 ` [PATCH v2 3/5] mergetool: fallback to tool when guitool unavailable Denton Liu 2019-04-23 8:54 ` [PATCH v2 4/5] difftool: make --gui, --tool and --extcmd mutually exclusive Denton Liu 2019-04-23 8:54 ` [PATCH v2 5/5] difftool: fallback on merge.guitool Denton Liu 2019-04-24 22:46 ` [PATCH v3 0/6] difftool and mergetool improvements Denton Liu 2019-04-24 22:46 ` [PATCH v3 1/6] t7610: unsuppress output Denton Liu 2019-04-25 2:31 ` Junio C Hamano 2019-04-24 22:46 ` [PATCH v3 2/6] t7610: add mergetool --gui tests Denton Liu 2019-04-24 22:47 ` [PATCH v3 3/6] mergetool: use get_merge_tool function Denton Liu 2019-04-25 2:36 ` Junio C Hamano 2019-04-24 22:47 ` [PATCH v3 4/6] mergetool: fallback to tool when guitool unavailable Denton Liu 2019-04-25 3:02 ` Junio C Hamano 2019-04-25 5:16 ` Denton Liu 2019-04-24 22:47 ` [PATCH v3 5/6] difftool: make --gui, --tool and --extcmd mutually exclusive Denton Liu 2019-04-24 22:47 ` [PATCH v3 6/6] difftool: fallback on merge.guitool Denton Liu 2019-04-25 3:10 ` Junio C Hamano 2019-04-25 9:54 ` [PATCH v4 0/6] difftool and mergetool improvements Denton Liu 2019-04-25 9:54 ` [PATCH v4 1/6] t7610: unsuppress output Denton Liu 2019-04-25 9:54 ` [PATCH v4 2/6] t7610: add mergetool --gui tests Denton Liu 2019-04-25 9:54 ` [PATCH v4 3/6] mergetool: use get_merge_tool function Denton Liu 2019-04-28 23:52 ` David Aguilar 2019-04-25 9:54 ` [PATCH v4 4/6] mergetool: fallback to tool when guitool unavailable Denton Liu 2019-04-28 23:56 ` David Aguilar 2019-04-25 9:54 ` [PATCH v4 5/6] difftool: make --gui, --tool and --extcmd mutually exclusive Denton Liu 2019-04-25 9:54 ` [PATCH v4 6/6] difftool: fallback on merge.guitool Denton Liu 2019-04-29 6:20 ` [PATCH v5 0/7] difftool and mergetool improvements Denton Liu 2019-04-29 6:21 ` [PATCH v5 1/7] t7610: unsuppress output Denton Liu 2019-04-29 6:21 ` [PATCH v5 2/7] t7610: add mergetool --gui tests Denton Liu 2019-04-29 6:21 ` [PATCH v5 3/7] mergetool: use get_merge_tool function Denton Liu 2019-04-29 6:21 ` [PATCH v5 4/7] mergetool--lib: create gui_mode function Denton Liu 2019-04-29 6:21 ` [PATCH v5 5/7] mergetool: fallback to tool when guitool unavailable Denton Liu 2019-04-29 6:21 ` [PATCH v5 6/7] difftool: make --gui, --tool and --extcmd mutually exclusive Denton Liu 2019-04-29 6:21 ` [PATCH v5 7/7] difftool: fallback on merge.guitool Denton Liu
Code repositories for project(s) associated with this public inbox https://80x24.org/mirrors/git.git This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).