From: Junio C Hamano <gitster@pobox.com>
Date: Sun, 19 May 2019 07:45:30 +0000 (+0900)
Subject: Merge branch 'dl/difftool-mergetool'
X-Git-Tag: v2.22.0-rc1~21
X-Git-Url: https://git.lorimer.id.au/gitweb.git/diff_plain/85ac27e04fd0cf1d086028e87f69ffb498c1451b?hp=-c

Merge branch 'dl/difftool-mergetool'

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
---

85ac27e04fd0cf1d086028e87f69ffb498c1451b
diff --combined builtin/difftool.c
index 04ffa1d943,2205de1214..53188df714
--- a/builtin/difftool.c
+++ b/builtin/difftool.c
@@@ -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[] = {
@@@ -34,11 -32,6 +33,6 @@@
  
  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,
@@@ -712,7 -707,6 +706,7 @@@
  			    "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()
  	};
  
@@@ -726,17 -720,15 +720,20 @@@
  	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 aaa4eed0bc,696eb49160..204a5acd66
--- a/git-mergetool--lib.sh
+++ b/git-mergetool--lib.sh
@@@ -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 480dd0633f,48b74c0434..6bac9ed180
--- a/t/t7800-difftool.sh
+++ b/t/t7800-difftool.sh
@@@ -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
@@@ -555,7 -571,7 +571,7 @@@
  	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