Browse files

fix composition of changes in EditorClient (closes #19)

  • Loading branch information...
1 parent 096c135 commit 9f164f19a431020c61ca13d28939f8e14408dbee @timjb timjb committed Mar 18, 2013
Showing with 56 additions and 31 deletions.
  1. +2 −9 lib/editor-client.js
  2. +43 −22 lib/text-operation.js
  3. +11 −0 test/lib/test-text-operation.js
View
11 lib/editor-client.js
@@ -110,7 +110,6 @@ ot.EditorClient = (function () {
this.serverAdapter = serverAdapter;
this.editorAdapter = editorAdapter;
this.undoManager = new UndoManager();
- this.lastOperation = null;
this.initializeClientList();
this.initializeClients(clients);
@@ -237,14 +236,8 @@ ot.EditorClient = (function () {
var meta = new SelfMeta(cursorBefore, this.cursor);
var operation = new WrappedOperation(textOperation, meta);
- var compose;
- if (!this.undoManager.dontCompose && this.lastOperation && this.lastOperation.shouldBeComposedWith(textOperation)) {
- compose = true;
- this.lastOperation = this.lastOperation.compose(textOperation);
- } else {
- compose = false;
- this.lastOperation = textOperation;
- }
+ var compose = this.undoManager.undoStack.length > 0 &&
+ inverse.shouldBeComposedWithInverted(last(this.undoManager.undoStack).wrapped);
var inverseMeta = new SelfMeta(this.cursor, cursorBefore);
this.undoManager.add(new WrappedOperation(inverse, inverseMeta), compose);
this.applyClient(textOperation);
View
65 lib/text-operation.js
@@ -334,6 +334,25 @@ ot.TextOperation = (function () {
return operation;
};
+ function getSimpleOp (operation, fn) {
+ var ops = operation.ops;
+ var isRetain = TextOperation.isRetain;
+ switch (ops.length) {
+ case 1:
+ return ops[0];
+ case 2:
+ return isRetain(ops[0]) ? ops[1] : (isRetain(ops[1]) ? ops[0] : null);
+ case 3:
+ if (isRetain(ops[0]) && isRetain(ops[2])) { return ops[1]; }
+ }
+ return null;
+ }
+
+ function getStartIndex (operation) {
+ if (isRetain(operation.ops[0])) { return operation.ops[0]; }
+ return 0;
+ }
+
// When you use ctrl-z to undo your latest changes, you expect the program not
// to undo every single keystroke but to undo your last sentence you wrote at
// a stretch or the deletion you did by holding the backspace key down. This
@@ -343,25 +362,6 @@ ot.TextOperation = (function () {
// operations delete text at the same position. You may want to include other
// factors like the time since the last change in your decision.
TextOperation.prototype.shouldBeComposedWith = function (other) {
- function getSimpleOp (operation, fn) {
- var ops = operation.ops;
- var isRetain = TextOperation.isRetain;
- switch (ops.length) {
- case 1:
- return ops[0];
- case 2:
- return isRetain(ops[0]) ? ops[1] : (isRetain(ops[1]) ? ops[0] : null);
- case 3:
- if (isRetain(ops[0]) && isRetain(ops[2])) { return ops[1]; }
- }
- return null;
- }
-
- function getStartIndex (operation) {
- if (isRetain(operation.ops[0])) { return operation.ops[0]; }
- return 0;
- }
-
if (this.isNoop() || other.isNoop()) { return true; }
var startA = getStartIndex(this), startB = getStartIndex(other);
@@ -381,10 +381,31 @@ ot.TextOperation = (function () {
return false;
};
+ // Decides whether two operations should be composed with each other
+ // if they were inverted, that is
+ // `shouldBeComposedWith(a, b) = shouldBeComposedWithInverted(b^{-1}, a^{-1})`.
+ TextOperation.prototype.shouldBeComposedWithInverted = function (other) {
+ if (this.isNoop() || other.isNoop()) { return true; }
+
+ var startA = getStartIndex(this), startB = getStartIndex(other);
+ var simpleA = getSimpleOp(this), simpleB = getSimpleOp(other);
+ if (!simpleA || !simpleB) { return false; }
+
+ if (isInsert(simpleA) && isInsert(simpleB)) {
+ return startA + simpleA.length === startB || startA === startB;
+ }
+
+ if (isDelete(simpleA) && isDelete(simpleB)) {
+ return startB - simpleB === startA;
+ }
+
+ return false;
+ };
+
// Transform takes two operations A and B that happened concurrently and
- // produces two operations A' and B' (in an arry) such that
- // apply(apply(S, A), B') = apply(apply(S, B), A'). This function is the heart
- // of OT.
+ // produces two operations A' and B' (in an array) such that
+ // `apply(apply(S, A), B') = apply(apply(S, B), A')`. This function is the
+ // heart of OT.
TextOperation.transform = function (operation1, operation2) {
if (operation1.baseLength !== operation2.baseLength) {
throw new Error("Both operations have to have the same base length");
View
11 test/lib/test-text-operation.js
@@ -179,6 +179,17 @@ exports.testShouldBeComposedWith = function (test) {
test.done();
};
+exports.testShouldBeComposedWithInverted = h.randomTest(2*n, function (test) {
+ // invariant: shouldBeComposedWith(a, b) = shouldBeComposedWithInverted(b^{-1}, a^{-1})
+ var str = h.randomString();
+ var a = h.randomOperation(str);
+ var aInv = a.invert(str);
+ var afterA = a.apply(str);
+ var b = h.randomOperation(afterA);
+ var bInv = b.invert(afterA);
+ test.strictEqual(a.shouldBeComposedWith(b), bInv.shouldBeComposedWithInverted(aInv));
+});
+
exports.testCompose = h.randomTest(n, function (test) {
// invariant: apply(str, compose(a, b)) === apply(apply(str, a), b)
var str = h.randomString(20);

0 comments on commit 9f164f1

Please sign in to comment.