[PATCH] diff: squelch empty diffs even more

[PATCH] diff: squelch empty diffs even more

[René Scharfe]

When we compare two non-tracked files or explicitly specify
--no-index then the suggestion to run git-status is not helpful.
max_count is set to -2 in these cases; don't print the warning
then.

Signed-off-by: Rene Scharfe <rene.scharfe@lsfire.ath.cx>
---

 builtin-diff.c |    4 ++--
 1 files changed, 2 insertions(+), 2 deletions(-)

diff --git a/builtin-diff.c b/builtin-diff.c
index 6ed7b68..b2fd8c8 100644
--- a/builtin-diff.c
+++ b/builtin-diff.c
@@ -346,8 +346,8 @@ int cmd_diff(int argc, const char **argv, const char *prefix)
 	if (rev.diffopt.exit_with_status)
 		result = rev.diffopt.has_changes;
 
-	if ((rev.diffopt.output_format & DIFF_FORMAT_PATCH)
-	    && (1 < rev.diffopt.skip_stat_unmatch))
+	if ((rev.diffopt.output_format & DIFF_FORMAT_PATCH) &&
+	    1 < rev.diffopt.skip_stat_unmatch && rev.max_count != -2)
 		printf("Warning: %d path%s touched but unmodified. "
 		       "Consider running git-status.\n",
 		       rev.diffopt.skip_stat_unmatch - 1,

Re: [PATCH] diff: squelch empty diffs even more

[Junio C Hamano]

René Scharfe <rene.scharfe@lsrfire.ath.cx> writes:

> When we compare two non-tracked files or explicitly specify
> --no-index then the suggestion to run git-status is not helpful.

That observation is correct but how does skip_stat_unmatch count
up in such a case?  Shouldn't diffcore_skip_stat_unmatch()
function be taught about such a "fake" filepair that did not
come from git, and ignore the stat differences?

Re: [PATCH] diff: squelch empty diffs even more

[René Scharfe]

Junio C Hamano schrieb:
> René Scharfe <rene.scharfe@lsrfire.ath.cx> writes:
> 
>> When we compare two non-tracked files or explicitly specify
>> --no-index then the suggestion to run git-status is not helpful.
> 
> That observation is correct but how does skip_stat_unmatch count
> up in such a case?  Shouldn't diffcore_skip_stat_unmatch()
> function be taught about such a "fake" filepair that did not
> come from git, and ignore the stat differences?

Hmm.  Like this?

The patch adds a new diff_options bitfield member, no_index, that
is used instead of the special value of -2 of the rev_info field
max_count to indicate that the index is not to be used.  This makes
it possible to pass that flag down to diffcore_skip_stat_unmatch(),
which only has one diff_options parameter.

This could even become a cleanup if we removed all assignments of
max_count to a value of -2 (viz. replacement of a magic value with
a self-documenting field name) but I didn't dare to do that so late
in the rc game..

The no_index bit, if set, then tells diffcore_skip_stat_unmatch()
to not account for any skipped stat-mismatches, which avoids the
suggestion to run git-status.

René
---

 diff-lib.c |    8 ++++++--
 diff.c     |    3 ++-
 diff.h     |    1 +
 3 files changed, 9 insertions(+), 3 deletions(-)

diff --git a/diff-lib.c b/diff-lib.c
index 92c0e39..f5568c3 100644
--- a/diff-lib.c
+++ b/diff-lib.c
@@ -189,6 +189,7 @@ static int handle_diff_files_args(struct rev_info *revs,
 				!strcmp(argv[1], "--no-index")) {
 			revs->max_count = -2;
 			revs->diffopt.exit_with_status = 1;
+			revs->diffopt.no_index = 1;
 		}
 		else if (!strcmp(argv[1], "-q"))
 			*silent = 1;
@@ -204,8 +205,10 @@ static int handle_diff_files_args(struct rev_info *revs,
 		 */
 		read_cache();
 		if (!is_in_index(revs->diffopt.paths[0]) ||
-					!is_in_index(revs->diffopt.paths[1]))
+					!is_in_index(revs->diffopt.paths[1])) {
 			revs->max_count = -2;
+			revs->diffopt.no_index = 1;
+		}
 	}
 
 	/*
@@ -293,6 +296,7 @@ int setup_diff_no_index(struct rev_info *revs,
 	else
 		revs->diffopt.paths = argv + argc - 2;
 	revs->diffopt.nr_paths = 2;
+	revs->diffopt.no_index = 1;
 	revs->max_count = -2;
 	return 0;
 }
@@ -304,7 +308,7 @@ int run_diff_files_cmd(struct rev_info *revs, int argc, const char **argv)
 	if (handle_diff_files_args(revs, argc, argv, &silent_on_removed))
 		return -1;
 
-	if (revs->max_count == -2) {
+	if (revs->diffopt.no_index) {
 		if (revs->diffopt.nr_paths != 2)
 			return error("need two files/directories with --no-index");
 		if (queue_diff(&revs->diffopt, revs->diffopt.paths[0],
diff --git a/diff.c b/diff.c
index f884de7..97cc5bc 100644
--- a/diff.c
+++ b/diff.c
@@ -3185,7 +3185,8 @@ static void diffcore_skip_stat_unmatch(struct diff_options *diffopt)
 			 * to determine how many paths were dirty only
 			 * due to stat info mismatch.
 			 */
-			diffopt->skip_stat_unmatch++;
+			if (!diffopt->no_index)
+				diffopt->skip_stat_unmatch++;
 			diff_free_filepair(p);
 		}
 	}
diff --git a/diff.h b/diff.h
index de21f8e..4546aad 100644
--- a/diff.h
+++ b/diff.h
@@ -60,6 +60,7 @@ struct diff_options {
 		 color_diff_words:1,
 		 has_changes:1,
 		 quiet:1,
+		 no_index:1,
 		 allow_external:1,
 		 exit_with_status:1;
 	int context;

Re: [PATCH] diff: squelch empty diffs even more

[Junio C Hamano]

René Scharfe <rene.scharfe@lsrfire.ath.cx> writes:

> Hmm.  Like this?
>
> The patch adds a new diff_options bitfield member, no_index, that
> is used instead of the special value of -2 of the rev_info field
> max_count to indicate that the index is not to be used.  This makes
> it possible to pass that flag down to diffcore_skip_stat_unmatch(),
> which only has one diff_options parameter.
>
> This could even become a cleanup if we removed all assignments of
> max_count to a value of -2 (viz. replacement of a magic value with
> a self-documenting field name) but I didn't dare to do that so late
> in the rc game..
>
> The no_index bit, if set, then tells diffcore_skip_stat_unmatch()
> to not account for any skipped stat-mismatches, which avoids the
> suggestion to run git-status.

Yeah, I've always hated that -2 magic hack, and I think this is
the right way to go.  Dscho?

Re: [PATCH] diff: squelch empty diffs even more

[Johannes Schindelin]

Hi,

On Tue, 14 Aug 2007, Junio C Hamano wrote:

> Yeah, I've always hated that -2 magic hack, and I think this is the 
> right way to go.  Dscho?

Right.  It is how I should have done it to begin with.  If I find the time 
(and nobody beats be to it), I'll try to find the rest of the places 
tomorrow, and send a fix.

Ciao,
Dscho