Skip to content

Commit f245194

Browse files
committed
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 <[email protected]>
1 parent 0a53e9d commit f245194

File tree

4 files changed

+97
-4
lines changed

4 files changed

+97
-4
lines changed

diff.c

Lines changed: 31 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -2378,6 +2378,20 @@ int diff_setup_done(struct diff_options *options)
23782378
if (count > 1)
23792379
die("--name-only, --name-status, --check and -s are mutually exclusive");
23802380

2381+
/*
2382+
* Most of the time we can say "there are changes"
2383+
* only by checking if there are changed paths, but
2384+
* --ignore-whitespace* options force us to look
2385+
* inside contets.
2386+
*/
2387+
2388+
if (DIFF_XDL_TST(options, IGNORE_WHITESPACE) ||
2389+
DIFF_XDL_TST(options, IGNORE_WHITESPACE_CHANGE) ||
2390+
DIFF_XDL_TST(options, IGNORE_WHITESPACE_AT_EOL))
2391+
DIFF_OPT_SET(options, DIFF_FROM_CONTENTS);
2392+
else
2393+
DIFF_OPT_CLR(options, DIFF_FROM_CONTENTS);
2394+
23812395
if (DIFF_OPT_TST(options, FIND_COPIES_HARDER))
23822396
options->detect_rename = DIFF_DETECT_COPY;
23832397

@@ -3330,6 +3344,18 @@ void diff_flush(struct diff_options *options)
33303344
q->nr = q->alloc = 0;
33313345
if (options->close_file)
33323346
fclose(options->file);
3347+
3348+
/*
3349+
* Report the contents level differences with HAS_CHANGES;
3350+
* diff_addremove/diff_change does not set the bit when
3351+
* DIFF_FROM_CONTENTS is in effect (e.g. with -w).
3352+
*/
3353+
if (DIFF_OPT_TST(options, DIFF_FROM_CONTENTS)) {
3354+
if (options->found_changes)
3355+
DIFF_OPT_SET(options, HAS_CHANGES);
3356+
else
3357+
DIFF_OPT_CLR(options, HAS_CHANGES);
3358+
}
33333359
}
33343360

33353361
static void diffcore_apply_filter(const char *filter)
@@ -3466,7 +3492,7 @@ void diffcore_std(struct diff_options *options)
34663492
diff_resolve_rename_copy();
34673493
diffcore_apply_filter(options->filter);
34683494

3469-
if (diff_queued_diff.nr)
3495+
if (diff_queued_diff.nr && !DIFF_OPT_TST(options, DIFF_FROM_CONTENTS))
34703496
DIFF_OPT_SET(options, HAS_CHANGES);
34713497
else
34723498
DIFF_OPT_CLR(options, HAS_CHANGES);
@@ -3526,7 +3552,8 @@ void diff_addremove(struct diff_options *options,
35263552
fill_filespec(two, sha1, mode);
35273553

35283554
diff_queue(&diff_queued_diff, one, two);
3529-
DIFF_OPT_SET(options, HAS_CHANGES);
3555+
if (!DIFF_OPT_TST(options, DIFF_FROM_CONTENTS))
3556+
DIFF_OPT_SET(options, HAS_CHANGES);
35303557
}
35313558

35323559
void diff_change(struct diff_options *options,
@@ -3558,7 +3585,8 @@ void diff_change(struct diff_options *options,
35583585
fill_filespec(two, new_sha1, new_mode);
35593586

35603587
diff_queue(&diff_queued_diff, one, two);
3561-
DIFF_OPT_SET(options, HAS_CHANGES);
3588+
if (!DIFF_OPT_TST(options, DIFF_FROM_CONTENTS))
3589+
DIFF_OPT_SET(options, HAS_CHANGES);
35623590
}
35633591

35643592
void diff_unmerge(struct diff_options *options,

diff.h

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -66,6 +66,7 @@ typedef void (*diff_format_fn_t)(struct diff_queue_struct *q,
6666
#define DIFF_OPT_DIRSTAT_CUMULATIVE (1 << 19)
6767
#define DIFF_OPT_DIRSTAT_BY_FILE (1 << 20)
6868
#define DIFF_OPT_ALLOW_TEXTCONV (1 << 21)
69+
#define DIFF_OPT_DIFF_FROM_CONTENTS (1 << 22)
6970
#define DIFF_OPT_TST(opts, flag) ((opts)->flags & DIFF_OPT_##flag)
7071
#define DIFF_OPT_SET(opts, flag) ((opts)->flags |= DIFF_OPT_##flag)
7172
#define DIFF_OPT_CLR(opts, flag) ((opts)->flags &= ~DIFF_OPT_##flag)

t/t4037-whitespace-status.sh

Lines changed: 63 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,63 @@
1+
#!/bin/sh
2+
3+
test_description='diff --exit-code with whitespace'
4+
. ./test-lib.sh
5+
6+
test_expect_success setup '
7+
mkdir a b &&
8+
echo >c &&
9+
echo >a/d &&
10+
echo >b/e &&
11+
git add . &&
12+
test_tick &&
13+
git commit -m initial &&
14+
echo " " >a/d &&
15+
test_tick &&
16+
git commit -a -m second &&
17+
echo " " >a/d &&
18+
echo " " >b/e &&
19+
git add a/d
20+
'
21+
22+
test_expect_success 'diff-tree --exit-code' '
23+
test_must_fail git diff --exit-code HEAD^ HEAD &&
24+
test_must_fail git diff-tree --exit-code HEAD^ HEAD
25+
'
26+
27+
test_expect_success 'diff-tree -b --exit-code' '
28+
git diff -b --exit-code HEAD^ HEAD &&
29+
git diff-tree -b -p --exit-code HEAD^ HEAD &&
30+
git diff-tree -b --exit-code HEAD^ HEAD
31+
'
32+
33+
test_expect_success 'diff-index --cached --exit-code' '
34+
test_must_fail git diff --cached --exit-code HEAD &&
35+
test_must_fail git diff-index --cached --exit-code HEAD
36+
'
37+
38+
test_expect_success 'diff-index -b -p --cached --exit-code' '
39+
git diff -b --cached --exit-code HEAD &&
40+
git diff-index -b -p --cached --exit-code HEAD
41+
'
42+
43+
test_expect_success 'diff-index --exit-code' '
44+
test_must_fail git diff --exit-code HEAD &&
45+
test_must_fail git diff-index --exit-code HEAD
46+
'
47+
48+
test_expect_success 'diff-index -b -p --exit-code' '
49+
git diff -b --exit-code HEAD &&
50+
git diff-index -b -p --exit-code HEAD
51+
'
52+
53+
test_expect_success 'diff-files --exit-code' '
54+
test_must_fail git diff --exit-code &&
55+
test_must_fail git diff-files --exit-code
56+
'
57+
58+
test_expect_success 'diff-files -b -p --exit-code' '
59+
git diff -b --exit-code &&
60+
git diff-files -b -p --exit-code
61+
'
62+
63+
test_done

tree-diff.c

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -286,7 +286,8 @@ int diff_tree(struct tree_desc *t1, struct tree_desc *t2, const char *base, stru
286286
int baselen = strlen(base);
287287

288288
for (;;) {
289-
if (DIFF_OPT_TST(opt, QUIET) && DIFF_OPT_TST(opt, HAS_CHANGES))
289+
if (DIFF_OPT_TST(opt, QUIET) &&
290+
DIFF_OPT_TST(opt, HAS_CHANGES))
290291
break;
291292
if (opt->nr_paths) {
292293
skip_uninteresting(t1, base, baselen, opt);

0 commit comments

Comments
 (0)