Re: filtering out mode-change-only changes
Junio C Hamano <gitster@pobox.com> writes:
Show 7 quoted lines
> Having said that, if we _were_ to do this built-in, an obvious logical
> place to do so is to define a new DIFF_OPT_IGNORE_EXECUTABLE_BIT, teach
> "--ignore-executable-bit" command line option to diff_opt_parse(), and
> then teach diff_resolve_rename_copy() to consider this bit when the code
> originally set DIFF_STATUS_MODIFIED. Instead, the updated code that is
> working under --ignore-executable-bit option would drop such a filepair
> from diff_queued_diff.
A patch to do so may look like this (untested, of course). A few things to note on the changes:
- The "header" strbuf holds the lines starting from "diff --git" and the
meta-information lines such as mode change, similarity, etc. It is
passed to the the interface code fn_out_consume() via the xdiff
machinery when an actual diff is found and emitted before the first
hunk of the diff.
- The must_show_header toggle is set at the strategic places when
information is added to the "header" strbuf that makes the output for
this filepair a must, even if it turns out that xdiff machinery does
not find any content changes. Before this patch, we flipped this
toggle when we noticed a mode change, so that the header is shown even
if there is no content change. The patch has to make make it
conditional, which is what the first hunk is about.
- The second hunk is not related but I think it is a worthy bit-rot fix.
The original code before "must_show_header" was introduced showed the
header upfront unless we are ignoring any whitespace changes, the
reasoning behind it being that two different blobs may produce no patch
under --ignore-space-change and in such a case we do not want to show
anything for the filepair. When 296c6bb (diff: fix "git show -C -C"
output when renaming a binary file, 2010-05-26) introduced
"must_show_header", it retained that logic.
But I think it was a mistake. If xdiff machinery decides there is no
patch to show, taking the --ignore-space-change into account, our
fn_out_consume() won't be called so we won't show the header
unnecessarily. And when we do see a line of patch, fn_out_consume()
will show the header.
diff.c | 31 +++++++++++++++++++++++++++++--
diff.h | 1 +
2 files changed, 30 insertions(+), 2 deletions(-)
diff --git a/Makefile b/Makefile
old mode 100644
new mode 100755
diff --git a/diff.c b/diff.c
index a1c06b5..acf7232 100644
--- a/diff.c
+++ b/diff.c
@@ -2152,7 +2152,8 @@ static void builtin_diff(const char *name_a,
if (one->mode != two->mode) {
strbuf_addf(&header, "%s%sold mode %06o%s\n", line_prefix, set, one->mode, reset);
strbuf_addf(&header, "%s%snew mode %06o%s\n", line_prefix, set, two->mode, reset);
- must_show_header = 1;
+ if (!DIFF_OPT_TST(o, IGNORE_MODE_CHANGE))
+ must_show_header = 1;
}
if (xfrm_msg)
strbuf_addstr(&header, xfrm_msg);
@@ -2207,7 +2208,7 @@ static void builtin_diff(const char *name_a,
struct emit_callback ecbdata;
const struct userdiff_funcname *pe;
- if (!DIFF_XDL_TST(o, WHITESPACE_FLAGS) || must_show_header) {
+ if (must_show_header) {
fprintf(o->file, "%s", header.buf);
strbuf_reset(&header);
}
@@ -3446,6 +3447,8 @@ int diff_opt_parse(struct diff_options *options, const char **av, int ac)
}
else if (!strcmp(arg, "--no-renames"))
options->detect_rename = 0;
+ else if (!strcmp(arg, "--ignore-mode-change"))
+ DIFF_OPT_SET(options, IGNORE_MODE_CHANGE);
else if (!strcmp(arg, "--relative"))
DIFF_OPT_SET(options, RELATIVE_NAME);
else if (!prefixcmp(arg, "--relative=")) {
@@ -4509,10 +4512,34 @@ void diffcore_fix_diff_index(struct diff_options *options)
qsort(q->queue, q->nr, sizeof(q->queue[0]), diffnamecmp);
}
+static void diffcore_ignore_mode_change(struct diff_options *diffopt)
+{
+ int i;
+ struct diff_queue_struct *q = &diff_queued_diff;
+ struct diff_queue_struct outq;
+ DIFF_QUEUE_CLEAR(&outq);
+
+ for (i = 0; i < q->nr; i++) {
+ struct diff_filepair *p = q->queue[i];
+
+ if (DIFF_FILE_VALID(p->one) &&
+ DIFF_FILE_VALID(p->two) &&
+ (p->one->sha1_valid && p->two->sha1_valid) &&
+ !hashcmp(p->one->sha1, p->two->sha1))
+ diff_free_filepair(p); /* skip this */
+ else
+ diff_q(&outq, p);
+ }
+ free(q->queue);
+ *q = outq;
+}
+
void diffcore_std(struct diff_options *options)
{
if (options->skip_stat_unmatch)
diffcore_skip_stat_unmatch(options);
+ if (DIFF_OPT_TST(options, IGNORE_MODE_CHANGE))
+ diffcore_ignore_mode_change(options);
if (!options->found_follow) {
/* See try_to_follow_renames() in tree-diff.c */
if (options->break_opt != -1)
diff --git a/diff.h b/diff.h
index 7af5f1e..fabcc96 100644
--- a/diff.h
+++ b/diff.h
@@ -82,6 +82,7 @@ typedef struct strbuf *(*diff_prefix_fn_t)(struct diff_options *opt, void *data)
#define DIFF_OPT_OVERRIDE_SUBMODULE_CONFIG (1 << 27)
#define DIFF_OPT_DIRSTAT_BY_LINE (1 << 28)
#define DIFF_OPT_FUNCCONTEXT (1 << 29)
+#define DIFF_OPT_IGNORE_MODE_CHANGE (1 << 30)
#define DIFF_OPT_TST(opts, flag) ((opts)->flags & DIFF_OPT_##flag)
#define DIFF_OPT_SET(opts, flag) ((opts)->flags |= DIFF_OPT_##flag)
