>> 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