search

langram / node-diff3

This is a node.js library for text diffing and three-way-merge. It originally come from project Synchrotron, created by Tony Garnock-Jones. For more detail please visit: http://homepages.kcbbs.gen.nz/tonyg/projects/synchrotron.html.
MIT License
12 stars 5 forks source link

Avoid unintended use of toString-based sort; closes #3. #4

Open tonyg opened 8 years ago

tonyg commented 8 years ago

Fix taken from https://github.com/tonyg/synchrotron/commit/7eb715d08d3c7b479b30b9f413a4a61db9940e82.

alex-e-leon commented 7 years ago

@langram Any updates on this? This fixes a pretty serious issue with the diff algorithm, and would be nice to see it in the lib.

Dmole commented 7 years ago

With this patch A,b,c a,b,c a,B,c still conflicts due to > instead of >= https://github.com/Dmole/node-diff3/commit/61fdbee8613478e3e92a337a79256bbf6175213e

tonyg commented 7 years ago

@Dmole, the output of diff3 -m on your example gives the same results as Diff.diff3_merge in synchrotron. So I think the synchrotron code is working as intended. It's still an open question whether this particular form of diff3 algorithm is the "right thing", of course!

Dmole commented 7 years ago

@tonyg with what input? Without that extra change diffs that touch but do not overlap are falsely reported as a conflict.

What is wrong about the merge3 algorithm? It now seems to be working comparably to other diff3 implementations I use regularly, though I have not really done any thorough testing. Or are you just commenting that all merges should only be used as an assist and should really be sanity checked by a human? I took things a step further and line merged conflicts of the same size for extra convenience/danger.

tonyg commented 7 years ago

@Dmole, I mean specifically the output of diff3 -m a o b where o contains a\nb\nc, a contains A\nb\nc and b contains a\nB\nc. It reports a conflict just like the one reported from Diff.diff3_merge. So when you say "falsely reported as a conflict", I see what you mean, but am not sure that calling output that matches Unix diff3 "false" is the right thing to do here.

Regarding alternative algorithms (such as your variant!), I just mean to refer to the fact that none of diff, diff3 or merge are able to be said to be "correct" since they're not aware of the meaning of the texts they affect, and that different approaches are appropriate in different situations depending on the contents of the files being manipulated. This old wikipage might be interesting.

Dmole commented 7 years ago

@tonyg Wow I did not even notice that...I wonder why unix merge is doing that, there must be a [historical] reason...

(Yah context aware diff never really took off.)