diff mbox series

[v2,3/3] difftool: allow running outside Git worktrees with --no-index

Message ID 8cca9f800c2ce269a2ae644e4c15dc4115d3b0e2.1552562701.git.gitgitgadget@gmail.com (mailing list archive)
State New, archived
Headers show
Series Allow difftool to be run outside of Git worktrees | expand

Commit Message

Linus Arver via GitGitGadget March 14, 2019, 11:25 a.m. UTC 
From: Johannes Schindelin <johannes.schindelin@gmx.de>

As far as this developer can tell, the conversion from a Perl script to
a built-in caused the regression in the difftool that it no longer runs
outside of a Git worktree (with `--no-index`, of course).

It is a bit embarrassing that it took over two years after retiring the
Perl version to discover this regression, but at least we now know, and
can do something, about it.

This fixes https://github.com/git-for-windows/git/issues/2123

Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
---
 builtin/difftool.c  | 13 ++++++++++---
 git.c               |  2 +-
 t/t7800-difftool.sh | 10 ++++++++++
 3 files changed, 21 insertions(+), 4 deletions(-)

Comments

Jeff King March 15, 2019, 3:17 a.m. UTC | #1 
On Thu, Mar 14, 2019 at 04:25:04AM -0700, Johannes Schindelin via GitGitGadget wrote:

> @@ -714,6 +714,7 @@ int cmd_difftool(int argc, const char **argv, const char *prefix)
>  			    "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()
>  	};

Much nicer.

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

And this fixed all of my nits from the previous version. The whole
series looks good to me.

-Peff
Johannes Schindelin March 15, 2019, 1:20 p.m. UTC | #2 
Hi Peff,

On Thu, 14 Mar 2019, Jeff King wrote:

> On Thu, Mar 14, 2019 at 04:25:04AM -0700, Johannes Schindelin via GitGitGadget wrote:
> 
> > @@ -714,6 +714,7 @@ int cmd_difftool(int argc, const char **argv, const char *prefix)
> >  			    "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()
> >  	};
> 
> Much nicer.
> 
> > +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
> > +'
> 
> And this fixed all of my nits from the previous version. The whole
> series looks good to me.

Thanks! (စ ͜ စ)
Dscho
diff mbox series

Patch

diff --git a/builtin/difftool.c b/builtin/difftool.c
index 31eece0c8d..4fff1e83f9 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, no_index = 0;
 	static char *difftool_cmd = NULL, *extcmd = NULL;
 	struct option builtin_difftool_options[] = {
 		OPT_BOOL('g', "gui", &use_gui_tool,
@@ -714,6 +714,7 @@  int cmd_difftool(int argc, const char **argv, const char *prefix)
 			    "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()
 	};
 
@@ -727,8 +728,14 @@  int cmd_difftool(int argc, const char **argv, const char *prefix)
 	if (tool_help)
 		return print_tool_help();
 
-	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);
diff --git a/git.c b/git.c
index 2014aab6b8..46365ed86a 100644
--- a/git.c
+++ b/git.c
@@ -491,7 +491,7 @@  static struct cmd_struct commands[] = {
 	{ "diff-files", cmd_diff_files, RUN_SETUP | NEED_WORK_TREE | NO_PARSEOPT },
 	{ "diff-index", cmd_diff_index, RUN_SETUP | NO_PARSEOPT },
 	{ "diff-tree", cmd_diff_tree, RUN_SETUP | NO_PARSEOPT },
-	{ "difftool", cmd_difftool, RUN_SETUP | NEED_WORK_TREE },
+	{ "difftool", cmd_difftool, RUN_SETUP_GENTLY },
 	{ "fast-export", cmd_fast_export, RUN_SETUP },
 	{ "fetch", cmd_fetch, RUN_SETUP },
 	{ "fetch-pack", cmd_fetch_pack, RUN_SETUP | NO_PARSEOPT },
diff --git a/t/t7800-difftool.sh b/t/t7800-difftool.sh
index bb9a7f4ff9..480dd0633f 100755
--- a/t/t7800-difftool.sh
+++ b/t/t7800-difftool.sh
@@ -705,4 +705,14 @@  test_expect_success SYMLINKS 'difftool --dir-diff handles modified symlinks' '
 	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_done