jenkins-bot has submitted this change and it was merged.
Change subject: Implement ve.dm.Surface.prototype.undo() and redo() in terms of
change()
......................................................................
Implement ve.dm.Surface.prototype.undo() and redo() in terms of change()
...or really changeInternal(), so we can avoid adding undo transactions
to the undo stack.
Also get rid of the pattern where undo() and redo() return a selection
which the caller then has to restore, and instead just restore the
selection.
Bug: 53224
Change-Id: If5a3b4d4162e9f0713ee9cd26e79a66efe52770f
---
M modules/ve/dm/ve.dm.Surface.js
M modules/ve/test/ui/actions/ve.ui.IndentationAction.test.js
M modules/ve/test/ui/actions/ve.ui.ListAction.test.js
M modules/ve/test/ve.test.utils.js
M modules/ve/ui/actions/ve.ui.HistoryAction.js
5 files changed, 45 insertions(+), 50 deletions(-)
Approvals:
Divec: Looks good to me, approved
jenkins-bot: Verified
diff --git a/modules/ve/dm/ve.dm.Surface.js b/modules/ve/dm/ve.dm.Surface.js
index e7d912b..36952cd 100644
--- a/modules/ve/dm/ve.dm.Surface.js
+++ b/modules/ve/dm/ve.dm.Surface.js
@@ -458,12 +458,28 @@
* @method
* @param {ve.dm.Transaction|ve.dm.Transaction[]|null} transactions One or
more transactions to
* process, or null to process none
- * @param {ve.Range|undefined} selection
+ * @param {ve.Range} [selection] Selection to apply
* @fires lock
* @fires contextChange
* @fires unlock
*/
ve.dm.Surface.prototype.change = function ( transactions, selection ) {
+ this.changeInternal( transactions, selection, false );
+};
+
+/**
+ * Internal implementation of change(). Do not use this, use change() instead.
+ *
+ * @private
+ * @method
+ * @param {ve.dm.Transaction|ve.dm.Transaction[]|null} transactions
+ * @param {ve.Range} [selection] [selection]
+ * @param {boolean} [skipUndoStack=false] If true, do not modify the undo
stack. Used by undo/redo
+ * @fires lock
+ * @fires contextChange
+ * @fires unlock
+ */
+ve.dm.Surface.prototype.changeInternal = function ( transactions, selection,
skipUndoStack ) {
var i, len, selectionAfter, selectionBefore = this.selection,
contextChange = false;
if ( !this.enabled ) {
@@ -481,8 +497,10 @@
this.emit( 'lock' );
for ( i = 0, len = transactions.length; i < len; i++ ) {
if ( !transactions[i].isNoOp() ) {
- this.truncateUndoStack();
- this.smallStack.push( transactions[i] );
+ if ( !skipUndoStack ) {
+ this.truncateUndoStack();
+ this.smallStack.push( transactions[i] );
+ }
// The .commit() call below indirectly invokes
setSelection()
this.documentModel.commit( transactions[i] );
if (
transactions[i].hasElementAttributeOperations() ) {
@@ -545,66 +563,52 @@
* Step backwards in history.
*
* @method
- * @fires lock
- * @fires unlock
* @fires history
- * @returns {ve.Range} Selection or null if no further state could be reached
*/
ve.dm.Surface.prototype.undo = function () {
+ var i, item, selection, transaction, transactions = [];
if ( !this.enabled || !this.hasPastState() ) {
return;
}
- var item, i, transaction, selection;
+
this.breakpoint();
this.undoIndex++;
- if ( this.bigStack[this.bigStack.length - this.undoIndex] ) {
- this.emit( 'lock' );
- item = this.bigStack[this.bigStack.length - this.undoIndex];
+ item = this.bigStack[this.bigStack.length - this.undoIndex];
+ if ( item ) {
+ // Apply reversed transactions in reversed order, and translate
the selection accordingly
selection = item.selection;
-
for ( i = item.stack.length - 1; i >= 0; i-- ) {
transaction = item.stack[i].reversed();
selection = transaction.translateRange( selection );
- this.documentModel.commit( transaction );
+ transactions.push( transaction );
}
- this.emit( 'unlock' );
+ this.changeInternal( transactions, selection, true );
this.emit( 'history' );
- return selection;
}
- return null;
};
/**
* Step forwards in history.
*
* @method
- * @fires lock
- * @fires unlock
* @fires history
- * @returns {ve.Range} Selection or null if no further state could be reached
*/
ve.dm.Surface.prototype.redo = function () {
+ var item;
if ( !this.enabled || !this.hasFutureState() ) {
return;
}
- var item, i, transaction, selection;
+
this.breakpoint();
- if ( this.bigStack[this.bigStack.length - this.undoIndex] ) {
- this.emit( 'lock' );
- item = this.bigStack[this.bigStack.length - this.undoIndex];
- selection = item.selection;
- for ( i = 0; i < item.stack.length; i++ ) {
- transaction = item.stack[i].clone();
- this.documentModel.commit( transaction );
- }
+ item = this.bigStack[this.bigStack.length - this.undoIndex];
+ if ( item ) {
+ // ve.copy( item.stack ) invokes .clone() on each transaction
in item.stack
+ this.changeInternal( ve.copy( item.stack ), item.selection,
true );
this.undoIndex--;
- this.emit( 'unlock' );
this.emit( 'history' );
- return selection;
}
- return null;
};
/**
diff --git a/modules/ve/test/ui/actions/ve.ui.IndentationAction.test.js
b/modules/ve/test/ui/actions/ve.ui.IndentationAction.test.js
index 1e112dc..df98ae9 100644
--- a/modules/ve/test/ui/actions/ve.ui.IndentationAction.test.js
+++ b/modules/ve/test/ui/actions/ve.ui.IndentationAction.test.js
@@ -10,8 +10,7 @@
/* Tests */
function runIndentationChangeTest( assert, range, method, expectedSelection,
expectedData, expectedOriginalData, msg ) {
- var selection,
- surface = ve.test.utils.createSurfaceFromHtml(
ve.dm.example.isolationHtml ),
+ var surface = ve.test.utils.createSurfaceFromHtml(
ve.dm.example.isolationHtml ),
indentationAction = new ve.ui.IndentationAction( surface ),
data = ve.copy( surface.getModel().getDocument().getFullData()
),
originalData = ve.copy( data );
@@ -27,10 +26,10 @@
assert.deepEqual( surface.getModel().getDocument().getFullData(), data,
msg + ': data models match' );
assert.deepEqual( surface.getModel().getSelection(), expectedSelection,
msg + ': selections match' );
- selection = surface.getModel().undo();
+ surface.getModel().undo();
assert.deepEqual( surface.getModel().getDocument().getFullData(),
originalData, msg + ' (undo): data models match' );
- assert.deepEqual( selection, range, msg + ' (undo): selections match' );
+ assert.deepEqual( surface.getModel().getSelection(), range, msg + '
(undo): selections match' );
surface.destroy();
}
diff --git a/modules/ve/test/ui/actions/ve.ui.ListAction.test.js
b/modules/ve/test/ui/actions/ve.ui.ListAction.test.js
index a41e6fa..bf4a1d5 100644
--- a/modules/ve/test/ui/actions/ve.ui.ListAction.test.js
+++ b/modules/ve/test/ui/actions/ve.ui.ListAction.test.js
@@ -10,8 +10,7 @@
/* Tests */
function runListConverterTest( assert, html, method, style, range,
expectedSelection, expectedData, expectedOriginalData, msg ) {
- var selection,
- surface = ve.test.utils.createSurfaceFromHtml( html ||
ve.dm.example.html ),
+ var surface = ve.test.utils.createSurfaceFromHtml( html ||
ve.dm.example.html ),
listAction = new ve.ui.ListAction( surface ),
data = ve.copy( surface.getModel().getDocument().getFullData()
),
originalData = ve.copy( data );
@@ -26,10 +25,10 @@
assert.deepEqual( surface.getModel().getDocument().getFullData(), data,
msg + ': data models match' );
assert.deepEqual( surface.getModel().getSelection(), expectedSelection,
msg + ': selections match' );
- selection = surface.getModel().undo();
+ surface.getModel().undo();
assert.deepEqual( surface.getModel().getDocument().getFullData(),
originalData, msg + ' (undo): data models match' );
- assert.deepEqual( selection, range, msg + ' (undo): selections match' );
+ assert.deepEqual( surface.getModel().getSelection(), range, msg + '
(undo): selections match' );
surface.destroy();
}
diff --git a/modules/ve/test/ve.test.utils.js b/modules/ve/test/ve.test.utils.js
index 961d35d..2fb5093 100644
--- a/modules/ve/test/ve.test.utils.js
+++ b/modules/ve/test/ve.test.utils.js
@@ -26,8 +26,7 @@
};
ve.test.utils.runFormatConverterTest = function ( assert, range, type,
attributes, expectedSelection, expectedData, msg ) {
- var selection,
- surface = ve.test.utils.createSurfaceFromHtml(
ve.dm.example.isolationHtml ),
+ var surface = ve.test.utils.createSurfaceFromHtml(
ve.dm.example.isolationHtml ),
formatAction = new ve.ui.FormatAction( surface ),
data = ve.copy( surface.getModel().getDocument().getFullData()
),
originalData = ve.copy( data );
@@ -40,10 +39,10 @@
assert.deepEqual( surface.getModel().getDocument().getFullData(), data,
msg + ': data models match' );
assert.deepEqual( surface.getModel().getSelection(), expectedSelection,
msg + ': selections match' );
- selection = surface.getModel().undo();
+ surface.getModel().undo();
assert.deepEqual( surface.getModel().getDocument().getFullData(),
originalData, msg + ' (undo): data models match' );
- assert.deepEqual( selection, range, msg + ' (undo): selections match' );
+ assert.deepEqual( surface.getModel().getSelection(), range, msg + '
(undo): selections match' );
surface.destroy();
};
diff --git a/modules/ve/ui/actions/ve.ui.HistoryAction.js
b/modules/ve/ui/actions/ve.ui.HistoryAction.js
index 3b520bf..f604156 100644
--- a/modules/ve/ui/actions/ve.ui.HistoryAction.js
+++ b/modules/ve/ui/actions/ve.ui.HistoryAction.js
@@ -42,10 +42,7 @@
* @method
*/
ve.ui.HistoryAction.prototype.undo = function () {
- var range = this.surface.getModel().undo();
- if ( range ) {
- this.surface.getModel().change( null, range );
- }
+ this.surface.getModel().undo();
};
/**
@@ -54,10 +51,7 @@
* @method
*/
ve.ui.HistoryAction.prototype.redo = function () {
- var range = this.surface.getModel().redo();
- if ( range ) {
- this.surface.getModel().change( null, range );
- }
+ this.surface.getModel().redo();
};
/* Registration */
--
To view, visit https://gerrit.wikimedia.org/r/88728
To unsubscribe, visit https://gerrit.wikimedia.org/r/settings
Gerrit-MessageType: merged
Gerrit-Change-Id: If5a3b4d4162e9f0713ee9cd26e79a66efe52770f
Gerrit-PatchSet: 11
Gerrit-Project: mediawiki/extensions/VisualEditor
Gerrit-Branch: master
Gerrit-Owner: Catrope <[email protected]>
Gerrit-Reviewer: Catrope <[email protected]>
Gerrit-Reviewer: Cscott <[email protected]>
Gerrit-Reviewer: Divec <[email protected]>
Gerrit-Reviewer: Esanders <[email protected]>
Gerrit-Reviewer: jenkins-bot
_______________________________________________
MediaWiki-commits mailing list
[email protected]
https://lists.wikimedia.org/mailman/listinfo/mediawiki-commits
