From f245194f9a13d5108c3a59fd4ab1770ae9fd5b65 Mon Sep 17 00:00:00 2001 From: Junio C Hamano Date: Fri, 22 May 2009 12:45:29 -0700 Subject: diff: change semantics of "ignore whitespace" options Traditionally, the --ignore-whitespace* options have merely meant to tell the diff output routine that some class of differences are not worth showing in the textual diff output, so that the end user has easier time to review the remaining (presumably more meaningful) changes. These options never affected the outcome of the command, given as the exit status when the --exit-code option was in effect (either directly or indirectly). When you have only whitespace changes, however, you might expect git diff -b --exit-code to report that there is _no_ change with zero exit status. Change the semantics of --ignore-whitespace* options to mean more than "omit showing the difference in text". The exit status, when --exit-code is in effect, is computed by checking if we found any differences at the path level, while diff frontends feed filepairs to the diffcore engine. When "ignore whitespace" options are in effect, we defer this determination until the very end of diffcore transformation. We simply do not know until the textual diff is generated, which comes very late in the pipeline. When --quiet is in effect, various diff frontends optimize by breaking out early from the loop that enumerates the filepairs, when we find the first path level difference; when --ignore-whitespace* is used the above change automatically disables this optimization. Signed-off-by: Junio C Hamano --- diff.c | 34 +++++++++++++++++++++++++++++++--- 1 file changed, 31 insertions(+), 3 deletions(-) (limited to 'diff.c') diff --git a/diff.c b/diff.c index cd35e0c2d..467925d93 100644 --- a/diff.c +++ b/diff.c @@ -2378,6 +2378,20 @@ int diff_setup_done(struct diff_options *options) if (count > 1) die("--name-only, --name-status, --check and -s are mutually exclusive"); + /* + * Most of the time we can say "there are changes" + * only by checking if there are changed paths, but + * --ignore-whitespace* options force us to look + * inside contets. + */ + + if (DIFF_XDL_TST(options, IGNORE_WHITESPACE) || + DIFF_XDL_TST(options, IGNORE_WHITESPACE_CHANGE) || + DIFF_XDL_TST(options, IGNORE_WHITESPACE_AT_EOL)) + DIFF_OPT_SET(options, DIFF_FROM_CONTENTS); + else + DIFF_OPT_CLR(options, DIFF_FROM_CONTENTS); + if (DIFF_OPT_TST(options, FIND_COPIES_HARDER)) options->detect_rename = DIFF_DETECT_COPY; @@ -3330,6 +3344,18 @@ free_queue: q->nr = q->alloc = 0; if (options->close_file) fclose(options->file); + + /* + * Report the contents level differences with HAS_CHANGES; + * diff_addremove/diff_change does not set the bit when + * DIFF_FROM_CONTENTS is in effect (e.g. with -w). + */ + if (DIFF_OPT_TST(options, DIFF_FROM_CONTENTS)) { + if (options->found_changes) + DIFF_OPT_SET(options, HAS_CHANGES); + else + DIFF_OPT_CLR(options, HAS_CHANGES); + } } static void diffcore_apply_filter(const char *filter) @@ -3466,7 +3492,7 @@ void diffcore_std(struct diff_options *options) diff_resolve_rename_copy(); diffcore_apply_filter(options->filter); - if (diff_queued_diff.nr) + if (diff_queued_diff.nr && !DIFF_OPT_TST(options, DIFF_FROM_CONTENTS)) DIFF_OPT_SET(options, HAS_CHANGES); else DIFF_OPT_CLR(options, HAS_CHANGES); @@ -3526,7 +3552,8 @@ void diff_addremove(struct diff_options *options, fill_filespec(two, sha1, mode); diff_queue(&diff_queued_diff, one, two); - DIFF_OPT_SET(options, HAS_CHANGES); + if (!DIFF_OPT_TST(options, DIFF_FROM_CONTENTS)) + DIFF_OPT_SET(options, HAS_CHANGES); } void diff_change(struct diff_options *options, @@ -3558,7 +3585,8 @@ void diff_change(struct diff_options *options, fill_filespec(two, new_sha1, new_mode); diff_queue(&diff_queued_diff, one, two); - DIFF_OPT_SET(options, HAS_CHANGES); + if (!DIFF_OPT_TST(options, DIFF_FROM_CONTENTS)) + DIFF_OPT_SET(options, HAS_CHANGES); } void diff_unmerge(struct diff_options *options, -- cgit v1.2.1 From 90b1994170900514a1ce7a3345e25cb7216915cc Mon Sep 17 00:00:00 2001 From: Junio C Hamano Date: Sat, 23 May 2009 01:15:35 -0700 Subject: diff: Rename QUIET internal option to QUICK The option "QUIET" primarily meant "find if we have _any_ difference as quick as possible and report", which means we often do not even have to look at blobs if we know the trees are different by looking at the higher level (e.g. "diff-tree A B"). As a side effect, because there is no point showing one change that we happened to have found first, it also enables NO_OUTPUT and EXIT_WITH_STATUS options, making the end result look quiet. Rename the internal option to QUICK to reflect this better; it also makes grepping the source tree much easier, as there are other kinds of QUIET option everywhere. Signed-off-by: Junio C Hamano --- diff.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) (limited to 'diff.c') diff --git a/diff.c b/diff.c index 467925d93..91d6ea21a 100644 --- a/diff.c +++ b/diff.c @@ -2452,7 +2452,7 @@ int diff_setup_done(struct diff_options *options) * to have found. It does not make sense not to return with * exit code in such a case either. */ - if (DIFF_OPT_TST(options, QUIET)) { + if (DIFF_OPT_TST(options, QUICK)) { options->output_format = DIFF_FORMAT_NO_OUTPUT; DIFF_OPT_SET(options, EXIT_WITH_STATUS); } @@ -2643,7 +2643,7 @@ int diff_opt_parse(struct diff_options *options, const char **av, int ac) else if (!strcmp(arg, "--exit-code")) DIFF_OPT_SET(options, EXIT_WITH_STATUS); else if (!strcmp(arg, "--quiet")) - DIFF_OPT_SET(options, QUIET); + DIFF_OPT_SET(options, QUICK); else if (!strcmp(arg, "--ext-diff")) DIFF_OPT_SET(options, ALLOW_EXTERNAL); else if (!strcmp(arg, "--no-ext-diff")) -- cgit v1.2.1 From 97bf2a08095197f43b20b3bc1124552ae24d71bf Mon Sep 17 00:00:00 2001 From: Jim Meyering Date: Sun, 30 Aug 2009 22:27:02 +0200 Subject: diff.c: fix typoes in comments Should be squashed when we reroll 'next' into the main commit. --- diff.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) (limited to 'diff.c') diff --git a/diff.c b/diff.c index 91d6ea21a..24bd3fce3 100644 --- a/diff.c +++ b/diff.c @@ -2382,7 +2382,7 @@ int diff_setup_done(struct diff_options *options) * Most of the time we can say "there are changes" * only by checking if there are changed paths, but * --ignore-whitespace* options force us to look - * inside contets. + * inside contents. */ if (DIFF_XDL_TST(options, IGNORE_WHITESPACE) || @@ -3346,7 +3346,7 @@ free_queue: fclose(options->file); /* - * Report the contents level differences with HAS_CHANGES; + * Report the content-level differences with HAS_CHANGES; * diff_addremove/diff_change does not set the bit when * DIFF_FROM_CONTENTS is in effect (e.g. with -w). */ -- cgit v1.2.1