Merge branch 'dl/difftool-mergetool'
authorJunio C Hamano <gitster@pobox.com>
Sun, 19 May 2019 07:45:30 +0000 (16:45 +0900)
committerJunio C Hamano <gitster@pobox.com>
Sun, 19 May 2019 07:45:30 +0000 (16:45 +0900)
Update "git difftool" and "git mergetool" so that the combinations
of {diff,merge}.{tool,guitool} configuration variables serve as
fallback settings of each other in a sensible order.

* dl/difftool-mergetool:
difftool: fallback on merge.guitool
difftool: make --gui, --tool and --extcmd mutually exclusive
mergetool: fallback to tool when guitool unavailable
mergetool--lib: create gui_mode function
mergetool: use get_merge_tool function
t7610: add mergetool --gui tests
t7610: unsuppress output

1  2 
builtin/difftool.c
git-mergetool--lib.sh
t/t7800-difftool.sh
diff --combined builtin/difftool.c
index 04ffa1d943d6e3a22eb919a21ad775c0bb29a6db,2205de1214695f1741dd9a46002183328807de0e..53188df71438ad83186bcae37a8742704ae048a5
@@@ -11,7 -11,6 +11,7 @@@
   *
   * Copyright (C) 2016 Johannes Schindelin
   */
 +#define USE_THE_INDEX_COMPATIBILITY_MACROS
  #include "cache.h"
  #include "config.h"
  #include "builtin.h"
@@@ -24,7 -23,6 +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[] = {
  
  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;
@@@ -65,12 -58,14 +59,12 @@@ static int parse_index_info(char *p, in
        *mode2 = (int)strtol(p + 1, &p, 8);
        if (*p != ' ')
                return error("expected ' ', got '%c'", *p);
 -      if (get_oid_hex(++p, oid1))
 -              return error("expected object ID, got '%s'", p + 1);
 -      p += GIT_SHA1_HEXSZ;
 +      if (parse_oid_hex(++p, oid1, (const char **)&p))
 +              return error("expected object ID, got '%s'", p);
        if (*p != ' ')
                return error("expected ' ', got '%c'", *p);
 -      if (get_oid_hex(++p, oid2))
 -              return error("expected object ID, got '%s'", p + 1);
 -      p += GIT_SHA1_HEXSZ;
 +      if (parse_oid_hex(++p, oid2, (const char **)&p))
 +              return error("expected object ID, got '%s'", p);
        if (*p != ' ')
                return error("expected ' ', got '%c'", *p);
        *status = *++p;
@@@ -322,7 -317,7 +316,7 @@@ static int checkout_path(unsigned mode
        int ret;
  
        ce = make_transient_cache_entry(mode, oid, path, 0);
 -      ret = checkout_entry(ce, state, NULL);
 +      ret = checkout_entry(ce, state, NULL, NULL);
  
        discard_cache_entry(ce);
        return ret;
@@@ -688,7 -683,7 +682,7 @@@ static int run_file_diff(int prompt, co
  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, no_index = 0;
        static char *difftool_cmd = NULL, *extcmd = NULL;
        struct option builtin_difftool_options[] = {
                OPT_BOOL('g', "gui", &use_gui_tool,
                            "tool returns a non - zero exit code")),
                OPT_STRING('x', "extcmd", &extcmd, N_("command"),
                           N_("specify a custom command for viewing diffs")),
 +              OPT_ARGUMENT("no-index", &no_index, N_("passed to `diff`")),
                OPT_END()
        };
  
        if (tool_help)
                return print_tool_help();
  
 -      /* NEEDSWORK: once we no longer spawn anything, remove this */
 -      setenv(GIT_DIR_ENVIRONMENT, absolute_path(get_git_dir()), 1);
 -      setenv(GIT_WORK_TREE_ENVIRONMENT, absolute_path(get_git_work_tree()), 1);
 +      if (!no_index && !startup_info->have_repository)
 +              die(_("difftool requires worktree or --no-index"));
 +
 +      if (!no_index){
 +              setup_work_tree();
 +              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 && diff_gui_tool && *diff_gui_tool)
-               setenv("GIT_DIFF_TOOL", diff_gui_tool, 1);
+       if (use_gui_tool + !!difftool_cmd + !!extcmd > 1)
+               die(_("--gui, --tool and --extcmd are mutually exclusive"));
+       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 --combined git-mergetool--lib.sh
index aaa4eed0bc7fc2c5706853d39e4644c9d31e1e0d,696eb491609ca08a037e04b24cd01384f74d525d..204a5acd66f5444412f0088b8df6cb0ae2fd6b41
@@@ -80,14 -80,18 +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"
  }
@@@ -279,7 -283,6 +283,7 @@@ list_merge_tool_candidates () 
                fi
                tools="$tools gvimdiff diffuse diffmerge ecmerge"
                tools="$tools p4merge araxis bc codecompare"
 +              tools="$tools smerge"
        fi
        case "${VISUAL:-$EDITOR}" in
        *vim*)
@@@ -351,20 -354,36 +355,36 @@@ guess_merge_tool () 
  }
  
  get_configured_merge_tool () {
-       # If first argument is true, find the guitool instead
-       if test "$1" = true
-       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"
@@@ -404,14 -423,17 +424,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)
        # 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 --combined t/t7800-difftool.sh
index 480dd0633fd3e979aecca82784d38642232d44b4,48b74c04346eb061ed414dbb254543a11f87886f..6bac9ed180e7342b34401b2d6af9e83b4472f08d
@@@ -279,11 -279,27 +279,27 @@@ test_expect_success 'difftool + mergeto
        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
  '
  
@@@ -332,7 -348,7 +348,7 @@@ test_expect_success 'difftool --extcmd 
  test_expect_success 'difftool --extcmd cat arg2' '
        echo branch >expect &&
        git difftool --no-prompt \
 -              --extcmd sh\ -c\ \"cat\ \$2\" branch >actual &&
 +              --extcmd sh\ -c\ \"cat\ \\\"\$2\\\"\" branch >actual &&
        test_cmp expect actual
  '
  
@@@ -546,7 -562,7 +562,7 @@@ d
  done >actual
  EOF
  
 -test_expect_success SYMLINKS 'difftool --dir-diff --symlink without unstaged changes' '
 +test_expect_success SYMLINKS 'difftool --dir-diff --symlinks without unstaged changes' '
        cat >expect <<-EOF &&
        file
        $PWD/file
        sub/sub
        $PWD/sub/sub
        EOF
 -      git difftool --dir-diff --symlink \
 +      git difftool --dir-diff --symlinks \
                --extcmd "./.git/CHECK_SYMLINKS" branch HEAD &&
        test_cmp expect actual
  '
@@@ -705,14 -721,12 +721,22 @@@ test_expect_success SYMLINKS 'difftool 
        test_cmp expect actual
  '
  
 +test_expect_success 'outside worktree' '
 +      echo 1 >1 &&
 +      echo 2 >2 &&
 +      test_expect_code 1 nongit git \
 +              -c diff.tool=echo -c difftool.echo.cmd="echo \$LOCAL \$REMOTE" \
 +              difftool --no-prompt --no-index ../1 ../2 >actual &&
 +      echo "../1 ../2" >expect &&
 +      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