{"thread":{"id":"33056","subject":"[PATCH] Allow combined diff to ignore white-spaces","startedAt":"2013-03-02T15:04:18Z","lastAt":"2013-03-14T21:43:19Z","messageCount":13,"participants":["Antoine Pelisse","Junio C Hamano","Johannes Sixt"],"isPatch":true,"patchVersion":1,"patchTotal":null},"messages":[{"id":"210512","messageId":"1362236658-17200-1-git-send-email-apelisse@gmail.com","threadId":"33056","inReplyTo":null,"subject":"[PATCH] Allow combined diff to ignore white-spaces","fromName":"Antoine Pelisse","fromEmail":"apelisse@gmail.com","sentAt":"2013-03-02T15:04:18Z","receivedAt":"2013-03-02T15:04:18Z","isPatch":true,"sender":{"key":"apelisse@gmail.com","avatar":"https://avatars.githubusercontent.com/u/1929644?v=4"},"body":"Currently, it's not possible to use the space-ignoring options (-b, -w,\n--ignore-space-at-eol) with combined diff. It makes it pretty impossible\nto read a merge between a branch that changed all tabs to spaces, and a\nbranch with functional changes.\n\nPass diff flags to diff engine, so that combined diff behaves as normal\ndiff does with spaces.\n\nIt also means that a conflict-less merge done using a ignore-* strategy\noption will not show any conflict if shown in combined-diff using the\nsame option.\n\nSigned-off-by: Antoine Pelisse <apelisse@gmail.com>\n---\nThat should be reviewed carefully as I'm not exactly sure that does make\nsense with the way combined-diff works.\nStill it seems natural to me to be able to remove the space in combined\ndiff as we do with normal diff. Especially as I unfortunately have to\ndeal with many space + feature merges that are very hard to analyze/handle\nif space differences can't be hidden.\n\nCheers,\nAntoine\n\n combine-diff.c |    6 +++---\n 1 file changed, 3 insertions(+), 3 deletions(-)\n\ndiff --git a/combine-diff.c b/combine-diff.c\nindex 35d41cd..7ca0a72 100644\n--- a/combine-diff.c\n+++ b/combine-diff.c\n@@ -215,7 +215,7 @@ static void combine_diff(const unsigned char *parent, unsigned int mode,\n \t\t\t struct sline *sline, unsigned int cnt, int n,\n \t\t\t int num_parent, int result_deleted,\n \t\t\t struct userdiff_driver *textconv,\n-\t\t\t const char *path)\n+\t\t\t const char *path, long flags)\n {\n \tunsigned int p_lno, lno;\n \tunsigned long nmask = (1UL << n);\n@@ -231,7 +231,7 @@ static void combine_diff(const unsigned char *parent, unsigned int mode,\n \tparent_file.ptr = grab_blob(parent, mode, &sz, textconv, path);\n \tparent_file.size = sz;\n \tmemset(&xpp, 0, sizeof(xpp));\n-\txpp.flags = 0;\n+\txpp.flags = flags;\n \tmemset(&xecfg, 0, sizeof(xecfg));\n \tmemset(&state, 0, sizeof(state));\n \tstate.nmask = nmask;\n@@ -962,7 +962,7 @@ static void show_patch_diff(struct combine_diff_path *elem, int num_parent,\n \t\t\t\t     elem->parent[i].mode,\n \t\t\t\t     &result_file, sline,\n \t\t\t\t     cnt, i, num_parent, result_deleted,\n-\t\t\t\t     textconv, elem->path);\n+\t\t\t\t     textconv, elem->path, opt->xdl_opts);\n \t}\n\n \tshow_hunks = make_hunks(sline, cnt, num_parent, dense);\n--\n1.7.9.5\n"},{"id":"210522","messageId":"7v38wdc4ei.fsf@alter.siamese.dyndns.org","threadId":"33056","inReplyTo":"1362236658-17200-1-git-send-email-apelisse@gmail.com","subject":"Re: [PATCH] Allow combined diff to ignore white-spaces","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2013-03-03T07:45:57Z","receivedAt":"2013-03-03T07:45:57Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Antoine Pelisse <apelisse@gmail.com> writes:\n\n> Currently, it's not possible to use the space-ignoring options (-b, -w,\n> --ignore-space-at-eol) with combined diff. It makes it pretty impossible\n> to read a merge between a branch that changed all tabs to spaces, and a\n> branch with functional changes.\n>\n> Pass diff flags to diff engine, so that combined diff behaves as normal\n> diff does with spaces.\n>\n> It also means that a conflict-less merge done using a ignore-* strategy\n> option will not show any conflict if shown in combined-diff using the\n> same option.\n>\n> Signed-off-by: Antoine Pelisse <apelisse@gmail.com>\n> ---\n> That should be reviewed carefully as I'm not exactly sure that does make\n> sense with the way combined-diff works.\n> Still it seems natural to me to be able to remove the space in combined\n> diff as we do with normal diff. Especially as I unfortunately have to\n> deal with many space + feature merges that are very hard to analyze/handle\n> if space differences can't be hidden.\n\nAssuming \"That\" at the beginning of the off-line comment refers to\n\"this patch\", I actually I do not think this patch needs to be\nreviewed carefully at all.\n\nThe change in your patch to make the internal pairwise diff to\nignore whitespaces is an obvious and a sensible first step for your\nstated goal.  With it, a block of lines where the postimage makes no\nchange other than whitespace changes from one parent and matches\nwith the other parent will see no hunks in either of the pair-wise\ndiff, and such a hunk will not appear in the final output, which is\nexactly what you want.\n\nWhat needs to be audited carefully is the part of combine diff logic\nthat this patch does *not* touch.\n\nAn example.  After collecting pairwise diffs between the shared\npostimage and individual parents, we align the hunks and coalesce\n\"identical\" preimage lines to form shared \"-\" (removed) lines.  I\nthink that step is done with byte-for-byte comparison.  If the\npostimage removes a line from one parent and a corresponding line\nfrom another parent, and if these corresponding lines in the parents\ndiffer only by whitespaces in a way the user told us to ignore with\n-b/-w/etc., you would need to tweak the logic to coalesce\ncorresponding \"lost\" lines in the append_lost() function. Otherwise,\nyou will see two \" -\" and \"- \" lines that remove these corresponding\nlines from the parents that are identical when you ignore\nwhitespaces.\n\nYou should add some tests; it would help you think about these\nissues through.\n\nFor example, in this history:\n\n       git init\n       seq 11 >ten\n       git add ten\n       git commit -m initial\n\n       seq 10 | sed -e 's/6/6 six/' -e 's/2/2 two/' >ten\n       echo 11 >>ten\n       git commit -m ten -a\n\n       git checkout -b side HEAD^\n       seq 10 | sed -e 's/^/  /' | sed -e 's/2/2 dos/' >ten\n       echo 11 >>ten\n       git commit -m indent -a\n\n       git merge master\n\n       seq 10 | sed -e 's/5/5 go/' -e 's/2/2 dois/' >ten\n       git commit -m merged -a\n\none side indents the original and tweaks some lines, while the other\nside tweaks some other lines without reindenting.  Most notably, the\nfifth line on one side is \"5\" while on the other side is \"  5\", and\nthis line is rewritten to \"5 go\" in the final.  The lost lines are\nnot coalesced to a single \"-- 5\" (nor \"--   5\") but shows that two\npreimages were different only by whitespaces.  Compare that with\nwhat happens to the final line \"11\" that gets lost in the result.\n\nThanks.\n\n> Cheers,\n> Antoine\n>\n>  combine-diff.c |    6 +++---\n>  1 file changed, 3 insertions(+), 3 deletions(-)\n>\n> diff --git a/combine-diff.c b/combine-diff.c\n> index 35d41cd..7ca0a72 100644\n> --- a/combine-diff.c\n> +++ b/combine-diff.c\n> @@ -215,7 +215,7 @@ static void combine_diff(const unsigned char *parent, unsigned int mode,\n>  \t\t\t struct sline *sline, unsigned int cnt, int n,\n>  \t\t\t int num_parent, int result_deleted,\n>  \t\t\t struct userdiff_driver *textconv,\n> -\t\t\t const char *path)\n> +\t\t\t const char *path, long flags)\n>  {\n>  \tunsigned int p_lno, lno;\n>  \tunsigned long nmask = (1UL << n);\n> @@ -231,7 +231,7 @@ static void combine_diff(const unsigned char *parent, unsigned int mode,\n>  \tparent_file.ptr = grab_blob(parent, mode, &sz, textconv, path);\n>  \tparent_file.size = sz;\n>  \tmemset(&xpp, 0, sizeof(xpp));\n> -\txpp.flags = 0;\n> +\txpp.flags = flags;\n>  \tmemset(&xecfg, 0, sizeof(xecfg));\n>  \tmemset(&state, 0, sizeof(state));\n>  \tstate.nmask = nmask;\n> @@ -962,7 +962,7 @@ static void show_patch_diff(struct combine_diff_path *elem, int num_parent,\n>  \t\t\t\t     elem->parent[i].mode,\n>  \t\t\t\t     &result_file, sline,\n>  \t\t\t\t     cnt, i, num_parent, result_deleted,\n> -\t\t\t\t     textconv, elem->path);\n> +\t\t\t\t     textconv, elem->path, opt->xdl_opts);\n>  \t}\n>\n>  \tshow_hunks = make_hunks(sline, cnt, num_parent, dense);\n> --\n> 1.7.9.5\n"},{"id":"210581","messageId":"CALWbr2z0eok-VOQX7DDZafevQUi7asCv_tD8=y6XA2PVYf52AQ@mail.gmail.com","threadId":"33056","inReplyTo":"7v38wdc4ei.fsf@alter.siamese.dyndns.org","subject":"Re: [PATCH] Allow combined diff to ignore white-spaces","fromName":"Antoine Pelisse","fromEmail":"apelisse@gmail.com","sentAt":"2013-03-04T18:17:19Z","receivedAt":"2013-03-04T18:17:19Z","isPatch":true,"sender":{"key":"apelisse@gmail.com","avatar":"https://avatars.githubusercontent.com/u/1929644?v=4"},"body":">> That should be reviewed carefully as I'm not exactly sure that does make\n>> sense with the way combined-diff works.\n>> Still it seems natural to me to be able to remove the space in combined\n>> diff as we do with normal diff. Especially as I unfortunately have to\n>> deal with many space + feature merges that are very hard to analyze/handle\n>> if space differences can't be hidden.\n>\n> Assuming \"That\" at the beginning of the off-line comment refers to\n> \"this patch\", I actually I do not think this patch needs to be\n> reviewed carefully at all.\n\nI actually meant: \"is that patch enough or is there anything I\nmissed\". Considering your answer, it looks like I did ;)\n\n> An example.  After collecting pairwise diffs between the shared\n> postimage and individual parents, we align the hunks and coalesce\n> \"identical\" preimage lines to form shared \"-\" (removed) lines.  I\n> think that step is done with byte-for-byte comparison.  If the\n> postimage removes a line from one parent and a corresponding line\n> from another parent, and if these corresponding lines in the parents\n> differ only by whitespaces in a way the user told us to ignore with\n> -b/-w/etc., you would need to tweak the logic to coalesce\n> corresponding \"lost\" lines in the append_lost() function. Otherwise,\n> you will see two \" -\" and \"- \" lines that remove these corresponding\n> lines from the parents that are identical when you ignore\n> whitespaces.\n\nThat is the part I missed, and it's nicely explained.\n\n> You should add some tests; it would help you think about these\n> issues through.\n\nI will definitely add some tests.\n\n> For example, in this history:\n>\n>        git init\n>        seq 11 >ten\n>        git add ten\n>        git commit -m initial\n>\n>        seq 10 | sed -e 's/6/6 six/' -e 's/2/2 two/' >ten\n>        echo 11 >>ten\n>        git commit -m ten -a\n>\n>        git checkout -b side HEAD^\n>        seq 10 | sed -e 's/^/  /' | sed -e 's/2/2 dos/' >ten\n>        echo 11 >>ten\n>        git commit -m indent -a\n>\n>        git merge master\n>\n>        seq 10 | sed -e 's/5/5 go/' -e 's/2/2 dois/' >ten\n>        git commit -m merged -a\n>\n> one side indents the original and tweaks some lines, while the other\n> side tweaks some other lines without reindenting.  Most notably, the\n> fifth line on one side is \"5\" while on the other side is \"  5\", and\n> this line is rewritten to \"5 go\" in the final.  The lost lines are\n> not coalesced to a single \"-- 5\" (nor \"--   5\") but shows that two\n> preimages were different only by whitespaces.  Compare that with\n> what happens to the final line \"11\" that gets lost in the result.\n\nIt feels incorrect to me to coalsesce \"- 5\" and \"-  5\" as it might\nlook incorrect to the user. But still the idea is appealing. I have an\nimplementation for that that requires more testing.\n\nUsing the exact example you gave, and running the latest next, I have\nthis output, where 11 is not coalesced.\nIs that a bug ?\n\ndiff --cc ten\nindex d213a99,ed40ab2..37c2af2\n--- a/ten\n+++ b/ten\n@@@ -1,11 -1,11 +1,10 @@@\n-   1\n-   2 dos\n-   3\n-   4\n-   5\n-   6\n-   7\n-   8\n-   9\n-   10\n- 11\n+ 1\n -2 two\n++2 dois\n+ 3\n+ 4\n -5\n -6 six\n++5 go\n++6\n+ 7\n+ 8\n+ 9\n+ 10\n -11\n"},{"id":"210583","messageId":"7vy5e3vwos.fsf@alter.siamese.dyndns.org","threadId":"33056","inReplyTo":"CALWbr2z0eok-VOQX7DDZafevQUi7asCv_tD8=y6XA2PVYf52AQ@mail.gmail.com","subject":"Re: [PATCH] Allow combined diff to ignore white-spaces","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2013-03-04T18:36:51Z","receivedAt":"2013-03-04T18:36:51Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Antoine Pelisse <apelisse@gmail.com> writes:\n\n> It feels incorrect to me to coalsesce \"- 5\" and \"-  5\" as it might\n> look incorrect to the user. But still the idea is appealing.\n\nThe users already need to see that when reading a regular patch with\none or more context lines and -b/-w/etc., anyway.  The context lines\nare made into context only because whitespace differences were\nignored, and in the regular unified patch format we can show only\none version, either from preimage or from postimage, and have to\npick one.  Coalescing \"- 5\" and \"-  5\" into \"--5\" or \"--  5\" by\npicking one or the other is the same thing, no?\n\n> Using the exact example you gave, and running the latest next, I have\n> this output, where 11 is not coalesced.\n> Is that a bug ?\n\nIt could be tickling a corner case because the removal is at the end\nof the file.  Perhaps adding 12 that is all common across three\nversions and see what happens?\n"},{"id":"210585","messageId":"CALWbr2xPJQ1tvyT6Q84ccn=k6fg8A1aUbJG7FQjjJGZGC5voGg@mail.gmail.com","threadId":"33056","inReplyTo":"7vy5e3vwos.fsf@alter.siamese.dyndns.org","subject":"Re: [PATCH] Allow combined diff to ignore white-spaces","fromName":"Antoine Pelisse","fromEmail":"apelisse@gmail.com","sentAt":"2013-03-04T18:50:33Z","receivedAt":"2013-03-04T18:50:33Z","isPatch":true,"sender":{"key":"apelisse@gmail.com","avatar":"https://avatars.githubusercontent.com/u/1929644?v=4"},"body":"On Mon, Mar 4, 2013 at 7:36 PM, Junio C Hamano <gitster@pobox.com> wrote:\n> Antoine Pelisse <apelisse@gmail.com> writes:\n>\n>> It feels incorrect to me to coalsesce \"- 5\" and \"-  5\" as it might\n>> look incorrect to the user. But still the idea is appealing.\n>\n> The users already need to see that when reading a regular patch with\n> one or more context lines and -b/-w/etc., anyway.  The context lines\n> are made into context only because whitespace differences were\n> ignored, and in the regular unified patch format we can show only\n> one version, either from preimage or from postimage, and have to\n> pick one.  Coalescing \"- 5\" and \"-  5\" into \"--5\" or \"--  5\" by\n> picking one or the other is the same thing, no?\n\nThat's all I needed to be convinced. I obviously don't care which one we pick.\n\n>> Using the exact example you gave, and running the latest next, I have\n>> this output, where 11 is not coalesced.\n>> Is that a bug ?\n>\n> It could be tickling a corner case because the removal is at the end\n> of the file.  Perhaps adding 12 that is all common across three\n> versions and see what happens?\n\nDoesn't make a difference. Still have \"- 11\" and \" -11\".\nI will try to have a look at it.\n"},{"id":"211253","messageId":"1363209683-10264-1-git-send-email-apelisse@gmail.com","threadId":"33056","inReplyTo":"7v38wdc4ei.fsf@alter.siamese.dyndns.org","subject":"[PATCH] Allow combined diff to ignore white-spaces","fromName":"Antoine Pelisse","fromEmail":"apelisse@gmail.com","sentAt":"2013-03-13T21:21:23Z","receivedAt":"2013-03-13T21:21:23Z","isPatch":true,"sender":{"key":"apelisse@gmail.com","avatar":"https://avatars.githubusercontent.com/u/1929644?v=4"},"body":"Currently, it's not possible to use the space-ignoring options (-b, -w,\n--ignore-space-at-eol) with combined diff. It makes it pretty impossible\nto read a merge between a branch that changed all tabs to spaces, and a\nbranch with functional changes.\n\nPass diff flags to diff engine, so that combined diff behaves as normal\ndiff does with spaces.\nAlso coalesce lines that are removed from both (or more) parents.\n\nIt also means that a conflict-less merge done using a ignore-* strategy\noption will not show any conflict if shown in combined-diff using the\nsame option.\n\nSigned-off-by: Antoine Pelisse <apelisse@gmail.com>\n---\nOK, I added some tests and coalesce similar lost lines (using the same flags\nwe used for diff.\n\n combine-diff.c           |   57 ++++++++++++++++++++++++++++++-----\n t/t4038-diff-combined.sh |   75 ++++++++++++++++++++++++++++++++++++++++++++++\n 2 files changed, 125 insertions(+), 7 deletions(-)\n\ndiff --git a/combine-diff.c b/combine-diff.c\nindex 35d41cd..0f33983 100644\n--- a/combine-diff.c\n+++ b/combine-diff.c\n@@ -5,6 +5,7 @@\n #include \"diffcore.h\"\n #include \"quote.h\"\n #include \"xdiff-interface.h\"\n+#include \"xdiff/xmacros.h\"\n #include \"log-tree.h\"\n #include \"refs.h\"\n #include \"userdiff.h\"\n@@ -122,7 +123,47 @@ static char *grab_blob(const unsigned char *sha1, unsigned int mode,\n \treturn blob;\n }\n\n-static void append_lost(struct sline *sline, int n, const char *line, int len)\n+static int match_string_spaces(const char *line1, int len1,\n+\t\t\t       const char *line2, int len2,\n+\t\t\t       long flags)\n+{\n+\tif (flags & XDF_WHITESPACE_FLAGS) {\n+\t\tfor (; len1 > 0 && XDL_ISSPACE(line1[len1 - 1]); len1--);\n+\t\tfor (; len2 > 0 && XDL_ISSPACE(line2[len2 - 1]); len2--);\n+\t}\n+\n+\tif (!(flags & (XDF_IGNORE_WHITESPACE | XDF_IGNORE_WHITESPACE_CHANGE)))\n+\t\treturn (len1 == len2 && !memcmp(line1, line2, len1));\n+\n+\twhile (len1 > 0 && len2 > 0) {\n+\t\tlen1--;\n+\t\tlen2--;\n+\t\tif (XDL_ISSPACE(line1[len1]) || XDL_ISSPACE(line2[len2])) {\n+\t\t\tif ((flags & XDF_IGNORE_WHITESPACE_CHANGE) &&\n+\t\t\t    (!XDL_ISSPACE(line1[len1]) || !XDL_ISSPACE(line2[len2])))\n+\t\t\t\treturn 0;\n+\n+\t\t\tfor (; len1 > 0 && XDL_ISSPACE(line1[len1]); len1--);\n+\t\t\tfor (; len2 > 0 && XDL_ISSPACE(line2[len2]); len2--);\n+\t\t}\n+\t\tif (line1[len1] != line2[len2])\n+\t\t\treturn 0;\n+\t}\n+\n+\tif (flags & XDF_IGNORE_WHITESPACE) {\n+\t\t// Consume remaining spaces\n+\t\tfor (; len1 > 0 && XDL_ISSPACE(line1[len1 - 1]); len1--);\n+\t\tfor (; len2 > 0 && XDL_ISSPACE(line2[len2 - 1]); len2--);\n+\t}\n+\n+\t// We matched full line1 and line2\n+\tif (!len1 && !len2)\n+\t\treturn 1;\n+\n+\treturn 0;\n+}\n+\n+static void append_lost(struct sline *sline, int n, const char *line, int len, long flags)\n {\n \tstruct lline *lline;\n \tunsigned long this_mask = (1UL<<n);\n@@ -133,8 +174,8 @@ static void append_lost(struct sline *sline, int n, const char *line, int len)\n \tif (sline->lost_head) {\n \t\tlline = sline->next_lost;\n \t\twhile (lline) {\n-\t\t\tif (lline->len == len &&\n-\t\t\t    !memcmp(lline->line, line, len)) {\n+\t\t\tif (match_string_spaces(lline->line, lline->len,\n+\t\t\t\t\t\tline, len, flags)) {\n \t\t\t\tlline->parent_map |= this_mask;\n \t\t\t\tsline->next_lost = lline->next;\n \t\t\t\treturn;\n@@ -162,6 +203,7 @@ struct combine_diff_state {\n \tint n;\n \tstruct sline *sline;\n \tstruct sline *lost_bucket;\n+\tlong flags;\n };\n\n static void consume_line(void *state_, char *line, unsigned long len)\n@@ -201,7 +243,7 @@ static void consume_line(void *state_, char *line, unsigned long len)\n \t\treturn; /* not in any hunk yet */\n \tswitch (line[0]) {\n \tcase '-':\n-\t\tappend_lost(state->lost_bucket, state->n, line+1, len-1);\n+\t\tappend_lost(state->lost_bucket, state->n, line+1, len-1, state->flags);\n \t\tbreak;\n \tcase '+':\n \t\tstate->sline[state->lno-1].flag |= state->nmask;\n@@ -215,7 +257,7 @@ static void combine_diff(const unsigned char *parent, unsigned int mode,\n \t\t\t struct sline *sline, unsigned int cnt, int n,\n \t\t\t int num_parent, int result_deleted,\n \t\t\t struct userdiff_driver *textconv,\n-\t\t\t const char *path)\n+\t\t\t const char *path, long flags)\n {\n \tunsigned int p_lno, lno;\n \tunsigned long nmask = (1UL << n);\n@@ -231,9 +273,10 @@ static void combine_diff(const unsigned char *parent, unsigned int mode,\n \tparent_file.ptr = grab_blob(parent, mode, &sz, textconv, path);\n \tparent_file.size = sz;\n \tmemset(&xpp, 0, sizeof(xpp));\n-\txpp.flags = 0;\n+\txpp.flags = flags;\n \tmemset(&xecfg, 0, sizeof(xecfg));\n \tmemset(&state, 0, sizeof(state));\n+\tstate.flags = flags;\n \tstate.nmask = nmask;\n \tstate.sline = sline;\n \tstate.lno = 1;\n@@ -962,7 +1005,7 @@ static void show_patch_diff(struct combine_diff_path *elem, int num_parent,\n \t\t\t\t     elem->parent[i].mode,\n \t\t\t\t     &result_file, sline,\n \t\t\t\t     cnt, i, num_parent, result_deleted,\n-\t\t\t\t     textconv, elem->path);\n+\t\t\t\t     textconv, elem->path, opt->xdl_opts);\n \t}\n\n \tshow_hunks = make_hunks(sline, cnt, num_parent, dense);\ndiff --git a/t/t4038-diff-combined.sh b/t/t4038-diff-combined.sh\nindex 614425a..ba8a56b 100755\n--- a/t/t4038-diff-combined.sh\n+++ b/t/t4038-diff-combined.sh\n@@ -113,4 +113,79 @@ test_expect_success 'check --cc --raw with forty trees' '\n \tgrep \"^::::::::::::::::::::::::::::::::::::::::[^:]\" out\n '\n\n+test_expect_success 'setup combined ignore spaces' '\n+\tgit checkout master &&\n+\t>test &&\n+\tgit add test &&\n+\tgit commit -m initial &&\n+\n+\techo \"\n+\talways coalesce\n+\teol space coalesce \\n\\\n+\tspace  change coalesce\n+\tall spa ces coalesce\n+\teol spaces \\n\\\n+\tspace  change\n+\tall spa ces\" >test &&\n+\tgit commit -m \"change three\" -a &&\n+\n+\tgit checkout -b side HEAD^ &&\n+\techo \"\n+\talways coalesce\n+\teol space coalesce\n+\tspace change coalesce\n+\tall spaces coalesce\n+\teol spaces\n+\tspace change\n+\tall spaces\" >test &&\n+\tgit commit -m indent -a &&\n+\n+\ttest_must_fail git merge master &&\n+\techo \"\n+\teol spaces \\n\\\n+\tspace  change\n+\tall spa ces\" > test &&\n+\tgit commit -m merged -a\n+'\n+\n+test_expect_success 'check combined output (no ignore space)' '\n+\tgit show | test_i18ngrep \"^-\\s*eol spaces\" &&\n+\tgit show | test_i18ngrep \"^-\\s*eol space coalesce\" &&\n+\tgit show | test_i18ngrep \"^-\\s*space change\" &&\n+\tgit show | test_i18ngrep \"^-\\s*space change coalesce\" &&\n+\tgit show | test_i18ngrep \"^-\\s*all spaces\" &&\n+\tgit show | test_i18ngrep \"^-\\s*all spaces coalesce\" &&\n+\tgit show | test_i18ngrep \"^--\\s*always coalesce\"\n+'\n+\n+test_expect_success 'check combined output (ignore space at eol)' '\n+\tgit show --ignore-space-at-eol | test_i18ngrep \"^\\s*eol spaces\" &&\n+\tgit show --ignore-space-at-eol | test_i18ngrep \"^--\\s*eol space coalesce\" &&\n+\tgit show --ignore-space-at-eol | test_i18ngrep \"^-\\s*space change\" &&\n+\tgit show --ignore-space-at-eol | test_i18ngrep \"^-\\s*space change coalesce\" &&\n+\tgit show --ignore-space-at-eol | test_i18ngrep \"^-\\s*all spaces\" &&\n+\tgit show --ignore-space-at-eol | test_i18ngrep \"^-\\s*all spaces coalesce\" &&\n+\tgit show --ignore-space-at-eol | test_i18ngrep \"^--\\s*always coalesce\"\n+'\n+\n+test_expect_success 'check combined output (ignore space change)' '\n+\tgit show -b | test_i18ngrep \"^\\s*eol spaces\" &&\n+\tgit show -b | test_i18ngrep \"^--\\s*eol space coalesce\" &&\n+\tgit show -b | test_i18ngrep \"^\\s*space  change\" &&\n+\tgit show -b | test_i18ngrep \"^--\\s*space change coalesce\" &&\n+\tgit show -b | test_i18ngrep \"^-\\s*all spaces\" &&\n+\tgit show -b | test_i18ngrep \"^-\\s*all spaces coalesce\" &&\n+\tgit show -b | test_i18ngrep \"^--\\s*always coalesce\"\n+'\n+\n+test_expect_success 'check combined output (ignore all spaces)' '\n+\tgit show -w | test_i18ngrep \"^\\s*eol spaces\" &&\n+\tgit show -w | test_i18ngrep \"^--\\s*eol space coalesce\" &&\n+\tgit show -w | test_i18ngrep \"^\\s*space  change\" &&\n+\tgit show -w | test_i18ngrep \"^--\\s*space change coalesce\" &&\n+\tgit show -w | test_i18ngrep \"^\\s*all spa ces\" &&\n+\tgit show -w | test_i18ngrep \"^--\\s*all spaces coalesce\" &&\n+\tgit show -w | test_i18ngrep \"^--\\s*always coalesce\"\n+'\n+\n test_done\n--\n1.7.9.5\n"},{"id":"211264","messageId":"7v4nge27fc.fsf@alter.siamese.dyndns.org","threadId":"33056","inReplyTo":"1363209683-10264-1-git-send-email-apelisse@gmail.com","subject":"Re: [PATCH] Allow combined diff to ignore white-spaces","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2013-03-13T23:42:31Z","receivedAt":"2013-03-13T23:42:31Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Antoine Pelisse <apelisse@gmail.com> writes:\n\n> Currently, it's not possible to use the space-ignoring options (-b, -w,\n> --ignore-space-at-eol) with combined diff. It makes it pretty impossible\n> to read a merge between a branch that changed all tabs to spaces, and a\n> branch with functional changes.\n>\n> Pass diff flags to diff engine, so that combined diff behaves as normal\n> diff does with spaces.\n> Also coalesce lines that are removed from both (or more) parents.\n>\n> It also means that a conflict-less merge done using a ignore-* strategy\n> option will not show any conflict if shown in combined-diff using the\n> same option.\n>\n> Signed-off-by: Antoine Pelisse <apelisse@gmail.com>\n> ---\n> OK, I added some tests and coalesce similar lost lines (using the same flags\n> we used for diff.\n>\n>  combine-diff.c           |   57 ++++++++++++++++++++++++++++++-----\n>  t/t4038-diff-combined.sh |   75 ++++++++++++++++++++++++++++++++++++++++++++++\n>  2 files changed, 125 insertions(+), 7 deletions(-)\n>\n> diff --git a/combine-diff.c b/combine-diff.c\n> index 35d41cd..0f33983 100644\n> --- a/combine-diff.c\n> +++ b/combine-diff.c\n> @@ -5,6 +5,7 @@\n>  #include \"diffcore.h\"\n>  #include \"quote.h\"\n>  #include \"xdiff-interface.h\"\n> +#include \"xdiff/xmacros.h\"\n>  #include \"log-tree.h\"\n>  #include \"refs.h\"\n>  #include \"userdiff.h\"\n> @@ -122,7 +123,47 @@ static char *grab_blob(const unsigned char *sha1, unsigned int mode,\n>  \treturn blob;\n>  }\n>\n> -static void append_lost(struct sline *sline, int n, const char *line, int len)\n> +static int match_string_spaces(const char *line1, int len1,\n> +\t\t\t       const char *line2, int len2,\n> +\t\t\t       long flags)\n> +{\n> +\tif (flags & XDF_WHITESPACE_FLAGS) {\n> +\t\tfor (; len1 > 0 && XDL_ISSPACE(line1[len1 - 1]); len1--);\n> +\t\tfor (; len2 > 0 && XDL_ISSPACE(line2[len2 - 1]); len2--);\n> +\t}\n\nThis says \"any XDF_WHITESPACE_FLAGS causes the trailing blanks\nignored\", which is correct: --ignore-space-at-eol, -b or -w are all\nwe have.  OK.\n\n> +\tif (!(flags & (XDF_IGNORE_WHITESPACE | XDF_IGNORE_WHITESPACE_CHANGE)))\n> +\t\treturn (len1 == len2 && !memcmp(line1, line2, len1));\n\nIf --ignore-space-at-eol is the only one given, or none of the\nwhitespace flags is given, then we can just do memcmp().  OK.\n\nThe remainder of the function is only used when either -b or -w is\n(or both are) in effect.\n\n> +\twhile (len1 > 0 && len2 > 0) {\n> +\t\tlen1--;\n> +\t\tlen2--;\n\nScanning from the tail end of the strings...\n\n> +\t\tif (XDL_ISSPACE(line1[len1]) || XDL_ISSPACE(line2[len2])) {\n> +\t\t\tif ((flags & XDF_IGNORE_WHITESPACE_CHANGE) &&\n> +\t\t\t    (!XDL_ISSPACE(line1[len1]) || !XDL_ISSPACE(line2[len2])))\n> +\t\t\t\treturn 0;\n\nIf --ignore-space-change and one side is blank while the other not,\nthen these cannot be the same.  If we see blank on both sides, that\nis fine under --ignore-whitespace, too.\n\n> +\t\t\tfor (; len1 > 0 && XDL_ISSPACE(line1[len1]); len1--);\n> +\t\t\tfor (; len2 > 0 && XDL_ISSPACE(line2[len2]); len2--);\n\nThen, strip the blanks, if any, so that the next comparison is done\nfor the last non-blank char from both sides.  OK.\n\n> +\t\t}\n> +\t\tif (line1[len1] != line2[len2])\n> +\t\t\treturn 0;\n\nAnd we can say \"not same\" as soon as we find differences.  Rinse\nand repeat.\n\n> +\t}\n> +\n> +\tif (flags & XDF_IGNORE_WHITESPACE) {\n> +\t\t// Consume remaining spaces\n\nNo C++/C99 comments in our codebase, please.\n\n> +\t\tfor (; len1 > 0 && XDL_ISSPACE(line1[len1 - 1]); len1--);\n> +\t\tfor (; len2 > 0 && XDL_ISSPACE(line2[len2 - 1]); len2--);\n\nIf we are ignoring all whitespaces, we can simply discard them.  OK.\n\nI wonder what happens when --ignore-space-change is in effect but\nnot --ignore-all-space?  The loop must have exited because one (or\nboth) of len1 and len2 went down to zero.  If both are zero, we\nobviously have a match, and otherwise, one has blanks at the\nbeginning and the other does not.  Wait, is that really true?  Yes,\nbecause if len1 == 0 && len2 != 0, line2[len2] cannot be blank\n(otherwise len2 would be zero because we have a loop to eat a run of\nblanks).\n\nOK, sounds good.\n\n> +\t}\n> +\n> +\t// We matched full line1 and line2\n\nLikewise for \"//\".\n\n> +\tif (!len1 && !len2)\n> +\t\treturn 1;\n> +\n> +\treturn 0;\n> +}\n\n\n> +static void append_lost(struct sline *sline, int n, const char *line, int len, long flags)\n>  {\n>  \tstruct lline *lline;\n>  \tunsigned long this_mask = (1UL<<n);\n> @@ -133,8 +174,8 @@ static void append_lost(struct sline *sline, int n, const char *line, int len)\n>  \tif (sline->lost_head) {\n>  \t\tlline = sline->next_lost;\n>  \t\twhile (lline) {\n> -\t\t\tif (lline->len == len &&\n> -\t\t\t    !memcmp(lline->line, line, len)) {\n> +\t\t\tif (match_string_spaces(lline->line, lline->len,\n> +\t\t\t\t\t\tline, len, flags)) {\n>  \t\t\t\tlline->parent_map |= this_mask;\n>  \t\t\t\tsline->next_lost = lline->next;\n>  \t\t\t\treturn;\n\nNicely done.\n\nThanks, will queue with comment style fix-ups.\n\n> diff --git a/t/t4038-diff-combined.sh b/t/t4038-diff-combined.sh\n> index 614425a..ba8a56b 100755\n> --- a/t/t4038-diff-combined.sh\n> +++ b/t/t4038-diff-combined.sh\n> @@ -113,4 +113,79 @@ test_expect_success 'check --cc --raw with forty trees' '\n>  \tgrep \"^::::::::::::::::::::::::::::::::::::::::[^:]\" out\n>  '\n>\n> +test_expect_success 'setup combined ignore spaces' '\n> +\tgit checkout master &&\n> +\t>test &&\n> +\tgit add test &&\n> +\tgit commit -m initial &&\n> +\n> +\techo \"\n> +\talways coalesce\n> +\teol space coalesce \\n\\\n> +\tspace  change coalesce\n> +\tall spa ces coalesce\n> +\teol spaces \\n\\\n> +\tspace  change\n> +\tall spa ces\" >test &&\n> +\tgit commit -m \"change three\" -a &&\n> +\n> +\tgit checkout -b side HEAD^ &&\n> +\techo \"\n> +\talways coalesce\n> +\teol space coalesce\n> +\tspace change coalesce\n> +\tall spaces coalesce\n> +\teol spaces\n> +\tspace change\n> +\tall spaces\" >test &&\n> +\tgit commit -m indent -a &&\n> +\n> +\ttest_must_fail git merge master &&\n> +\techo \"\n> +\teol spaces \\n\\\n> +\tspace  change\n> +\tall spa ces\" > test &&\n> +\tgit commit -m merged -a\n> +'\n> +\n> +test_expect_success 'check combined output (no ignore space)' '\n> +\tgit show | test_i18ngrep \"^-\\s*eol spaces\" &&\n> +\tgit show | test_i18ngrep \"^-\\s*eol space coalesce\" &&\n> +\tgit show | test_i18ngrep \"^-\\s*space change\" &&\n> +\tgit show | test_i18ngrep \"^-\\s*space change coalesce\" &&\n> +\tgit show | test_i18ngrep \"^-\\s*all spaces\" &&\n> +\tgit show | test_i18ngrep \"^-\\s*all spaces coalesce\" &&\n> +\tgit show | test_i18ngrep \"^--\\s*always coalesce\"\n> +'\n> +\n> +test_expect_success 'check combined output (ignore space at eol)' '\n> +\tgit show --ignore-space-at-eol | test_i18ngrep \"^\\s*eol spaces\" &&\n> +\tgit show --ignore-space-at-eol | test_i18ngrep \"^--\\s*eol space coalesce\" &&\n> +\tgit show --ignore-space-at-eol | test_i18ngrep \"^-\\s*space change\" &&\n> +\tgit show --ignore-space-at-eol | test_i18ngrep \"^-\\s*space change coalesce\" &&\n> +\tgit show --ignore-space-at-eol | test_i18ngrep \"^-\\s*all spaces\" &&\n> +\tgit show --ignore-space-at-eol | test_i18ngrep \"^-\\s*all spaces coalesce\" &&\n> +\tgit show --ignore-space-at-eol | test_i18ngrep \"^--\\s*always coalesce\"\n> +'\n> +\n> +test_expect_success 'check combined output (ignore space change)' '\n> +\tgit show -b | test_i18ngrep \"^\\s*eol spaces\" &&\n> +\tgit show -b | test_i18ngrep \"^--\\s*eol space coalesce\" &&\n> +\tgit show -b | test_i18ngrep \"^\\s*space  change\" &&\n> +\tgit show -b | test_i18ngrep \"^--\\s*space change coalesce\" &&\n> +\tgit show -b | test_i18ngrep \"^-\\s*all spaces\" &&\n> +\tgit show -b | test_i18ngrep \"^-\\s*all spaces coalesce\" &&\n> +\tgit show -b | test_i18ngrep \"^--\\s*always coalesce\"\n> +'\n> +\n> +test_expect_success 'check combined output (ignore all spaces)' '\n> +\tgit show -w | test_i18ngrep \"^\\s*eol spaces\" &&\n> +\tgit show -w | test_i18ngrep \"^--\\s*eol space coalesce\" &&\n> +\tgit show -w | test_i18ngrep \"^\\s*space  change\" &&\n> +\tgit show -w | test_i18ngrep \"^--\\s*space change coalesce\" &&\n> +\tgit show -w | test_i18ngrep \"^\\s*all spa ces\" &&\n> +\tgit show -w | test_i18ngrep \"^--\\s*all spaces coalesce\" &&\n> +\tgit show -w | test_i18ngrep \"^--\\s*always coalesce\"\n> +'\n> +\n>  test_done\n> --\n> 1.7.9.5\n"},{"id":"211265","messageId":"7vzjy6zwjg.fsf@alter.siamese.dyndns.org","threadId":"33056","inReplyTo":"1363209683-10264-1-git-send-email-apelisse@gmail.com","subject":"Re: [PATCH] Allow combined diff to ignore white-spaces","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2013-03-13T23:53:39Z","receivedAt":"2013-03-13T23:53:39Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Antoine Pelisse <apelisse@gmail.com> writes:\n\n> OK, I added some tests and coalesce similar lost lines (using the same flags\n> we used for diff.\n\nHmph, why doesn't this pass its own tests?\n\n> +test_expect_success 'check combined output (no ignore space)' '\n> +\tgit show | test_i18ngrep \"^-\\s*eol spaces\" &&\n\nWhat kind of grep pattern is \"\\s*\"?\n"},{"id":"211279","messageId":"51417773.5000401@viscovery.net","threadId":"33056","inReplyTo":"1363209683-10264-1-git-send-email-apelisse@gmail.com","subject":"Re: [PATCH] Allow combined diff to ignore white-spaces","fromName":"Johannes Sixt","fromEmail":"j.sixt@viscovery.net","sentAt":"2013-03-14T07:08:35Z","receivedAt":"2013-03-14T07:08:35Z","isPatch":true,"sender":{"key":"j6t@kdbg.org","avatar":"https://avatars.githubusercontent.com/u/14810926?v=4"},"body":"Am 3/13/2013 22:21, schrieb Antoine Pelisse:\n> Currently, it's not possible to use the space-ignoring options (-b, -w,\n> --ignore-space-at-eol) with combined diff. It makes it pretty impossible\n> to read a merge between a branch that changed all tabs to spaces, and a\n> branch with functional changes.\n> \n> Pass diff flags to diff engine, so that combined diff behaves as normal\n> diff does with spaces.\n> Also coalesce lines that are removed from both (or more) parents.\n> \n> It also means that a conflict-less merge done using a ignore-* strategy\n> option will not show any conflict if shown in combined-diff using the\n> same option.\n> \n> Signed-off-by: Antoine Pelisse <apelisse@gmail.com>\n> ---\n> OK, I added some tests and coalesce similar lost lines (using the same flags\n> we used for diff.\n> \n>  combine-diff.c           |   57 ++++++++++++++++++++++++++++++-----\n>  t/t4038-diff-combined.sh |   75 ++++++++++++++++++++++++++++++++++++++++++++++\n>  2 files changed, 125 insertions(+), 7 deletions(-)\n> \n> diff --git a/combine-diff.c b/combine-diff.c\n> index 35d41cd..0f33983 100644\n> --- a/combine-diff.c\n> +++ b/combine-diff.c\n> @@ -5,6 +5,7 @@\n>  #include \"diffcore.h\"\n>  #include \"quote.h\"\n>  #include \"xdiff-interface.h\"\n> +#include \"xdiff/xmacros.h\"\n>  #include \"log-tree.h\"\n>  #include \"refs.h\"\n>  #include \"userdiff.h\"\n> @@ -122,7 +123,47 @@ static char *grab_blob(const unsigned char *sha1, unsigned int mode,\n>  \treturn blob;\n>  }\n> \n> -static void append_lost(struct sline *sline, int n, const char *line, int len)\n> +static int match_string_spaces(const char *line1, int len1,\n> +\t\t\t       const char *line2, int len2,\n> +\t\t\t       long flags)\n> +{\n> +\tif (flags & XDF_WHITESPACE_FLAGS) {\n> +\t\tfor (; len1 > 0 && XDL_ISSPACE(line1[len1 - 1]); len1--);\n> +\t\tfor (; len2 > 0 && XDL_ISSPACE(line2[len2 - 1]); len2--);\n> +\t}\n> +\n> +\tif (!(flags & (XDF_IGNORE_WHITESPACE | XDF_IGNORE_WHITESPACE_CHANGE)))\n> +\t\treturn (len1 == len2 && !memcmp(line1, line2, len1));\n> +\n> +\twhile (len1 > 0 && len2 > 0) {\n> +\t\tlen1--;\n> +\t\tlen2--;\n> +\t\tif (XDL_ISSPACE(line1[len1]) || XDL_ISSPACE(line2[len2])) {\n> +\t\t\tif ((flags & XDF_IGNORE_WHITESPACE_CHANGE) &&\n> +\t\t\t    (!XDL_ISSPACE(line1[len1]) || !XDL_ISSPACE(line2[len2])))\n> +\t\t\t\treturn 0;\n> +\n> +\t\t\tfor (; len1 > 0 && XDL_ISSPACE(line1[len1]); len1--);\n> +\t\t\tfor (; len2 > 0 && XDL_ISSPACE(line2[len2]); len2--);\n> +\t\t}\n> +\t\tif (line1[len1] != line2[len2])\n> +\t\t\treturn 0;\n> +\t}\n> +\n> +\tif (flags & XDF_IGNORE_WHITESPACE) {\n> +\t\t// Consume remaining spaces\n> +\t\tfor (; len1 > 0 && XDL_ISSPACE(line1[len1 - 1]); len1--);\n> +\t\tfor (; len2 > 0 && XDL_ISSPACE(line2[len2 - 1]); len2--);\n> +\t}\n> +\n> +\t// We matched full line1 and line2\n> +\tif (!len1 && !len2)\n> +\t\treturn 1;\n> +\n> +\treturn 0;\n> +}\n> +\n> +static void append_lost(struct sline *sline, int n, const char *line, int len, long flags)\n>  {\n>  \tstruct lline *lline;\n>  \tunsigned long this_mask = (1UL<<n);\n> @@ -133,8 +174,8 @@ static void append_lost(struct sline *sline, int n, const char *line, int len)\n>  \tif (sline->lost_head) {\n>  \t\tlline = sline->next_lost;\n>  \t\twhile (lline) {\n> -\t\t\tif (lline->len == len &&\n> -\t\t\t    !memcmp(lline->line, line, len)) {\n> +\t\t\tif (match_string_spaces(lline->line, lline->len,\n> +\t\t\t\t\t\tline, len, flags)) {\n>  \t\t\t\tlline->parent_map |= this_mask;\n>  \t\t\t\tsline->next_lost = lline->next;\n>  \t\t\t\treturn;\n> @@ -162,6 +203,7 @@ struct combine_diff_state {\n>  \tint n;\n>  \tstruct sline *sline;\n>  \tstruct sline *lost_bucket;\n> +\tlong flags;\n>  };\n> \n>  static void consume_line(void *state_, char *line, unsigned long len)\n> @@ -201,7 +243,7 @@ static void consume_line(void *state_, char *line, unsigned long len)\n>  \t\treturn; /* not in any hunk yet */\n>  \tswitch (line[0]) {\n>  \tcase '-':\n> -\t\tappend_lost(state->lost_bucket, state->n, line+1, len-1);\n> +\t\tappend_lost(state->lost_bucket, state->n, line+1, len-1, state->flags);\n>  \t\tbreak;\n>  \tcase '+':\n>  \t\tstate->sline[state->lno-1].flag |= state->nmask;\n> @@ -215,7 +257,7 @@ static void combine_diff(const unsigned char *parent, unsigned int mode,\n>  \t\t\t struct sline *sline, unsigned int cnt, int n,\n>  \t\t\t int num_parent, int result_deleted,\n>  \t\t\t struct userdiff_driver *textconv,\n> -\t\t\t const char *path)\n> +\t\t\t const char *path, long flags)\n>  {\n>  \tunsigned int p_lno, lno;\n>  \tunsigned long nmask = (1UL << n);\n> @@ -231,9 +273,10 @@ static void combine_diff(const unsigned char *parent, unsigned int mode,\n>  \tparent_file.ptr = grab_blob(parent, mode, &sz, textconv, path);\n>  \tparent_file.size = sz;\n>  \tmemset(&xpp, 0, sizeof(xpp));\n> -\txpp.flags = 0;\n> +\txpp.flags = flags;\n>  \tmemset(&xecfg, 0, sizeof(xecfg));\n>  \tmemset(&state, 0, sizeof(state));\n> +\tstate.flags = flags;\n>  \tstate.nmask = nmask;\n>  \tstate.sline = sline;\n>  \tstate.lno = 1;\n> @@ -962,7 +1005,7 @@ static void show_patch_diff(struct combine_diff_path *elem, int num_parent,\n>  \t\t\t\t     elem->parent[i].mode,\n>  \t\t\t\t     &result_file, sline,\n>  \t\t\t\t     cnt, i, num_parent, result_deleted,\n> -\t\t\t\t     textconv, elem->path);\n> +\t\t\t\t     textconv, elem->path, opt->xdl_opts);\n>  \t}\n> \n>  \tshow_hunks = make_hunks(sline, cnt, num_parent, dense);\n> diff --git a/t/t4038-diff-combined.sh b/t/t4038-diff-combined.sh\n> index 614425a..ba8a56b 100755\n> --- a/t/t4038-diff-combined.sh\n> +++ b/t/t4038-diff-combined.sh\n> @@ -113,4 +113,79 @@ test_expect_success 'check --cc --raw with forty trees' '\n>  \tgrep \"^::::::::::::::::::::::::::::::::::::::::[^:]\" out\n>  '\n> \n> +test_expect_success 'setup combined ignore spaces' '\n> +\tgit checkout master &&\n> +\t>test &&\n> +\tgit add test &&\n> +\tgit commit -m initial &&\n> +\n> +\techo \"\n> +\talways coalesce\n> +\teol space coalesce \\n\\\n> +\tspace  change coalesce\n> +\tall spa ces coalesce\n> +\teol spaces \\n\\\n> +\tspace  change\n> +\tall spa ces\" >test &&\n\nThis form of 'echo' is not sufficiently portable. How about:\n\n\ttr -d Q <<-\\EOF >test &&\n\n\talways coalesce\n\teol space coalesce Q\n\tspace  change coalesce\n\tall spa ces coalesce\n\teol spaces Q\n\tspace  change\n\tall spa ces\n\tEOF\n\n> +\tgit commit -m \"change three\" -a &&\n> +\n> +\tgit checkout -b side HEAD^ &&\n> +\techo \"\n> +\talways coalesce\n> +\teol space coalesce\n> +\tspace change coalesce\n> +\tall spaces coalesce\n> +\teol spaces\n> +\tspace change\n> +\tall spaces\" >test &&\n> +\tgit commit -m indent -a &&\n> +\n> +\ttest_must_fail git merge master &&\n> +\techo \"\n> +\teol spaces \\n\\\n> +\tspace  change\n> +\tall spa ces\" > test &&\n\nDitto.\n\n> +\tgit commit -m merged -a\n> +'\n> +\n> +test_expect_success 'check combined output (no ignore space)' '\n> +\tgit show | test_i18ngrep \"^-\\s*eol spaces\" &&\n> +\tgit show | test_i18ngrep \"^-\\s*eol space coalesce\" &&\n> +\tgit show | test_i18ngrep \"^-\\s*space change\" &&\n> +\tgit show | test_i18ngrep \"^-\\s*space change coalesce\" &&\n> +\tgit show | test_i18ngrep \"^-\\s*all spaces\" &&\n> +\tgit show | test_i18ngrep \"^-\\s*all spaces coalesce\" &&\n> +\tgit show | test_i18ngrep \"^--\\s*always coalesce\"\n\nThis loses the exit code of git show. We usually write this as\n\n\tgit show >actual &&\n\tgrep \"^- *eol spaces\" &&\n\tgrep \"^- *eol space coalesce\" &&\n\t...\n\n(Same for later tests.)\n\nThere is nothing i18n-ish in the test patterns. Use regular grep.\n\nBTW, there is compare_diff_patch() in diff-lib.sh. You can use it to\ncompare diff output to expected output. Then you do not need a grep\ninvocation for each line of the test file.\n\n-- Hannes\n"},{"id":"211304","messageId":"7v7glayp4l.fsf@alter.siamese.dyndns.org","threadId":"33056","inReplyTo":"51417773.5000401@viscovery.net","subject":"Re: [PATCH] Allow combined diff to ignore white-spaces","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2013-03-14T15:31:22Z","receivedAt":"2013-03-14T15:31:22Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Johannes Sixt <j.sixt@viscovery.net> writes:\n\n> This form of 'echo' is not sufficiently portable. How about:\n>\n> \ttr -d Q <<-\\EOF >test &&\n>\n> \talways coalesce\n> \teol space coalesce Q\n> ...\n> \tEOF\n\nMuch better.\n\n>> +test_expect_success 'check combined output (no ignore space)' '\n>> +\tgit show | test_i18ngrep \"^-\\s*eol spaces\" &&\n>> ...\n>> +\tgit show | test_i18ngrep \"^--\\s*always coalesce\"\n>\n> This loses the exit code of git show. We usually write this as\n>\n> \tgit show >actual &&\n> \tgrep \"^- *eol spaces\" &&\n> \tgrep \"^- *eol space coalesce\" &&\n> \t...\n>\n> (Same for later tests.)\n>\n> There is nothing i18n-ish in the test patterns. Use regular grep.\n>\n> BTW, there is compare_diff_patch() in diff-lib.sh. You can use it to\n> compare diff output to expected output. Then you do not need a grep\n> invocation for each line of the test file.\n\nAll good suggestions.  Thanks.\n"},{"id":"211307","messageId":"CALWbr2yU6Nwg3_7CD6XhvruaKrhvXbRS0bO6nO+GgavjO59+jQ@mail.gmail.com","threadId":"33056","inReplyTo":"7v7glayp4l.fsf@alter.siamese.dyndns.org","subject":"Re: [PATCH] Allow combined diff to ignore white-spaces","fromName":"Antoine Pelisse","fromEmail":"apelisse@gmail.com","sentAt":"2013-03-14T15:51:10Z","receivedAt":"2013-03-14T15:51:10Z","isPatch":true,"sender":{"key":"apelisse@gmail.com","avatar":"https://avatars.githubusercontent.com/u/1929644?v=4"},"body":"On Thu, Mar 14, 2013 at 4:31 PM, Junio C Hamano <gitster@pobox.com> wrote:\n> Johannes Sixt <j.sixt@viscovery.net> writes:\n>\n>> This form of 'echo' is not sufficiently portable. How about:\n>>\n>>       tr -d Q <<-\\EOF >test &&\n>>\n>>       always coalesce\n>>       eol space coalesce Q\n>> ...\n>>       EOF\n>\n> Much better.\n>\n>>> +test_expect_success 'check combined output (no ignore space)' '\n>>> +    git show | test_i18ngrep \"^-\\s*eol spaces\" &&\n>>> ...\n>>> +    git show | test_i18ngrep \"^--\\s*always coalesce\"\n>>\n>> This loses the exit code of git show. We usually write this as\n>>\n>>       git show >actual &&\n>>       grep \"^- *eol spaces\" &&\n>>       grep \"^- *eol space coalesce\" &&\n>>       ...\n>>\n>> (Same for later tests.)\n>>\n>> There is nothing i18n-ish in the test patterns. Use regular grep.\n>>\n>> BTW, there is compare_diff_patch() in diff-lib.sh. You can use it to\n>> compare diff output to expected output. Then you do not need a grep\n>> invocation for each line of the test file.\n>\n> All good suggestions.  Thanks.\n\nOK Very good,\nI will resubmit tonight. I was indeed not really sure about the best\nway to test here. Thanks for the tips and confirmation !\n\nCheers,\n"},{"id":"211325","messageId":"1363294994-3127-1-git-send-email-apelisse@gmail.com","threadId":"33056","inReplyTo":"7v7glayp4l.fsf@alter.siamese.dyndns.org","subject":"[PATCH v3] Allow combined diff to ignore white-spaces","fromName":"Antoine Pelisse","fromEmail":"apelisse@gmail.com","sentAt":"2013-03-14T21:03:14Z","receivedAt":"2013-03-14T21:03:14Z","isPatch":true,"sender":{"key":"apelisse@gmail.com","avatar":"https://avatars.githubusercontent.com/u/1929644?v=4"},"body":"Currently, it's not possible to use the space-ignoring options (-b, -w,\n--ignore-space-at-eol) with combined diff. It makes it pretty impossible\nto read a merge between a branch that changed all tabs to spaces, and a\nbranch with functional changes.\n\nPass diff flags to diff engine, so that combined diff behaves as normal\ndiff does with spaces.\nAlso coalesce lines that are removed from both (or more) parents.\n\nIt also means that a conflict-less merge done using a ignore-* strategy\noption will not show any conflict if shown in combined-diff using the\nsame option.\n\nSigned-off-by: Antoine Pelisse <apelisse@gmail.com>\n---\nChanges:\n- Fixed comments\n- Fixed tests (following Johannes suggestions)\n\n combine-diff.c           |   57 +++++++++++++++++++++---\n t/t4038-diff-combined.sh |  111 ++++++++++++++++++++++++++++++++++++++++++++++\n 2 files changed, 161 insertions(+), 7 deletions(-)\n\ndiff --git a/combine-diff.c b/combine-diff.c\nindex 35d41cd..6288485 100644\n--- a/combine-diff.c\n+++ b/combine-diff.c\n@@ -5,6 +5,7 @@\n #include \"diffcore.h\"\n #include \"quote.h\"\n #include \"xdiff-interface.h\"\n+#include \"xdiff/xmacros.h\"\n #include \"log-tree.h\"\n #include \"refs.h\"\n #include \"userdiff.h\"\n@@ -122,7 +123,47 @@ static char *grab_blob(const unsigned char *sha1, unsigned int mode,\n \treturn blob;\n }\n\n-static void append_lost(struct sline *sline, int n, const char *line, int len)\n+static int match_string_spaces(const char *line1, int len1,\n+\t\t\t       const char *line2, int len2,\n+\t\t\t       long flags)\n+{\n+\tif (flags & XDF_WHITESPACE_FLAGS) {\n+\t\tfor (; len1 > 0 && XDL_ISSPACE(line1[len1 - 1]); len1--);\n+\t\tfor (; len2 > 0 && XDL_ISSPACE(line2[len2 - 1]); len2--);\n+\t}\n+\n+\tif (!(flags & (XDF_IGNORE_WHITESPACE | XDF_IGNORE_WHITESPACE_CHANGE)))\n+\t\treturn (len1 == len2 && !memcmp(line1, line2, len1));\n+\n+\twhile (len1 > 0 && len2 > 0) {\n+\t\tlen1--;\n+\t\tlen2--;\n+\t\tif (XDL_ISSPACE(line1[len1]) || XDL_ISSPACE(line2[len2])) {\n+\t\t\tif ((flags & XDF_IGNORE_WHITESPACE_CHANGE) &&\n+\t\t\t    (!XDL_ISSPACE(line1[len1]) || !XDL_ISSPACE(line2[len2])))\n+\t\t\t\treturn 0;\n+\n+\t\t\tfor (; len1 > 0 && XDL_ISSPACE(line1[len1]); len1--);\n+\t\t\tfor (; len2 > 0 && XDL_ISSPACE(line2[len2]); len2--);\n+\t\t}\n+\t\tif (line1[len1] != line2[len2])\n+\t\t\treturn 0;\n+\t}\n+\n+\tif (flags & XDF_IGNORE_WHITESPACE) {\n+\t\t/* Consume remaining spaces */\n+\t\tfor (; len1 > 0 && XDL_ISSPACE(line1[len1 - 1]); len1--);\n+\t\tfor (; len2 > 0 && XDL_ISSPACE(line2[len2 - 1]); len2--);\n+\t}\n+\n+\t/* We matched full line1 and line2 */\n+\tif (!len1 && !len2)\n+\t\treturn 1;\n+\n+\treturn 0;\n+}\n+\n+static void append_lost(struct sline *sline, int n, const char *line, int len, long flags)\n {\n \tstruct lline *lline;\n \tunsigned long this_mask = (1UL<<n);\n@@ -133,8 +174,8 @@ static void append_lost(struct sline *sline, int n, const char *line, int len)\n \tif (sline->lost_head) {\n \t\tlline = sline->next_lost;\n \t\twhile (lline) {\n-\t\t\tif (lline->len == len &&\n-\t\t\t    !memcmp(lline->line, line, len)) {\n+\t\t\tif (match_string_spaces(lline->line, lline->len,\n+\t\t\t\t\t\tline, len, flags)) {\n \t\t\t\tlline->parent_map |= this_mask;\n \t\t\t\tsline->next_lost = lline->next;\n \t\t\t\treturn;\n@@ -162,6 +203,7 @@ struct combine_diff_state {\n \tint n;\n \tstruct sline *sline;\n \tstruct sline *lost_bucket;\n+\tlong flags;\n };\n\n static void consume_line(void *state_, char *line, unsigned long len)\n@@ -201,7 +243,7 @@ static void consume_line(void *state_, char *line, unsigned long len)\n \t\treturn; /* not in any hunk yet */\n \tswitch (line[0]) {\n \tcase '-':\n-\t\tappend_lost(state->lost_bucket, state->n, line+1, len-1);\n+\t\tappend_lost(state->lost_bucket, state->n, line+1, len-1, state->flags);\n \t\tbreak;\n \tcase '+':\n \t\tstate->sline[state->lno-1].flag |= state->nmask;\n@@ -215,7 +257,7 @@ static void combine_diff(const unsigned char *parent, unsigned int mode,\n \t\t\t struct sline *sline, unsigned int cnt, int n,\n \t\t\t int num_parent, int result_deleted,\n \t\t\t struct userdiff_driver *textconv,\n-\t\t\t const char *path)\n+\t\t\t const char *path, long flags)\n {\n \tunsigned int p_lno, lno;\n \tunsigned long nmask = (1UL << n);\n@@ -231,9 +273,10 @@ static void combine_diff(const unsigned char *parent, unsigned int mode,\n \tparent_file.ptr = grab_blob(parent, mode, &sz, textconv, path);\n \tparent_file.size = sz;\n \tmemset(&xpp, 0, sizeof(xpp));\n-\txpp.flags = 0;\n+\txpp.flags = flags;\n \tmemset(&xecfg, 0, sizeof(xecfg));\n \tmemset(&state, 0, sizeof(state));\n+\tstate.flags = flags;\n \tstate.nmask = nmask;\n \tstate.sline = sline;\n \tstate.lno = 1;\n@@ -962,7 +1005,7 @@ static void show_patch_diff(struct combine_diff_path *elem, int num_parent,\n \t\t\t\t     elem->parent[i].mode,\n \t\t\t\t     &result_file, sline,\n \t\t\t\t     cnt, i, num_parent, result_deleted,\n-\t\t\t\t     textconv, elem->path);\n+\t\t\t\t     textconv, elem->path, opt->xdl_opts);\n \t}\n\n \tshow_hunks = make_hunks(sline, cnt, num_parent, dense);\ndiff --git a/t/t4038-diff-combined.sh b/t/t4038-diff-combined.sh\nindex 614425a..b7e16a7 100755\n--- a/t/t4038-diff-combined.sh\n+++ b/t/t4038-diff-combined.sh\n@@ -3,6 +3,7 @@\n test_description='combined diff'\n\n . ./test-lib.sh\n+. \"$TEST_DIRECTORY\"/diff-lib.sh\n\n setup_helper () {\n \tone=$1 branch=$2 side=$3 &&\n@@ -113,4 +114,114 @@ test_expect_success 'check --cc --raw with forty trees' '\n \tgrep \"^::::::::::::::::::::::::::::::::::::::::[^:]\" out\n '\n\n+test_expect_success 'setup combined ignore spaces' '\n+\tgit checkout master &&\n+\t>test &&\n+\tgit add test &&\n+\tgit commit -m initial &&\n+\n+\ttr -d Q <<-\\EOF >test &&\n+\talways coalesce\n+\teol space coalesce Q\n+\tspace  change coalesce\n+\tall spa ces coalesce\n+\teol spaces Q\n+\tspace  change\n+\tall spa ces\n+\tEOF\n+\tgit commit -m \"test space change\" -a &&\n+\n+\tgit checkout -b side HEAD^ &&\n+\ttr -d Q <<-\\EOF >test &&\n+\talways coalesce\n+\teol space coalesce\n+\tspace change coalesce\n+\tall spaces coalesce\n+\teol spaces\n+\tspace change\n+\tall spaces\n+\tEOF\n+\tgit commit -m \"test other space changes\" -a &&\n+\n+\ttest_must_fail git merge master &&\n+\ttr -d Q <<-\\EOF >test &&\n+\teol spaces Q\n+\tspace  change\n+\tall spa ces\n+\tEOF\n+\tgit commit -m merged -a\n+'\n+\n+test_expect_success 'check combined output (no ignore space)' '\n+\tgit show >actual.tmp &&\n+\tsed -e \"1,/^@@@/d\" < actual.tmp >actual &&\n+\ttr -d Q <<-\\EOF >expected &&\n+\t--always coalesce\n+\t- eol space coalesce\n+\t- space change coalesce\n+\t- all spaces coalesce\n+\t- eol spaces\n+\t- space change\n+\t- all spaces\n+\t -eol space coalesce Q\n+\t -space  change coalesce\n+\t -all spa ces coalesce\n+\t+ eol spaces Q\n+\t+ space  change\n+\t+ all spa ces\n+\tEOF\n+\tcompare_diff_patch expected actual\n+'\n+\n+test_expect_success 'check combined output (ignore space at eol)' '\n+\tgit show --ignore-space-at-eol >actual.tmp &&\n+\tsed -e \"1,/^@@@/d\" < actual.tmp >actual &&\n+\ttr -d Q <<-\\EOF >expected &&\n+\t--always coalesce\n+\t--eol space coalesce\n+\t- space change coalesce\n+\t- all spaces coalesce\n+\t -space  change coalesce\n+\t -all spa ces coalesce\n+\t  eol spaces Q\n+\t- space change\n+\t- all spaces\n+\t+ space  change\n+\t+ all spa ces\n+\tEOF\n+\tcompare_diff_patch expected actual\n+'\n+\n+test_expect_success 'check combined output (ignore space change)' '\n+\tgit show -b >actual.tmp &&\n+\tsed -e \"1,/^@@@/d\" < actual.tmp >actual &&\n+\ttr -d Q <<-\\EOF >expected &&\n+\t--always coalesce\n+\t--eol space coalesce\n+\t--space change coalesce\n+\t- all spaces coalesce\n+\t -all spa ces coalesce\n+\t  eol spaces Q\n+\t  space  change\n+\t- all spaces\n+\t+ all spa ces\n+\tEOF\n+\tcompare_diff_patch expected actual\n+'\n+\n+test_expect_success 'check combined output (ignore all spaces)' '\n+\tgit show -w >actual.tmp &&\n+\tsed -e \"1,/^@@@/d\" < actual.tmp >actual &&\n+\ttr -d Q <<-\\EOF >expected &&\n+\t--always coalesce\n+\t--eol space coalesce\n+\t--space change coalesce\n+\t--all spaces coalesce\n+\t  eol spaces Q\n+\t  space  change\n+\t  all spa ces\n+\tEOF\n+\tcompare_diff_patch expected actual\n+'\n+\n test_done\n--\n1.7.9.5\n"},{"id":"211328","messageId":"7v1ubhy7wo.fsf@alter.siamese.dyndns.org","threadId":"33056","inReplyTo":"1363294994-3127-1-git-send-email-apelisse@gmail.com","subject":"Re: [PATCH v3] Allow combined diff to ignore white-spaces","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2013-03-14T21:43:19Z","receivedAt":"2013-03-14T21:43:19Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Antoine Pelisse <apelisse@gmail.com> writes:\n\n> +test_expect_success 'check combined output (no ignore space)' '\n> +\tgit show >actual.tmp &&\n> +\tsed -e \"1,/^@@@/d\" < actual.tmp >actual &&\n> +\ttr -d Q <<-\\EOF >expected &&\n> +\t--always coalesce\n> +\t- eol space coalesce\n> +\t- space change coalesce\n> +\t- all spaces coalesce\n> +\t- eol spaces\n> +\t- space change\n> +\t- all spaces\n> +\t -eol space coalesce Q\n> +\t -space  change coalesce\n> +\t -all spa ces coalesce\n> +\t+ eol spaces Q\n> +\t+ space  change\n> +\t+ all spa ces\n> +\tEOF\n> +\tcompare_diff_patch expected actual\n> +'\n\nNicely constructed ;-)\n\nThanks.\n"}]}