Other: Aligned the implementation to the new Command API (see https:/…

…/github.com/ckeditor/ckeditor5-core/issues/88).

BREAKING CHANGES: The command API has been changed.

src/basecommand.js

@@ -7,13 +7,13 @@
  * @module undo/basecommand
  */
 
-import Command from '@ckeditor/ckeditor5-core/src/command/command';
+import Command from '@ckeditor/ckeditor5-core/src/command';
 
 /**
  * Base class for undo feature commands: {@link module:undo/undocommand~UndoCommand} and {@link module:undo/redocommand~RedoCommand}.
  *
  * @protected
- * @extends module:core/command/command~Command
+ * @extends module:core/command~Command
  */
 export default class BaseCommand extends Command {
 	constructor( editor ) {
@@ -38,8 +38,15 @@ export default class BaseCommand extends Command {
 		 */
 		this._createdBatches = new WeakSet();
 
-		// Refresh state, so command is inactive just after initialization.
-		this.refreshState();
+		// Refresh state, so the command is inactive right after initialization.
+		this.refresh();
+	}
+
+	/**
+	 * @inheritDoc
+	 */
+	refresh() {
+		this.isEnabled = this._stack.length > 0;
 	}
 
 	/**
@@ -55,22 +62,15 @@ export default class BaseCommand extends Command {
 		};
 
 		this._stack.push( { batch, selection } );
-		this.refreshState();
+		this.refresh();
 	}
 
 	/**
 	 * Removes all items from the stack.
 	 */
 	clearStack() {
 		this._stack = [];
-		this.refreshState();
-	}
-
-	/**
-	 * @inheritDoc
-	 */
-	_checkEnabled() {
-		return this._stack.length > 0;
+		this.refresh();
 	}
 
 	/**

src/redocommand.js

@@ -28,9 +28,9 @@ export default class RedoCommand extends BaseCommand {
 	 * {@link module:engine/model/document~Document document} and removes the batch from the stack.
 	 * Then, it restores the {@link module:engine/model/document~Document#selection document selection}.
 	 *
-	 * @protected
+	 * @fires execute
 	 */
-	_doExecute() {
+	execute() {
 		const item = this._stack.pop();
 
 		// All changes have to be done in one `enqueueChanges` callback so other listeners will not
@@ -51,12 +51,12 @@ export default class RedoCommand extends BaseCommand {
 			this._redo( item.batch );
 		} );
 
-		this.refreshState();
+		this.refresh();
 	}
 
 	/**
 	 * Redoes a batch by reversing the batch that has undone it, transforming that batch and applying it. This is
-	 * a helper method for {@link #_doExecute}.
+	 * a helper method for {@link #execute}.
 	 *
 	 * @private
 	 * @param {module:engine/model/batch~Batch} storedBatch The batch whose deltas will be reversed, transformed and applied.

src/undocommand.js

@@ -26,11 +26,11 @@ export default class UndoCommand extends BaseCommand {
 	 * and applies the reverted version on the {@link module:engine/model/document~Document document} and removes the batch from the stack.
 	 * Then, it restores the {@link module:engine/model/document~Document#selection document selection}.
 	 *
-	 * @protected
+	 * @fires execute
 	 * @fires revert
 	 * @param {module:engine/model/batch~Batch} [batch] A batch that should be undone. If not set, the last added batch will be undone.
 	 */
-	_doExecute( batch = null ) {
+	execute( batch = null ) {
 		// If batch is not given, set `batchIndex` to the last index in command stack.
 		const batchIndex = batch ? this._stack.findIndex( a => a.batch == batch ) : this._stack.length - 1;
 
@@ -47,7 +47,7 @@ export default class UndoCommand extends BaseCommand {
 			this.fire( 'revert', item.batch, undoingBatch );
 		} );
 
-		this.refreshState();
+		this.refresh();
 	}
 
 	/**
@@ -70,7 +70,7 @@ export default class UndoCommand extends BaseCommand {
 
 	/**
 	 * Undoes a batch by reversing a batch from history, transforming that reversed batch and applying it. This is
-	 * a helper method for {@link #_doExecute}.
+	 * a helper method for {@link #execute}.
 	 *
 	 * @private
 	 * @param {module:engine/model/batch~Batch} batchToUndo A batch whose deltas will be reversed, transformed and applied.

src/undoengine.js

@@ -60,8 +60,8 @@ export default class UndoEngine extends Plugin {
 		this._redoCommand = new RedoCommand( this.editor );
 
 		// Register command to the editor.
-		this.editor.commands.set( 'undo', this._undoCommand );
-		this.editor.commands.set( 'redo', this._redoCommand );
+		this.editor.commands.add( 'undo', this._undoCommand );
+		this.editor.commands.add( 'redo', this._redoCommand );
 
 		this.listenTo( this.editor.document, 'change', ( evt, type, changes, batch ) => {
 			// If changes are not a part of a batch or this is not a new batch, omit those changes.

tests/basecommand.js

@@ -22,19 +22,19 @@ describe( 'BaseCommand', () => {
 
 	describe( 'constructor()', () => {
 		it( 'should create command with empty batch stack', () => {
-			expect( base._checkEnabled() ).to.be.false;
+			expect( base.isEnabled ).to.be.false;
 		} );
 	} );
 
-	describe( '_checkEnabled', () => {
-		it( 'should return false if there are no batches in command stack', () => {
-			expect( base._checkEnabled() ).to.be.false;
+	describe( 'isEnabled', () => {
+		it( 'should be false if there are no batches in command stack', () => {
+			expect( base.isEnabled ).to.be.false;
 		} );
 
-		it( 'should return true if there are batches in command stack', () => {
+		it( 'should be true if there are batches in command stack', () => {
 			base.addBatch( doc.batch() );
 
-			expect( base._checkEnabled() ).to.be.true;
+			expect( base.isEnabled ).to.be.true;
 		} );
 	} );
 
@@ -43,7 +43,7 @@ describe( 'BaseCommand', () => {
 			base.addBatch( doc.batch() );
 			base.clearStack();
 
-			expect( base._checkEnabled() ).to.be.false;
+			expect( base.isEnabled ).to.be.false;
 		} );
 	} );
 } );

tests/redocommand.js

@@ -23,11 +23,11 @@ describe( 'RedoCommand', () => {
 	} );
 
 	afterEach( () => {
-		redo.destroy();
+		return editor.destroy();
 	} );
 
 	describe( 'RedoCommand', () => {
-		describe( '_execute', () => {
+		describe( 'execute()', () => {
 			const p = pos => new Position( root, [].concat( pos ) );
 			const r = ( a, b ) => new Range( p( a ), p( b ) );
 
@@ -95,9 +95,9 @@ describe( 'RedoCommand', () => {
 			} );
 
 			it( 'should redo batch undone by undo command', () => {
-				undo._execute( batch2 );
+				undo.execute( batch2 );
 
-				redo._execute();
+				redo.execute();
 				// Should be back at original state:
 				/*
 				 [root]
@@ -117,11 +117,11 @@ describe( 'RedoCommand', () => {
 			} );
 
 			it( 'should redo series of batches undone by undo command', () => {
-				undo._execute();
-				undo._execute();
-				undo._execute();
+				undo.execute();
+				undo.execute();
+				undo.execute();
 
-				redo._execute();
+				redo.execute();
 				// Should be like after applying `batch0`:
 				/*
 				 [root]
@@ -138,7 +138,7 @@ describe( 'RedoCommand', () => {
 				expect( editor.document.selection.getRanges().next().value.isEqual( r( 6, 6 ) ) ).to.be.true;
 				expect( editor.document.selection.isBackward ).to.be.false;
 
-				redo._execute();
+				redo.execute();
 				// Should be like after applying `batch1`:
 				/*
 				 [root]
@@ -156,7 +156,7 @@ describe( 'RedoCommand', () => {
 				expect( editor.document.selection.getRanges().next().value.isEqual( r( 2, 4 ) ) ).to.be.true;
 				expect( editor.document.selection.isBackward ).to.be.true;
 
-				redo._execute();
+				redo.execute();
 				// Should be like after applying `batch2`:
 				/*
 				 [root]
@@ -176,8 +176,8 @@ describe( 'RedoCommand', () => {
 			} );
 
 			it( 'should redo batch selectively undone by undo command', () => {
-				undo._execute( batch0 );
-				redo._execute();
+				undo.execute( batch0 );
+				redo.execute();
 
 				// Should be back to original state:
 				/*
@@ -198,10 +198,10 @@ describe( 'RedoCommand', () => {
 			} );
 
 			it( 'should redo batch selectively undone by undo command #2', () => {
-				undo._execute( batch1 );
-				undo._execute( batch2 );
-				redo._execute();
-				redo._execute();
+				undo.execute( batch1 );
+				undo.execute( batch2 );
+				redo.execute();
+				redo.execute();
 
 				// Should be back to original state:
 				/*
@@ -224,19 +224,19 @@ describe( 'RedoCommand', () => {
 			it( 'should transform redo batch by changes written in history that happened after undo but before redo #2', () => {
 				// Now it is "fBaroO".
 				// Undo moving "oo" to the end of string. Now it is "foOBar". Capitals mean set attribute.
-				undo._execute();
+				undo.execute();
 
 				// Remove "ar".
 				doc.batch().remove( r( 4, 6 ) );
 
 				// Undo setting attribute on "ob". Now it is "foob".
-				undo._execute();
+				undo.execute();
 
 				// Append "xx" at the beginning. Now it is "xxfoob".
 				doc.batch().insert( p( 0 ), 'xx' );
 
 				// Redo setting attribute on "ob". Now it is "xxfoOB".
-				redo._execute();
+				redo.execute();
 
 				expect( getText( root ) ).to.equal( 'xxfoob' );
 				expect( itemAt( root, 4 ).getAttribute( 'key' ) ).to.equal( 'value' );
@@ -245,7 +245,7 @@ describe( 'RedoCommand', () => {
 				expect( editor.document.selection.isBackward ).to.be.true;
 
 				// Redo moving "oo". Now it is "xxfBoO". Selection is expected to be on just moved "oO".
-				redo._execute();
+				redo.execute();
 
 				expect( getText( root ) ).to.equal( 'xxfboo' );
 				expect( itemAt( root, 3 ).getAttribute( 'key' ) ).to.equal( 'value' );