[MediaWiki-commits] [Gerrit] Implement ve.dm.Surface.prototype.undo() and redo() in terms... - change (mediawiki...VisualEditor)
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 (
[MediaWiki-commits] [Gerrit] Implement ve.dm.Surface.prototype.undo() and redo() in terms... - change (mediawiki...VisualEditor)
Catrope has uploaded a new change for review.
https://gerrit.wikimedia.org/r/88728
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/ce/ve.ce.Surface.test.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
6 files changed, 47 insertions(+), 52 deletions(-)
git pull ssh://gerrit.wikimedia.org:29418/mediawiki/extensions/VisualEditor
refs/changes/28/88728/1
diff --git a/modules/ve/dm/ve.dm.Surface.js b/modules/ve/dm/ve.dm.Surface.js
index bf949ea..a5690c2 100644
--- a/modules/ve/dm/ve.dm.Surface.js
+++ b/modules/ve/dm/ve.dm.Surface.js
@@ -454,12 +454,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
* @emits lock
* @emits contextChange
* @emits 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
+ * @emits lock
+ * @emits contextChange
+ * @emits unlock
+ */
+ve.dm.Surface.prototype.changeInternal = function ( transactions, selection,
skipUndoStack ) {
var i, len, contextChange = false;
if ( !this.enabled ) {
@@ -477,8 +493,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() ) {
@@ -533,66 +551,52 @@
* Step backwards in history.
*
* @method
- * @emits lock
- * @emits unlock
* @emits 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
- * @emits lock
- * @emits unlock
* @emits 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;
+
