An approach that considers the suggested edit location #2
Conversation
… edit location (cursor position)
| * E.g. | ||
| * cursor_normalize_diff([[DIFF_EQUAL, 'abc']], cursorPos) | ||
| * => [1, [[DIFF_EQUAL, 'a'], [DIFF_EQUAL, 'bc']]] | ||
| * cursor_normalize_diff([[DIFF_INSERT, 'new'], [DIFF_DELETE, 'xyz']], 1) |
There was a problem hiding this comment.
I think the cursorPos should be 2 to produce the below result?
| /* | ||
| * Modify a diff such that the cursor position points to the start of a change: | ||
| * E.g. | ||
| * cursor_normalize_diff([[DIFF_EQUAL, 'abc']], cursorPos) |
| @@ -73,6 +74,9 @@ function diff_main(text1, text2) { | |||
| diffs.push([DIFF_EQUAL, commonsuffix]); | |||
| } | |||
| diff_cleanupMerge(diffs); | |||
| if (cursor_pos != null) { | |||
There was a problem hiding this comment.
I'm not 100% sure on this but what does merge_tuples do that diff_cleanupMerge does not? Is is possible to just use diff_cleanupMerge after fix_cursor and not have to call/implement merge_tuples at all?
There was a problem hiding this comment.
diff_cleanupMerge also shifts edits (see line 541), reversing some of my modifications:
diff_cleanupMerge(fix_cursor([[0, 'aa'], [1, 'a']], 1))
= diff_cleanupMerge([[0, 'a'], [1, 'a'], [0, 'a']])
= [[0, 'aa'], [1, 'a']]
I could extend diff_cleanupMerge to accept some flag to ignore the shifting part. But then there is also the problem that it checks the whole array, and performs other modifications I don't fully understand.
There was a problem hiding this comment.
Okay that's fine--did not know diff_cleanupMerge shifted as well. Maybe it could be something to be improved in the future, but it's not a large performance penalty or anything practically significant at the moment.
This was in place because the issue of misplaced deltas regarding the edit position when adding characters to a character sequence, because of the diff strategy of the fast-diff lib detailed on this issue/PR, slab/quill#746. Since this has been fixed on fast-diff at jhchen/fast-diff#2, the loopback fix/workaround is no longer necessary.
This is a naive approach to "shift" the edit location to the suggested edit location (cursor position). It is supposed to solve all cursor location problems for simple insert / delete modifications. This is related to slab/quill#746
Assuming the cursor location is at position 0, we check the following cases:
Case 1) Switch two diff tuples if it holds the same result. E.g.
[0, "a"], [1, "a"] -> [1, "a"], [0, "a"]Case 2) Try to shift the edit location to the beginning
Does this implementation suffice?