| Message ID | pull.1381.v4.git.1679153263217.gitgitgadget@gmail.com (mailing list archive) |
|---|---|
| State | Accepted |
| Commit | 42943b950e12f2d3e56688ecef1b0502e162b436 |
| Headers | show |
| Series | [v4] mergetool: new config guiDefault supports auto-toggling gui by DISPLAY | expand |
On Sat, Mar 18, 2023 at 03:27:43PM +0000, Tao Klerks via GitGitGadget wrote: > From: Tao Klerks <tao@klerks.biz> > > When no merge.tool or diff.tool is configured or manually selected, the > selection of a default tool is sensitive to the DISPLAY variable; in a > GUI session a gui-specific tool will be proposed if found, and > otherwise a terminal-based one. This "GUI-optimizing" behavior is > important because a GUI can make a huge difference to a user's ability > to understand and correctly complete a non-trivial conflicting merge. > > Some time ago the merge.guitool and diff.guitool config options were > introduced to enable users to configure both a GUI tool, and a non-GUI > tool (with fallback if no GUI tool configured), in the same environment. > > Unfortunately, the --gui argument introduced to support the selection of > the guitool is still explicit. When using configured tools, there is no > equivalent of the no-tool-configured "propose a GUI tool if we are in a GUI > environment" behavior. I'm not sure what problem this is solving. Please help me understand. This commit seems to be talking about the behavior w.r.t. the DISPLAY variable and also about automatically enabling `--gui` automatically. Regarding `--gui`: Someone that configures `mergetool.guiDefault` means that all of their invocations should be as-if they had written `--gui` on the command-line. So.. if you find that you always use `--gui` then why are you not configuring `merge.tool` directly and instead configuring `merge.guitool`? The purpose of `--gui` is to provide an override for use by tools (such as Git Cola) to execute an optional, 2nd merge tool separate from that which the user might configure for command-line use. This patch basically makes the `merge.tool` configuration useless, so I don't understand why someone would want to configure this option rather than just configuring `merge.tool` directly, leading to an overall simpler and easier to understand system. If we add this boolean then it becomes harder to understand. I'm not sure it really provides any additional value to the user. From my vantage point, the user is better off configuring `merge.tool` directly rather than the tool becoming more complex. That said, I probably don't understand and I'd be happy to be enlightened. > As proposed in <xmqqmtb8jsej.fsf@gitster.g>, introduce new configuration > options, difftool.guiDefault and mergetool.guiDefault, supporting a special > value "auto" which causes the corresponding tool or guitool to be selected > depending on the presence of a non-empty DISPLAY value. Also support "true" > to say "default to the guitool (unless --no-gui is passed on the > commandline)", and "false" as the previous default behavior when these new > configuration options are not specified. The rationale here is slightly surprising to me as well. Apparently the user cannot be bothered to configure `merge.tool` (because we care about this DISPLAY behavior) yet the user is expected to configure `merge.guiDefault` instead? I think it'd be better to encourage users to go ahead and configure `merge.tool`, but again, I'm probably missing context around why. Small patch note below... > diff --git a/git-difftool--helper.sh b/git-difftool--helper.sh > index 992124cc67c..e4e820e6809 100755 > --- a/git-difftool--helper.sh > +++ b/git-difftool--helper.sh > @@ -75,6 +75,11 @@ then > merge_tool="$GIT_DIFF_TOOL" > else > merge_tool="$(get_merge_tool)" > + subshell_exit_status=$? > + if test $subshell_exit_status -gt 1 Is exit status 1 special here? Should this be "if test $subshell_exit_status != 0" instead?
First of all, thank you for reviewing!! On Tue, Apr 4, 2023 at 11:46 AM David Aguilar <davvid@gmail.com> wrote: > > On Sat, Mar 18, 2023 at 03:27:43PM +0000, Tao Klerks via GitGitGadget wrote: > > From: Tao Klerks <tao@klerks.biz> > > > > When no merge.tool or diff.tool is configured or manually selected, the > > selection of a default tool is sensitive to the DISPLAY variable; in a > > GUI session a gui-specific tool will be proposed if found, and > > otherwise a terminal-based one. This "GUI-optimizing" behavior is > > important because a GUI can make a huge difference to a user's ability > > to understand and correctly complete a non-trivial conflicting merge. > > > > Some time ago the merge.guitool and diff.guitool config options were > > introduced to enable users to configure both a GUI tool, and a non-GUI > > tool (with fallback if no GUI tool configured), in the same environment. > > > > Unfortunately, the --gui argument introduced to support the selection of > > the guitool is still explicit. When using configured tools, there is no > > equivalent of the no-tool-configured "propose a GUI tool if we are in a GUI > > environment" behavior. > > I'm not sure what problem this is solving. Please help me understand. > This commit seems to be talking about the behavior w.r.t. the DISPLAY > variable and also about automatically enabling `--gui` automatically. Background: If you don't set a mergetool at all, git defaults one if it can. The default you get depends on the DISPLAY variable - if it looks like you're in a gui context (because we detect that DISPLAY is set), then we propose a gui tool. Otherwise, we propose a cli tool. This is particularly valuable for contexts where you might access the same environment, the same workstation, via multiple contexts - CLI-only, and GUI. For example, sometimes you connect to the desktop, and sometimes you SSH into it (without X forwarding). You don't need to tell git "I want a GUI now please", it figures that out. Point/problem: As soon as you set "merge.tool", you lose that autoselection. A reasonable user would assume "there is `merge.tool`, there is `merge.guitool`, and git knows how to autodetect GUI context based on the DISPLAY variable... so if I set both `merge.tool` and `merge.guitool`, I'll get the right one based on context, right? Like I do when I set neither?" Unfortunately, that is *not* the case. The gui autodetection only applies when git is choosing a *defaulted* merge tool, not an explicitly-defined one. As soon as you specify a tool you want to use, you need to start *explicitly* telling git whether you want to use a GUI tool or not. If you get it wrong, an inconvenient or even bad thing happens: If you're in a GUI context and you forget to say `--gui`, then you get the CLI tool - not the end of the world, you might argue. If you're in a non-GUI context and you say `--gui`, you get a (hopefully intelligible) error about not finding a display, or worst case, you hang - and git doesn't know what happened, the messaging has to assume you aborted explicitly. If there were a way to get git to autoselect "--gui" by presence of the DISPLAY, as it does for the *default* mergetool, then this kind of flow would be much more intuitive, usable, etc. I'm not sure whether I'm explaining better, or just repeating myself, so I'll stop here :) > > Regarding `--gui`: Someone that configures `mergetool.guiDefault` > means that all of their invocations should be as-if they had written > `--gui` on the command-line. Correction: Someone that sets `mergetool.guiDefault` to *true* should get behavior as-if they had written `--gui` on the command-line. They could also set *false* (current behavior - explicit), or *auto*, and get different behavior. > > So.. if you find that you always use `--gui` then why are > you not configuring `merge.tool` directly and instead configuring > `merge.guitool`? One reason is because you might choose to do something different in a *specific* invocation, eg by passing `--no-gui`. This is a common pattern in git - the config allows you to change the default behavior, but you can override it on a specific invocation by using the positive or negative version of the argument. > > The purpose of `--gui` is to provide an override for use by tools (such > as Git Cola) to execute an optional, 2nd merge tool separate from that > which the user might configure for command-line use. I'm not aware of having read a statement identifying this *singular*/sole purpose anywhere, but as you state it with authority (and I see you're the author of a popular git GUI), I'm happy to accept that this was originally its only purpose. That would certainly explain the incongruousness of its behavior wrt (lack of) autodetection of DISPLAY. That said, this patch does not in any way diminish the validity or usability of this usecase. If you don't set `mergetool.guiDefault` or leave it set to false, that purpose will continue to be perfectly served. For other users, like me, who prefer to select a GUI mergetool or not depending on the *availability* of a GUI environment (auto), or default to gui but allow manually downgrading to a (defaulted or configured) CLI-only tool, this configuration will allow a simpler, more coherent, easier UX. If everything else here makes sense, I will happily update the abstract/commit message to reflect the proposed broadening of application / correct my misunderstanding as to the original intended scope of the setting. > > This patch basically makes the `merge.tool` configuration useless, I don't understand why you say this. Unless I misunderstand something, this patch does not break, or even change, any existing behavior. > so I > don't understand why someone would want to configure this option rather > than just configuring `merge.tool` directly, leading to an overall > simpler and easier to understand system. (see above - the main usecase is a system you access via both GUI and CLI contexts, and the secondary usecase is having a default GUI behavior that you can easily override with `--no-gui`.) > > If we add this boolean then it becomes harder to understand. > I'm not sure it really provides any additional value to the user. > > From my vantage point, the user is better off configuring `merge.tool` > directly rather than the tool becoming more complex. > > That said, I probably don't understand and I'd be happy to be > enlightened. Please let me know if it makes more sense now. You could argue that the value is too limited to warrant the additional complexity (obviously I would disagree), but hopefully the concrete value is clearer now. > > > > As proposed in <xmqqmtb8jsej.fsf@gitster.g>, introduce new configuration > > options, difftool.guiDefault and mergetool.guiDefault, supporting a special > > value "auto" which causes the corresponding tool or guitool to be selected > > depending on the presence of a non-empty DISPLAY value. Also support "true" > > to say "default to the guitool (unless --no-gui is passed on the > > commandline)", and "false" as the previous default behavior when these new > > configuration options are not specified. > > The rationale here is slightly surprising to me as well. > > Apparently the user cannot be bothered to configure `merge.tool` > (because we care about this DISPLAY behavior) yet the user is expected > to configure `merge.guiDefault` instead? Umm, no: The user wants `merge.tool` to be set to a specific CLI tool, they want `merge.guitool` to be set to a specific GUI tool, and they want the GUI tool to be used in GUI contexts, and the CLI tool to be used when there is no GUI (eg SSH without X forwarding) - like the behavior they get if they configure neither `merge.tool` nor `merge.guitool`. > > I think it'd be better to encourage users to go ahead and configure > `merge.tool`, but again, I'm probably missing context around why. > > > Small patch note below... > > > > diff --git a/git-difftool--helper.sh b/git-difftool--helper.sh > > index 992124cc67c..e4e820e6809 100755 > > --- a/git-difftool--helper.sh > > +++ b/git-difftool--helper.sh > > @@ -75,6 +75,11 @@ then > > merge_tool="$GIT_DIFF_TOOL" > > else > > merge_tool="$(get_merge_tool)" > > + subshell_exit_status=$? > > + if test $subshell_exit_status -gt 1 > > Is exit status 1 special here? > Should this be "if test $subshell_exit_status != 0" instead? Yes, it is special: 1 means "mergetool was guessed/defaulted", anything greater than 1 means "mergetool determination failed; aborting". I was not very happy with this way of passing information around when I implemented, and commented in the previous version of the patch https://lore.kernel.org/git/pull.1381.v3.git.1666076086910.gitgitgadget@gmail.com/ as my last doubt as to the proposed/RFC patch: After implementing Junio's recommended "fail if defaulting config is consulted and is invalid" flow, there now needs to be a distinction between subshell exit code 1, which was used before and indicates "tool not found or broken; falling back to default" and other (higher) exit codes, which newly mean "something went wrong, stop!". The resulting code looks awkward, I can't tell whether I'm missing a code or even commenting pattern that would make it clearer. Unfortunately I didn't get any feedback on this, so I went ahead and resumitted the change as a non-Draft. I'd be very grateful for any recommendations on how to handle this better.
Tao Klerks <tao@klerks.biz> writes: > If you get it wrong, an inconvenient or even bad > thing happens: If you're in a GUI context and you forget to say > `--gui`, then you get the CLI tool - not the end of the world, you > might argue. If you're in a non-GUI context and you say `--gui`, you > get a (hopefully intelligible) error about not finding a display, or > worst case, you hang - and git doesn't know what happened, the > messaging has to assume you aborted explicitly. The worst case I can think of is if you have a machine that allows a persistent virtual graphical session (remote desktop). You can go there, open a terminal and create a screen session, which captures the $DISPLAY in effect. You may disconnect from the screen session before you disconnect from the remote desktop. As the remote desktop is persistent, the $DISPLAY would be valid even after you disconnect from there. Then you ssh into the machine and connect to the screen session. If you say "xterm &" in that screen session, a new window with a terminal will open and be shown on that remote display session, which you cannot see until you reconnect to the remote desktop. Running "git mergetool --gui" would be the same. With the proposed patch, the experience would become even worse in that scenario, I am afraid. "git mergetool" (without "--[no-]gui") would detect the presense of $DISPLAY and start a GUI mergetool that you cannot view or interact with ;-). You need to explicitly say "git mergetool --no-gui" in such a case to force use of cli tool. Even with that caveat, I think the proposed behaviour would be a net improvement. I almost always work in non-GUI environment, but I can see many people work in both and depending on their environment wish to use the best tool for the situation. > If there were a way to get git to autoselect "--gui" by presence of > the DISPLAY, as it does for the *default* mergetool, then this kind of > flow would be much more intuitive, usable, etc. > > I'm not sure whether I'm explaining better, or just repeating myself, > so I'll stop here :) Hopefully your explanation was clear enough. Others may be able to offer solution better than "look at DISPLAY and choose --[no-]gui when neither is given from the command line", but I think at least I understand your motivation behind this change. Thanks.
On Tue, Apr 4, 2023 at 1:35 PM Junio C Hamano <gitster@pobox.com> wrote: > > Tao Klerks <tao@klerks.biz> writes: > > > If you get it wrong, an inconvenient or even bad > > thing happens: If you're in a GUI context and you forget to say > > `--gui`, then you get the CLI tool - not the end of the world, you > > might argue. If you're in a non-GUI context and you say `--gui`, you > > get a (hopefully intelligible) error about not finding a display, or > > worst case, you hang - and git doesn't know what happened, the > > messaging has to assume you aborted explicitly. > > The worst case I can think of is if you have a machine that allows a > persistent virtual graphical session (remote desktop). You can go > there, open a terminal and create a screen session, which captures > the $DISPLAY in effect. You may disconnect from the screen session > before you disconnect from the remote desktop. As the remote > desktop is persistent, the $DISPLAY would be valid even after you > disconnect from there. > > Then you ssh into the machine and connect to the screen session. If > you say "xterm &" in that screen session, a new window with a > terminal will open and be shown on that remote display session, > which you cannot see until you reconnect to the remote desktop. > Running "git mergetool --gui" would be the same. > > With the proposed patch, the experience would become even worse in > that scenario, I am afraid. "git mergetool" (without "--[no-]gui") > would detect the presense of $DISPLAY and start a GUI mergetool that > you cannot view or interact with ;-). You need to explicitly say > "git mergetool --no-gui" in such a case to force use of cli tool. > > Even with that caveat, I think the proposed behaviour would be a net > improvement. I almost always work in non-GUI environment, but I can > see many people work in both and depending on their environment wish > to use the best tool for the situation. > > > If there were a way to get git to autoselect "--gui" by presence of > > the DISPLAY, as it does for the *default* mergetool, then this kind of > > flow would be much more intuitive, usable, etc. > > > > I'm not sure whether I'm explaining better, or just repeating myself, > > so I'll stop here :) > > Hopefully your explanation was clear enough. Others may be able to > offer solution better than "look at DISPLAY and choose --[no-]gui > when neither is given from the command line", but I think at least I > understand your motivation behind this change. > > Thanks. Thanks for the thorough explanation. With that in mind, this all sounds good to me.
diff --git a/Documentation/config/difftool.txt b/Documentation/config/difftool.txt index a3f82112102..447c40d85a2 100644 --- a/Documentation/config/difftool.txt +++ b/Documentation/config/difftool.txt @@ -34,3 +34,10 @@ See the `--trust-exit-code` option in linkgit:git-difftool[1] for more details. difftool.prompt:: Prompt before each invocation of the diff tool. + +difftool.guiDefault:: + Set `true` to use the `diff.guitool` by default (equivalent to specifying + the `--gui` argument), or `auto` to select `diff.guitool` or `diff.tool` + depending on the presence of a `DISPLAY` environment variable value. The + default is `false`, where the `--gui` argument must be provided + explicitly for the `diff.guitool` to be used. diff --git a/Documentation/config/mergetool.txt b/Documentation/config/mergetool.txt index e779a122d8a..56a7eeeffb4 100644 --- a/Documentation/config/mergetool.txt +++ b/Documentation/config/mergetool.txt @@ -85,3 +85,10 @@ mergetool.writeToTemp:: mergetool.prompt:: Prompt before each invocation of the merge resolution program. + +mergetool.guiDefault:: + Set `true` to use the `merge.guitool` by default (equivalent to + specifying the `--gui` argument), or `auto` to select `merge.guitool` + or `merge.tool` depending on the presence of a `DISPLAY` environment + variable value. The default is `false`, where the `--gui` argument + must be provided explicitly for the `merge.guitool` to be used. diff --git a/Documentation/git-difftool.txt b/Documentation/git-difftool.txt index 9d14c3c9f09..ac0ac6fa022 100644 --- a/Documentation/git-difftool.txt +++ b/Documentation/git-difftool.txt @@ -97,10 +97,12 @@ instead. `--no-symlinks` is the default on Windows. --[no-]gui:: 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. If `diff.guitool` - is not set, we will fallback in the order of `merge.guitool`, - `diff.tool`, `merge.tool` until a tool is found. + `diff.guitool` variable instead of `diff.tool`. This may be + selected automatically using the configuration variable + `difftool.guiDefault`. The `--no-gui` option can be used to + override these settings. 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/Documentation/git-mergetool.txt b/Documentation/git-mergetool.txt index c44e205629b..07535f6576e 100644 --- a/Documentation/git-mergetool.txt +++ b/Documentation/git-mergetool.txt @@ -85,12 +85,13 @@ success of the resolution after the custom tool has exited. the default merge tool will be read from the configured `merge.guitool` variable instead of `merge.tool`. If `merge.guitool` is not set, we will fallback to the tool - configured under `merge.tool`. + configured under `merge.tool`. This may be autoselected using + the configuration variable `mergetool.guiDefault`. --no-gui:: - This overrides a previous `-g` or `--gui` setting and reads the - default merge tool will be read from the configured `merge.tool` - variable. + This overrides a previous `-g` or `--gui` setting or + `mergetool.guiDefault` configuration and reads the default merge + tool from the configured `merge.tool` variable. -O<orderfile>:: Process files in the order specified in the diff --git a/builtin/difftool.c b/builtin/difftool.c index dbbfb19f192..88a00abfbdb 100644 --- a/builtin/difftool.c +++ b/builtin/difftool.c @@ -684,7 +684,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, + int use_gui_tool = -1, dir_diff = 0, prompt = -1, symlinks = 0, tool_help = 0, no_index = 0; static char *difftool_cmd = NULL, *extcmd = NULL; struct option builtin_difftool_options[] = { @@ -734,13 +734,21 @@ int cmd_difftool(int argc, const char **argv, const char *prefix) } else if (dir_diff) die(_("options '%s' and '%s' cannot be used together"), "--dir-diff", "--no-index"); - die_for_incompatible_opt3(use_gui_tool, "--gui", + die_for_incompatible_opt3(use_gui_tool == 1, "--gui", !!difftool_cmd, "--tool", !!extcmd, "--extcmd"); - if (use_gui_tool) + /* + * Explicitly specified GUI option is forwarded to git-mergetool--lib.sh; + * empty or unset means "use the difftool.guiDefault config or default to + * false". + */ + if (use_gui_tool == 1) setenv("GIT_MERGETOOL_GUI", "true", 1); - else if (difftool_cmd) { + else if (use_gui_tool == 0) + setenv("GIT_MERGETOOL_GUI", "false", 1); + + if (difftool_cmd) { if (*difftool_cmd) setenv("GIT_DIFF_TOOL", difftool_cmd, 1); else diff --git a/git-difftool--helper.sh b/git-difftool--helper.sh index 992124cc67c..e4e820e6809 100755 --- a/git-difftool--helper.sh +++ b/git-difftool--helper.sh @@ -75,6 +75,11 @@ then merge_tool="$GIT_DIFF_TOOL" else merge_tool="$(get_merge_tool)" + subshell_exit_status=$? + if test $subshell_exit_status -gt 1 + then + exit $subshell_exit_status + fi fi fi diff --git a/git-mergetool--lib.sh b/git-mergetool--lib.sh index 9f99201bcca..1ff26170ffc 100644 --- a/git-mergetool--lib.sh +++ b/git-mergetool--lib.sh @@ -97,7 +97,42 @@ merge_mode () { test "$TOOL_MODE" = merge } +get_gui_default () { + if diff_mode + then + GUI_DEFAULT_KEY="difftool.guiDefault" + else + GUI_DEFAULT_KEY="mergetool.guiDefault" + fi + GUI_DEFAULT_CONFIG_LCASE=$(git config --default false --get "$GUI_DEFAULT_KEY" | tr 'A-Z' 'a-z') + if test "$GUI_DEFAULT_CONFIG_LCASE" = "auto" + then + if test -n "$DISPLAY" + then + GUI_DEFAULT=true + else + GUI_DEFAULT=false + fi + else + GUI_DEFAULT=$(git config --default false --bool --get "$GUI_DEFAULT_KEY") + subshell_exit_status=$? + if test $subshell_exit_status -ne 0 + then + exit $subshell_exit_status + fi + fi + echo $GUI_DEFAULT +} + gui_mode () { + if test -z "$GIT_MERGETOOL_GUI" + then + GIT_MERGETOOL_GUI=$(get_gui_default) + if test $? -ne 0 + then + exit 2 + fi + fi test "$GIT_MERGETOOL_GUI" = true } @@ -467,6 +502,11 @@ get_merge_tool () { is_guessed=false # Check if a merge tool has been configured merge_tool=$(get_configured_merge_tool) + subshell_exit_status=$? + if test $subshell_exit_status -gt "1" + then + exit $subshell_exit_status + fi # Try to guess an appropriate merge tool if no tool has been set. if test -z "$merge_tool" then diff --git a/git-mergetool.sh b/git-mergetool.sh index f751d9cfe20..8a922893f75 100755 --- a/git-mergetool.sh +++ b/git-mergetool.sh @@ -451,7 +451,7 @@ print_noop_and_exit () { main () { prompt=$(git config --bool mergetool.prompt) - GIT_MERGETOOL_GUI=false + GIT_MERGETOOL_GUI= guessed_merge_tool=false orderfile= @@ -511,9 +511,14 @@ main () { if test -z "$merge_tool" then - if ! merge_tool=$(get_merge_tool) + merge_tool=$(get_merge_tool) + subshell_exit_status=$? + if test $subshell_exit_status = 1 then guessed_merge_tool=true + elif test $subshell_exit_status -gt 1 + then + exit $subshell_exit_status fi fi merge_keep_backup="$(git config --bool mergetool.keepBackup || echo true)" diff --git a/t/t7610-mergetool.sh b/t/t7610-mergetool.sh index 7b957022f1a..22b3a85b3e9 100755 --- a/t/t7610-mergetool.sh +++ b/t/t7610-mergetool.sh @@ -860,4 +860,42 @@ test_expect_success 'mergetool hideResolved' ' git commit -m "test resolved with mergetool" ' +test_expect_success 'mergetool with guiDefault' ' + 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 main && + + test_config mergetool.guiDefault auto && + DISPLAY=SOMETHING && export DISPLAY && + yes "" | git mergetool both && + yes "" | git mergetool file1 file1 && + + DISPLAY= && export DISPLAY && + yes "" | git mergetool file2 "spaced name" && + + test_config mergetool.guiDefault true && + yes "" | git mergetool subdir/file3 && + + yes "d" | git mergetool file11 && + yes "d" | git mergetool file12 && + yes "l" | git mergetool submod && + + echo "gui main updated" >expect && + test_cmp expect file1 && + + echo "main new" >expect && + test_cmp expect file2 && + + echo "gui main new sub" >expect && + test_cmp expect subdir/file3 && + + echo "branch1 submodule" >expect && + test_cmp expect submod/bar && + git commit -m "branch1 resolved with mergetool" +' + test_done diff --git a/t/t7800-difftool.sh b/t/t7800-difftool.sh index 24297e26ca0..59d3847bf87 100755 --- a/t/t7800-difftool.sh +++ b/t/t7800-difftool.sh @@ -155,6 +155,58 @@ test_expect_success 'difftool honors --gui' ' test_cmp expect actual ' +test_expect_success 'difftool with guiDefault auto selects gui tool when there is DISPLAY' ' + difftool_test_setup && + test_config merge.tool bogus-tool && + test_config diff.tool bogus-tool && + test_config diff.guitool test-tool && + test_config difftool.guiDefault auto && + DISPLAY=SOMETHING && export DISPLAY && + + echo branch >expect && + git difftool --no-prompt branch >actual && + test_cmp expect actual +' +test_expect_success 'difftool with guiDefault auto selects regular tool when no DISPLAY' ' + difftool_test_setup && + test_config diff.guitool bogus-tool && + test_config diff.tool test-tool && + test_config difftool.guiDefault Auto && + DISPLAY= && export DISPLAY && + + echo branch >expect && + git difftool --no-prompt branch >actual && + test_cmp expect actual +' + +test_expect_success 'difftool with guiDefault true selects gui tool' ' + difftool_test_setup && + test_config diff.tool bogus-tool && + test_config diff.guitool test-tool && + test_config difftool.guiDefault true && + + DISPLAY= && export DISPLAY && + echo branch >expect && + git difftool --no-prompt branch >actual && + test_cmp expect actual && + + DISPLAY=Something && export DISPLAY && + echo branch >expect && + git difftool --no-prompt branch >actual && + test_cmp expect actual +' + +test_expect_success 'difftool --no-gui trumps config guiDefault' ' + difftool_test_setup && + test_config diff.guitool bogus-tool && + test_config diff.tool test-tool && + test_config difftool.guiDefault true && + + echo branch >expect && + git difftool --no-prompt --no-gui branch >actual && + test_cmp expect actual +' + test_expect_success 'difftool --gui last setting wins' ' difftool_test_setup && : >expect &&