Skip to content
New issue

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

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

Already on GitHub? Sign in to your account

An approach that considers the suggested edit location #2

Merged
merged 2 commits into from
Oct 19, 2016
Merged
Show file tree
Hide file tree
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 4 additions & 0 deletions README.md
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,10 @@ var bad = 'Bad dog';
var result = diff(good, bad);
// [[-1, "Goo"], [1, "Ba"], [0, "d dog"]]

// Respect suggested edit location (cursor position)
diff('aaa', 'aaaa', 1)
// [[0, "a"], [1, "a"], [0, "aa"]]

// For convenience
diff.INSERT === 1;
diff.EQUAL === 0;
Expand Down
123 changes: 121 additions & 2 deletions diff.js
Original file line number Diff line number Diff line change
Expand Up @@ -39,9 +39,10 @@ var DIFF_EQUAL = 0;
* any common prefix or suffix off the texts before diffing.
* @param {string} text1 Old string to be diffed.
* @param {string} text2 New string to be diffed.
* @param {Int} cursor_pos Expected edit position in text1 (optional)
* @return {Array} Array of diff tuples.
*/
function diff_main(text1, text2) {
function diff_main(text1, text2, cursor_pos) {
// Check for equality (speedup).
if (text1 == text2) {
if (text1) {
Expand Down Expand Up @@ -73,6 +74,9 @@ function diff_main(text1, text2) {
diffs.push([DIFF_EQUAL, commonsuffix]);
}
diff_cleanupMerge(diffs);
if (cursor_pos != null) {
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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?

Copy link
Contributor Author

@dmonad dmonad Oct 19, 2016

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

diffs = fix_cursor(diffs, cursor_pos);
}
return diffs;
};

Expand Down Expand Up @@ -566,5 +570,120 @@ diff.INSERT = DIFF_INSERT;
diff.DELETE = DIFF_DELETE;
diff.EQUAL = DIFF_EQUAL;


module.exports = diff;

/*
* Modify a diff such that the cursor position points to the start of a change:
* E.g.
* cursor_normalize_diff([[DIFF_EQUAL, 'abc']], cursorPos)
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

cursorPos should be 1 here?

* => [1, [[DIFF_EQUAL, 'a'], [DIFF_EQUAL, 'bc']]]
* cursor_normalize_diff([[DIFF_INSERT, 'new'], [DIFF_DELETE, 'xyz']], 1)
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think the cursorPos should be 2 to produce the below result?

* => [2, [[DIFF_INSERT, 'new'], [DIFF_DELETE, 'xy'], [DIFF_DELETE, 'z']]]
*
* @param {Array} diffs Array of diff tuples
* @param {Int} cursor_pos Suggested edit position. Must not be out of bounds!
* @return {Array} A tuple [cursor location in the modified diff, modified diff]
*/
function cursor_normalize_diff (diffs, cursor_pos) {
if (cursor_pos === 0) {
return [DIFF_EQUAL, diffs];
}
for (var current_pos = 0, i = 0; i < diffs.length; i++) {
var d = diffs[i];
if (d[0] === DIFF_DELETE || d[0] === DIFF_EQUAL) {
var next_pos = current_pos + d[1].length;
if (cursor_pos === next_pos) {
return [i + 1, diffs];
} else if (cursor_pos < next_pos) {
// copy to prevent side effects
diffs = diffs.slice();
// split d into two diff changes
var split_pos = cursor_pos - current_pos;
var d_left = [d[0], d[1].slice(0, split_pos)];
var d_right = [d[0], d[1].slice(split_pos)];
diffs.splice(i, 1, d_left, d_right);
return [i + 1, diffs];
} else {
current_pos = next_pos;
}
}
}
throw new Error('cursor_pos is out of bounds!')
}

/*
* Modify a diff such that the edit position is "shifted" to the proposed edit location (cursor_position).
*
* Case 1)
* Check if a naive shift is possible:
* [0, X], [ 1, Y] -> [ 1, Y], [0, X] (if X + Y === Y + X)
* [0, X], [-1, Y] -> [-1, Y], [0, X] (if X + Y === Y + X) - holds same result
* Case 2)
* Check if the following shifts are possible:
* [0, 'pre'], [ 1, 'prefix'] -> [ 1, 'pre'], [0, 'pre'], [ 1, 'fix']
* [0, 'pre'], [-1, 'prefix'] -> [-1, 'pre'], [0, 'pre'], [-1, 'fix']
* ^ ^
* d d_next
*
* @param {Array} diffs Array of diff tuples
* @param {Int} cursor_pos Suggested edit position. Must not be out of bounds!
* @return {Array} Array of diff tuples
*/
function fix_cursor (diffs, cursor_pos) {
var norm = cursor_normalize_diff(diffs, cursor_pos);
var ndiffs = norm[1];
var cursor_pointer = norm[0];
var d = ndiffs[cursor_pointer];
var d_next = ndiffs[cursor_pointer + 1];

if (d[0] !== DIFF_EQUAL) {
// A modification happened at the cursor location.
// This is the expected outcome, so we can return the original diff.
return diffs;
} else {
if (d_next != null && d[1] + d_next[1] === d_next[1] + d[1]) {
// Case 1)
// It is possible to perform a naive shift
ndiffs.splice(cursor_pointer, 2, d_next, d)
return merge_tuples(ndiffs, cursor_pointer, 2)
} else if (d_next != null && d_next[1].indexOf(d[1]) === 0) {
// Case 2)
// d[1] is a prefix of d_next[1]
// We can assume that d_next[0] !== 0, since d[0] === 0
// Shift edit locations..
ndiffs.splice(cursor_pointer, 2, [d_next[0], d[1]], [0, d[1]]);
var suffix = d_next[1].slice(d[1].length);
if (suffix.length > 0) {
ndiffs.splice(cursor_pointer + 2, 0, [d_next[0], suffix]);
}
return merge_tuples(ndiffs, cursor_pointer, 3)
} else {
// Not possible to perform any modification
return diffs;
}
}

}

/*
* Try to merge tuples with their neigbors in a given range.
* E.g. [0, 'a'], [0, 'b'] -> [0, 'ab']
*
* @param {Array} diffs Array of diff tuples.
* @param {Int} start Position of the first element to merge (diffs[start] is also merged with diffs[start - 1]).
* @param {Int} length Number of consecutive elements to check.
* @return {Array} Array of merged diff tuples.
*/
function merge_tuples (diffs, start, length) {
// Check from (start-1) to (start+length).
for (var i = start + length - 1; i >= 0 && i >= start - 1; i--) {
if (i + 1 < diffs.length) {
var left_d = diffs[i];
var right_d = diffs[i+1];
if (left_d[0] === right_d[1]) {
diffs.splice(i, 2, [left_d[0], left_d[1] + right_d[1]]);
}
}
}
return diffs;
}
15 changes: 14 additions & 1 deletion test.js
Original file line number Diff line number Diff line change
Expand Up @@ -25,7 +25,7 @@ for(var i = 0; i <= ITERATIONS; ++i) {
strings.push(chars.join(''));
}

console.log('Running tests...');
console.log('Running tests *without* cursor information...');
for(var i = 0; i < ITERATIONS; ++i) {
var result = diff(strings[i], strings[i+1]);
var expected = googlediff.diff_main(strings[i], strings[i+1]);
Expand All @@ -36,4 +36,17 @@ for(var i = 0; i < ITERATIONS; ++i) {
}
}

console.log('Running tests *with* cursor information');
for(var i = 0; i < ITERATIONS; ++i) {
var cursor_pos = Math.floor(random() * strings[i].length);
var diffs = diff(strings[i], strings[i+1], cursor_pos);
var patch = googlediff.patch_make(strings[i], strings[i+1], diffs);
var expected = googlediff.patch_apply(patch, strings[i])[0];
if (expected !== strings[i+1]) {
console.log('Expected', expected);
console.log('Result', strings[i+1]);
throw new Error('Diff produced difference results.');
}
}

console.log("Success!");