On Wed, Feb 13, 2019 at 06:50:19PM +0000, Sylvain Lacaze wrote:
> I have “p4merge” setup as diff.tool and merge.tool, and it work great
> in normal operation (i.e., “p4merge” opens), e.g:
>
> $: git difftool HEAD~3 somePath/someFile.m
>
> However, when I use “—no-index” to compare 2 arbitrary file, difftool
> just uses diff:
>
> $: git difftool --no-index somePath/someFile.m somePath/someOtherFile.m
> $: git difftool --no-index --tool=p4merge somePath/someFile.m
> somePath/someOtherFile.m
>
> Both the above command just yield normal diff.
The root of the problem is that "git diff --no-index" does not enable
external diff tools by default. You can probably make it work by passing
"--no-index --ext-diff".
It seems to me that "git diff --no-index" should generally behave the
same as a regular "git diff" when possible, though. Maybe we should do
something like the patch below?
-- >8 --
Subject: [PATCH] diff: reuse diff setup for --no-index case
When "--no-index" is in effect (or implied by the arguments), git-diff
jumps early to a special code path to perform that diff. This means we
miss out on some settings like enabling --ext-diff and --textconv by
default.
Let's jump to the no-index path _after_ we've done more setup on
rev.diffopt. Some of these options won't affect us either way (e.g.,
items related to the index), but that makes it less likely for these two
paths to diverge again in the future.
Note that we also need to stop re-initializing the diffopt struct in
diff_no_index(). This should not be necessary, as it will already have
been initialized by cmd_diff() (and there are no other callers). That in
turn lets us drop the "repository" argument from diff_no_index (which
never made much sense, since the whole point is that you don't need a
repository).
Signed-off-by: Jeff King <peff@xxxxxxxx>
---
Generated with --inter-hunk-context=13 so you can see the actual list of
options.
builtin/diff.c | 7 ++++---
diff-no-index.c | 8 +-------
diff.h | 2 +-
t/t4053-diff-no-index.sh | 8 ++++++++
4 files changed, 14 insertions(+), 11 deletions(-)
diff --git a/builtin/diff.c b/builtin/diff.c
index 9f6109224b..458ce326c8 100644
--- a/builtin/diff.c
+++ b/builtin/diff.c
@@ -338,28 +338,29 @@ int cmd_diff(int argc, const char **argv, const char *prefix)
"--no-index" : "[--no-index]");
}
- if (no_index)
- /* If this is a no-index diff, just run it and exit there. */
- diff_no_index(the_repository, &rev, argc, argv);
/* Otherwise, we are doing the usual "git" diff */
rev.diffopt.skip_stat_unmatch = !!diff_auto_refresh_index;
/* Scale to real terminal size and respect statGraphWidth config */
rev.diffopt.stat_width = -1;
rev.diffopt.stat_graph_width = -1;
/* Default to let external and textconv be used */
rev.diffopt.flags.allow_external = 1;
rev.diffopt.flags.allow_textconv = 1;
/*
* Default to intent-to-add entries invisible in the
* index. This makes them show up as new files in diff-files
* and not at all in diff-cached.
*/
rev.diffopt.ita_invisible_in_index = 1;
+ if (no_index)
+ /* If this is a no-index diff, just run it and exit there. */
+ diff_no_index(&rev, argc, argv);
+
if (nongit)
die(_("Not a git repository"));
argc = setup_revisions(argc, argv, &rev, NULL);
diff --git a/diff-no-index.c b/diff-no-index.c
index 9414e922d1..6001baecd4 100644
--- a/diff-no-index.c
+++ b/diff-no-index.c
@@ -233,20 +233,14 @@ static void fixup_paths(const char **path, struct strbuf *replacement)
}
}
-void diff_no_index(struct repository *r,
- struct rev_info *revs,
+void diff_no_index(struct rev_info *revs,
int argc, const char **argv)
{
int i;
const char *paths[2];
struct strbuf replacement = STRBUF_INIT;
const char *prefix = revs->prefix;
- /*
- * FIXME: --no-index should not look at index and we should be
- * able to pass NULL repo. Maybe later.
- */
- repo_diff_setup(r, &revs->diffopt);
for (i = 1; i < argc - 2; ) {
int j;
if (!strcmp(argv[i], "--no-index"))
diff --git a/diff.h b/diff.h
index b512d0477a..a01e03985a 100644
--- a/diff.h
+++ b/diff.h
@@ -435,7 +435,7 @@ int diff_flush_patch_id(struct diff_options *, struct object_id *, int);
int diff_result_code(struct diff_options *, int);
-void diff_no_index(struct repository *, struct rev_info *, int, const char **);
+void diff_no_index(struct rev_info *, int, const char **);
int index_differs_from(struct repository *r, const char *def,
const struct diff_flags *flags,
diff --git a/t/t4053-diff-no-index.sh b/t/t4053-diff-no-index.sh
index 6e0dd6f9e5..4331b3118a 100755
--- a/t/t4053-diff-no-index.sh
+++ b/t/t4053-diff-no-index.sh
@@ -137,4 +137,12 @@ test_expect_success 'diff --no-index from repo subdir with absolute paths' '
test_cmp expect actual
'
+test_expect_success 'diff --no-index allows external diff' '
+ test_expect_code 1 \
+ env GIT_EXTERNAL_DIFF="echo external ;:" \
+ git diff --no-index non/git/a non/git/b >actual &&
+ echo external >expect &&
+ test_cmp expect actual
+'
+
test_done
--
2.21.0.rc0.586.gffba1126a0
