Skip to content
New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # to your account

textDiff option set to "0" should allow single letters to be merged instead of taking the default value 60. #104

Open
geyang opened this issue Sep 3, 2015 · 4 comments

Comments

@geyang
Copy link

geyang commented Sep 3, 2015

If you set textDiff: {minLength: 0}, the text diff falls back to the default minLength of 60 characters. This prevents two single letter commits to merge properly.

Setting minLength to 1 allows two strings with at least 2 characters to properly diff merge. However for changes such as:

"" => "a" and "" => "b" the merge simply takes the later one. If any further updates on a is made, it is also replaced by b.

@geyang geyang changed the title textDiff option set to "0" should allow single letters to be merged textDiff option set to "0" should allow single letters to be merged instead of taking the default value. Sep 3, 2015
@geyang geyang changed the title textDiff option set to "0" should allow single letters to be merged instead of taking the default value. textDiff option set to "0" should allow single letters to be merged instead of taking the default value 60. Sep 3, 2015
@benjamine
Copy link
Owner

I see, you can't set that minimum to 0 without the default getting applied, although I'm curious in which scenario would you like to get text diffs generated for 1 char strings? that would be just way more inefficient (both in CPU cost and in the delta size) than normal diff by value, without bringing any advantage.

@geyang
Copy link
Author

geyang commented Sep 5, 2015

haha oh well, the case is below. Thanks for this great library! The CPU cost and delta size is something we worry about at a higher level. For this library we need it to be able to handle the following case:

The key is that this minLength is the minimum length both of the two strings have to match to. In a collaboration environment, proper merge rely on Fraser's fuzzy merge algorithm to work. So substituting of a text diff with string replacement breaks the fuzzy merge.

Say we start with an empty string: Within a short 200ms, Adam adds three letters "thi", and this arrived at the server. ther server replaces the "" with "thi" and Adam is happy.

then Bob also made some change and types "red". Now with Fraser's algorithm, the merged result is properly "thired". However, unless the first diff is generated properly using Fraser's algorithm, the diff wouldn't be a nice text diff and the fuzzy patch wouldn't work. Now instead, the diff is a string replacement, and it just replaces what's already there, so Adam sees his work disappear, including the stuff he typed after "this is a little".

You can test this out here: The power in Fraser's code is the weighted fuzzy patch, and it works surprisingly well. For short strings, it completely negates the need for operational transform. I'm not sure about long strings yet.

@semireg
Copy link

semireg commented Aug 27, 2018

Is there a way to fully disable this mode all together? Entering a very large number works, but isn't ideal. I'll open a new issue if requested. Values of zero, null, or -1 do not disable this mode.

@ViktorQvarfordt
Copy link

PR to out put of text diffing #314 @semireg

# for free to join this conversation on GitHub. Already have an account? # to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants