From 446d12cb3fbee1b641f97336df3abe8968e3db59 Mon Sep 17 00:00:00 2001 From: Junio C Hamano Date: Thu, 26 Oct 2017 15:15:51 +0900 Subject: [PATCH 1/2] xdiff: reassign xpparm_t.flags bits We have packed the bits too tightly in such a way that it is not easy to add a new type of whitespace ignoring option, a new type of LCS algorithm, or a new type of post-cleanup heuristics. Reorder bits a bit to give room for these three classes of options to grow. Also make use of XDF_WHITESPACE_FLAGS macro where we check any of these bits are on, instead of using DIFF_XDL_TST() macro on individual possibilities. That way, the "is any of the bits on?" code does not have to change when we add more ways to ignore whitespaces. While at it, add a comment in front of the bit definitions to clarify in which structure these defined bits may appear. Signed-off-by: Junio C Hamano --- diff.c | 4 +--- xdiff/xdiff.h | 26 +++++++++++++++----------- 2 files changed, 16 insertions(+), 14 deletions(-) diff --git a/diff.c b/diff.c index 74283d9001..790250fe86 100644 --- a/diff.c +++ b/diff.c @@ -3434,9 +3434,7 @@ void diff_setup_done(struct diff_options *options) * inside contents. */ - if (DIFF_XDL_TST(options, IGNORE_WHITESPACE) || - DIFF_XDL_TST(options, IGNORE_WHITESPACE_CHANGE) || - DIFF_XDL_TST(options, IGNORE_WHITESPACE_AT_EOL)) + if ((options->xdl_opts & XDF_WHITESPACE_FLAGS)) DIFF_OPT_SET(options, DIFF_FROM_CONTENTS); else DIFF_OPT_CLR(options, DIFF_FROM_CONTENTS); diff --git a/xdiff/xdiff.h b/xdiff/xdiff.h index b090ad8eac..cbf5d8e166 100644 --- a/xdiff/xdiff.h +++ b/xdiff/xdiff.h @@ -27,22 +27,26 @@ extern "C" { #endif /* #ifdef __cplusplus */ +/* xpparm_t.flags */ +#define XDF_NEED_MINIMAL (1 << 0) -#define XDF_NEED_MINIMAL (1 << 1) -#define XDF_IGNORE_WHITESPACE (1 << 2) -#define XDF_IGNORE_WHITESPACE_CHANGE (1 << 3) -#define XDF_IGNORE_WHITESPACE_AT_EOL (1 << 4) -#define XDF_WHITESPACE_FLAGS (XDF_IGNORE_WHITESPACE | XDF_IGNORE_WHITESPACE_CHANGE | XDF_IGNORE_WHITESPACE_AT_EOL) - -#define XDF_PATIENCE_DIFF (1 << 5) -#define XDF_HISTOGRAM_DIFF (1 << 6) -#define XDF_DIFF_ALGORITHM_MASK (XDF_PATIENCE_DIFF | XDF_HISTOGRAM_DIFF) -#define XDF_DIFF_ALG(x) ((x) & XDF_DIFF_ALGORITHM_MASK) +#define XDF_IGNORE_WHITESPACE (1 << 1) +#define XDF_IGNORE_WHITESPACE_CHANGE (1 << 2) +#define XDF_IGNORE_WHITESPACE_AT_EOL (1 << 3) +#define XDF_WHITESPACE_FLAGS (XDF_IGNORE_WHITESPACE | \ + XDF_IGNORE_WHITESPACE_CHANGE | \ + XDF_IGNORE_WHITESPACE_AT_EOL) #define XDF_IGNORE_BLANK_LINES (1 << 7) -#define XDF_INDENT_HEURISTIC (1 << 8) +#define XDF_PATIENCE_DIFF (1 << 14) +#define XDF_HISTOGRAM_DIFF (1 << 15) +#define XDF_DIFF_ALGORITHM_MASK (XDF_PATIENCE_DIFF | XDF_HISTOGRAM_DIFF) +#define XDF_DIFF_ALG(x) ((x) & XDF_DIFF_ALGORITHM_MASK) +#define XDF_INDENT_HEURISTIC (1 << 23) + +/* xdemitconf_t.flags */ #define XDL_EMIT_FUNCNAMES (1 << 0) #define XDL_EMIT_FUNCCONTEXT (1 << 2) From e9282f02b2f21118f3383608718e38efc3d967e1 Mon Sep 17 00:00:00 2001 From: Junio C Hamano Date: Thu, 26 Oct 2017 15:32:27 +0900 Subject: [PATCH 2/2] diff: --ignore-cr-at-eol A new option --ignore-cr-at-eol tells the diff machinery to treat a carriage-return at the end of a (complete) line as if it does not exist. Just like other "--ignore-*" options to ignore various kinds of whitespace differences, this will help reviewing the real changes you made without getting distracted by spurious CRLF<->LF conversion made by your editor program. Helped-by: Johannes Schindelin [jch: squashed in command line completion by Dscho] Signed-off-by: Junio C Hamano --- Documentation/diff-options.txt | 3 ++ Documentation/merge-strategies.txt | 5 ++-- contrib/completion/git-completion.bash | 2 +- diff.c | 2 ++ merge-recursive.c | 2 ++ t/t4015-diff-whitespace.sh | 28 +++++++++++++++++++ xdiff/xdiff.h | 4 ++- xdiff/xutils.c | 38 ++++++++++++++++++++++++-- 8 files changed, 78 insertions(+), 6 deletions(-) diff --git a/Documentation/diff-options.txt b/Documentation/diff-options.txt index 89cc0f48de..aa2c0ff74d 100644 --- a/Documentation/diff-options.txt +++ b/Documentation/diff-options.txt @@ -519,6 +519,9 @@ endif::git-format-patch[] --text:: Treat all files as text. +--ignore-cr-at-eol:: + Ignore carrige-return at the end of line when doing a comparison. + --ignore-space-at-eol:: Ignore changes in whitespace at EOL. diff --git a/Documentation/merge-strategies.txt b/Documentation/merge-strategies.txt index 2eb92b9327..030744910e 100644 --- a/Documentation/merge-strategies.txt +++ b/Documentation/merge-strategies.txt @@ -57,11 +57,12 @@ diff-algorithm=[patience|minimal|histogram|myers];; ignore-space-change;; ignore-all-space;; ignore-space-at-eol;; +ignore-cr-at-eol;; Treats lines with the indicated type of whitespace change as unchanged for the sake of a three-way merge. Whitespace changes mixed with other changes to a line are not ignored. - See also linkgit:git-diff[1] `-b`, `-w`, and - `--ignore-space-at-eol`. + See also linkgit:git-diff[1] `-b`, `-w`, + `--ignore-space-at-eol`, and `--ignore-cr-at-eol`. + * If 'their' version only introduces whitespace changes to a line, 'our' version is used; diff --git a/contrib/completion/git-completion.bash b/contrib/completion/git-completion.bash index ba7d8dddc9..8ad084252f 100644 --- a/contrib/completion/git-completion.bash +++ b/contrib/completion/git-completion.bash @@ -1399,7 +1399,7 @@ __git_diff_common_options="--stat --numstat --shortstat --summary --patch-with-stat --name-only --name-status --color --no-color --color-words --no-renames --check --full-index --binary --abbrev --diff-filter= - --find-copies-harder + --find-copies-harder --ignore-cr-at-eol --text --ignore-space-at-eol --ignore-space-change --ignore-all-space --ignore-blank-lines --exit-code --quiet --ext-diff --no-ext-diff diff --git a/diff.c b/diff.c index 790250fe86..dd14e4190c 100644 --- a/diff.c +++ b/diff.c @@ -3888,6 +3888,8 @@ int diff_opt_parse(struct diff_options *options, DIFF_XDL_SET(options, IGNORE_WHITESPACE_CHANGE); else if (!strcmp(arg, "--ignore-space-at-eol")) DIFF_XDL_SET(options, IGNORE_WHITESPACE_AT_EOL); + else if (!strcmp(arg, "--ignore-cr-at-eol")) + DIFF_XDL_SET(options, IGNORE_CR_AT_EOL); else if (!strcmp(arg, "--ignore-blank-lines")) DIFF_XDL_SET(options, IGNORE_BLANK_LINES); else if (!strcmp(arg, "--indent-heuristic")) diff --git a/merge-recursive.c b/merge-recursive.c index 7a7d55aabe..006b94baf2 100644 --- a/merge-recursive.c +++ b/merge-recursive.c @@ -2214,6 +2214,8 @@ int parse_merge_opt(struct merge_options *o, const char *s) DIFF_XDL_SET(o, IGNORE_WHITESPACE); else if (!strcmp(s, "ignore-space-at-eol")) DIFF_XDL_SET(o, IGNORE_WHITESPACE_AT_EOL); + else if (!strcmp(s, "ignore-cr-at-eol")) + DIFF_XDL_SET(o, IGNORE_CR_AT_EOL); else if (!strcmp(s, "renormalize")) o->renormalize = 1; else if (!strcmp(s, "no-renormalize")) diff --git a/t/t4015-diff-whitespace.sh b/t/t4015-diff-whitespace.sh index 289806d0c7..32dd54c21d 100755 --- a/t/t4015-diff-whitespace.sh +++ b/t/t4015-diff-whitespace.sh @@ -106,6 +106,8 @@ test_expect_success 'another test, without options' ' git diff -w -b --ignore-space-at-eol >out && test_cmp expect out && + git diff -w --ignore-cr-at-eol >out && + test_cmp expect out && tr "Q_" "\015 " <<-\EOF >expect && diff --git a/x b/x @@ -128,6 +130,9 @@ test_expect_success 'another test, without options' ' git diff -b --ignore-space-at-eol >out && test_cmp expect out && + git diff -b --ignore-cr-at-eol >out && + test_cmp expect out && + tr "Q_" "\015 " <<-\EOF >expect && diff --git a/x b/x index d99af23..22d9f73 100644 @@ -145,6 +150,29 @@ test_expect_success 'another test, without options' ' CR at end EOF git diff --ignore-space-at-eol >out && + test_cmp expect out && + + git diff --ignore-space-at-eol --ignore-cr-at-eol >out && + test_cmp expect out && + + tr "Q_" "\015 " <<-\EOF >expect && + diff --git a/x b/x + index_d99af23..22d9f73 100644 + --- a/x + +++ b/x + @@ -1,6 +1,6 @@ + -whitespace at beginning + -whitespace change + -whitespace in the middle + -whitespace at end + +_ whitespace at beginning + +whitespace_ _change + +white space in the middle + +whitespace at end__ + unchanged line + CR at end + EOF + git diff --ignore-cr-at-eol >out && test_cmp expect out ' diff --git a/xdiff/xdiff.h b/xdiff/xdiff.h index cbf5d8e166..51f8926215 100644 --- a/xdiff/xdiff.h +++ b/xdiff/xdiff.h @@ -33,9 +33,11 @@ extern "C" { #define XDF_IGNORE_WHITESPACE (1 << 1) #define XDF_IGNORE_WHITESPACE_CHANGE (1 << 2) #define XDF_IGNORE_WHITESPACE_AT_EOL (1 << 3) +#define XDF_IGNORE_CR_AT_EOL (1 << 4) #define XDF_WHITESPACE_FLAGS (XDF_IGNORE_WHITESPACE | \ XDF_IGNORE_WHITESPACE_CHANGE | \ - XDF_IGNORE_WHITESPACE_AT_EOL) + XDF_IGNORE_WHITESPACE_AT_EOL | \ + XDF_IGNORE_CR_AT_EOL) #define XDF_IGNORE_BLANK_LINES (1 << 7) diff --git a/xdiff/xutils.c b/xdiff/xutils.c index 04d7b32e4e..b2cbcc818f 100644 --- a/xdiff/xutils.c +++ b/xdiff/xutils.c @@ -156,6 +156,24 @@ int xdl_blankline(const char *line, long size, long flags) return (i == size); } +/* + * Have we eaten everything on the line, except for an optional + * CR at the very end? + */ +static int ends_with_optional_cr(const char *l, long s, long i) +{ + int complete = s && l[s-1] == '\n'; + + if (complete) + s--; + if (s == i) + return 1; + /* do not ignore CR at the end of an incomplete line */ + if (complete && s == i + 1 && l[i] == '\r') + return 1; + return 0; +} + int xdl_recmatch(const char *l1, long s1, const char *l2, long s2, long flags) { int i1, i2; @@ -170,7 +188,8 @@ int xdl_recmatch(const char *l1, long s1, const char *l2, long s2, long flags) /* * -w matches everything that matches with -b, and -b in turn - * matches everything that matches with --ignore-space-at-eol. + * matches everything that matches with --ignore-space-at-eol, + * which in turn matches everything that matches with --ignore-cr-at-eol. * * Each flavor of ignoring needs different logic to skip whitespaces * while we have both sides to compare. @@ -204,6 +223,14 @@ int xdl_recmatch(const char *l1, long s1, const char *l2, long s2, long flags) i1++; i2++; } + } else if (flags & XDF_IGNORE_CR_AT_EOL) { + /* Find the first difference and see how the line ends */ + while (i1 < s1 && i2 < s2 && l1[i1] == l2[i2]) { + i1++; + i2++; + } + return (ends_with_optional_cr(l1, s1, i1) && + ends_with_optional_cr(l2, s2, i2)); } /* @@ -230,9 +257,16 @@ static unsigned long xdl_hash_record_with_whitespace(char const **data, char const *top, long flags) { unsigned long ha = 5381; char const *ptr = *data; + int cr_at_eol_only = (flags & XDF_WHITESPACE_FLAGS) == XDF_IGNORE_CR_AT_EOL; for (; ptr < top && *ptr != '\n'; ptr++) { - if (XDL_ISSPACE(*ptr)) { + if (cr_at_eol_only) { + /* do not ignore CR at the end of an incomplete line */ + if (*ptr == '\r' && + (ptr + 1 < top && ptr[1] == '\n')) + continue; + } + else if (XDL_ISSPACE(*ptr)) { const char *ptr2 = ptr; int at_eol; while (ptr + 1 < top && XDL_ISSPACE(ptr[1])