Re: [PATCH] Allow combined diff to ignore white-spaces

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



>> That should be reviewed carefully as I'm not exactly sure that does make
>> sense with the way combined-diff works.
>> Still it seems natural to me to be able to remove the space in combined
>> diff as we do with normal diff. Especially as I unfortunately have to
>> deal with many space + feature merges that are very hard to analyze/handle
>> if space differences can't be hidden.
>
> Assuming "That" at the beginning of the off-line comment refers to
> "this patch", I actually I do not think this patch needs to be
> reviewed carefully at all.

I actually meant: "is that patch enough or is there anything I
missed". Considering your answer, it looks like I did ;)

> An example.  After collecting pairwise diffs between the shared
> postimage and individual parents, we align the hunks and coalesce
> "identical" preimage lines to form shared "-" (removed) lines.  I
> think that step is done with byte-for-byte comparison.  If the
> postimage removes a line from one parent and a corresponding line
> from another parent, and if these corresponding lines in the parents
> differ only by whitespaces in a way the user told us to ignore with
> -b/-w/etc., you would need to tweak the logic to coalesce
> corresponding "lost" lines in the append_lost() function. Otherwise,
> you will see two " -" and "- " lines that remove these corresponding
> lines from the parents that are identical when you ignore
> whitespaces.

That is the part I missed, and it's nicely explained.

> You should add some tests; it would help you think about these
> issues through.

I will definitely add some tests.

> For example, in this history:
>
>        git init
>        seq 11 >ten
>        git add ten
>        git commit -m initial
>
>        seq 10 | sed -e 's/6/6 six/' -e 's/2/2 two/' >ten
>        echo 11 >>ten
>        git commit -m ten -a
>
>        git checkout -b side HEAD^
>        seq 10 | sed -e 's/^/  /' | sed -e 's/2/2 dos/' >ten
>        echo 11 >>ten
>        git commit -m indent -a
>
>        git merge master
>
>        seq 10 | sed -e 's/5/5 go/' -e 's/2/2 dois/' >ten
>        git commit -m merged -a
>
> one side indents the original and tweaks some lines, while the other
> side tweaks some other lines without reindenting.  Most notably, the
> fifth line on one side is "5" while on the other side is "  5", and
> this line is rewritten to "5 go" in the final.  The lost lines are
> not coalesced to a single "-- 5" (nor "--   5") but shows that two
> preimages were different only by whitespaces.  Compare that with
> what happens to the final line "11" that gets lost in the result.

It feels incorrect to me to coalsesce "- 5" and "-  5" as it might
look incorrect to the user. But still the idea is appealing. I have an
implementation for that that requires more testing.

Using the exact example you gave, and running the latest next, I have
this output, where 11 is not coalesced.
Is that a bug ?

diff --cc ten
index d213a99,ed40ab2..37c2af2
--- a/ten
+++ b/ten
@@@ -1,11 -1,11 +1,10 @@@
-   1
-   2 dos
-   3
-   4
-   5
-   6
-   7
-   8
-   9
-   10
- 11
+ 1
 -2 two
++2 dois
+ 3
+ 4
 -5
 -6 six
++5 go
++6
+ 7
+ 8
+ 9
+ 10
 -11
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html


[Index of Archives]     [Linux Kernel Development]     [Gcc Help]     [IETF Annouce]     [DCCP]     [Netdev]     [Networking]     [Security]     [V4L]     [Bugtraq]     [Yosemite]     [MIPS Linux]     [ARM Linux]     [Linux Security]     [Linux RAID]     [Linux SCSI]     [Fedora Users]