Merge pull request #98 from ckeditor/t/97

Other: Use transparent batches in undo & redo commands. Closes #97.

Author: Reinmar

src/redocommand.js

@@ -30,7 +30,7 @@ export default class RedoCommand extends BaseCommand {
 	 */
 	execute() {
 		const item = this._stack.pop();
-		const redoingBatch = this.editor.model.createBatch();
+		const redoingBatch = this.editor.model.createBatch( 'transparent' );
 
 		// All changes have to be done in one `enqueueChange` callback so other listeners will not step between consecutive
 		// operations, or won't do changes to the document before selection is properly restored.

src/undocommand.js

@@ -33,7 +33,7 @@ export default class UndoCommand extends BaseCommand {
 		const batchIndex = batch ? this._stack.findIndex( a => a.batch == batch ) : this._stack.length - 1;
 
 		const item = this._stack.splice( batchIndex, 1 )[ 0 ];
-		const undoingBatch = this.editor.model.createBatch();
+		const undoingBatch = this.editor.model.createBatch( 'transparent' );
 
 		// All changes has to be done in one `enqueueChange` callback so other listeners will not
 		// step between consecutive operations, or won't do changes to the document before selection is properly restored.

src/undoediting.js

@@ -78,14 +78,18 @@ export default class UndoEditing extends Plugin {
 
 			const batch = operation.batch;
 
+			const isRedoBatch = this._redoCommand._createdBatches.has( batch );
+			const isUndoBatch = this._undoCommand._createdBatches.has( batch );
+			const isRegisteredBatch = this._batchRegistry.has( batch );
+
 			// If changes are not a part of a batch or this is not a new batch, omit those changes.
-			if ( this._batchRegistry.has( batch ) || batch.type == 'transparent' ) {
+			if ( isRegisteredBatch || ( batch.type == 'transparent' && !isRedoBatch && !isUndoBatch ) ) {
 				return;
 			} else {
-				if ( this._redoCommand._createdBatches.has( batch ) ) {
+				if ( isRedoBatch ) {
 					// If this batch comes from `redoCommand`, add it to `undoCommand` stack.
 					this._undoCommand.addBatch( batch );
-				} else if ( !this._undoCommand._createdBatches.has( batch ) ) {
+				} else if ( !isUndoBatch ) {
 					// A default batch - these are new changes in the document, not introduced by undo feature.
 					// Add them to `undoCommand` stack and clear `redoCommand` stack.
 					this._undoCommand.addBatch( batch );

tests/undoediting.js

@@ -82,6 +82,27 @@ describe( 'UndoEditing', () => {
 		expect( undo._redoCommand.clearStack.called ).to.be.false;
 	} );
 
+	it( 'should add redo batch to undo', () => {
+		sinon.spy( undo._undoCommand, 'addBatch' );
+
+		model.change( writer => {
+			writer.insertText( 'foobar', root );
+		} );
+
+		model.change( writer => {
+			writer.insertText( 'baz', root );
+		} );
+
+		editor.execute( 'undo' );
+		editor.execute( 'undo' );
+
+		editor.execute( 'redo' );
+		sinon.assert.calledThrice( undo._undoCommand.addBatch );
+
+		editor.execute( 'redo' );
+		sinon.assert.callCount( undo._undoCommand.addBatch, 4 );
+	} );
+
 	it( 'should not add a batch that has only non-document operations', () => {
 		sinon.spy( undo._undoCommand, 'addBatch' );
 
@@ -95,6 +116,16 @@ describe( 'UndoEditing', () => {
 		expect( undo._undoCommand.addBatch.called ).to.be.false;
 	} );
 
+	it( 'should not add a transparent batch', () => {
+		sinon.spy( undo._undoCommand, 'addBatch' );
+
+		model.enqueueChange( 'transparent', writer => {
+			writer.insertText( 'foobar', root );
+		} );
+
+		expect( undo._undoCommand.addBatch.called ).to.be.false;
+	} );
+
 	it( 'should add a batch that has both document and non-document operations', () => {
 		sinon.spy( undo._undoCommand, 'addBatch' );