Merge pull request #65 from ckeditor/t/64

Feature: `BlockAutoformatEditing` will not format if the command is disabled. `InlineAutoformatEditing` will not format if the callback returned `false`. Closes #64.

Author: Piotr Jasiun

src/autoformat.js

@@ -78,24 +78,29 @@ export default class Autoformat extends Plugin {
 
 		if ( commands.get( 'bold' ) ) {
 			/* eslint-disable no-new */
-			new InlineAutoformatEditing( this.editor, /(\*\*)([^*]+)(\*\*)$/g, 'bold' );
-			new InlineAutoformatEditing( this.editor, /(__)([^_]+)(__)$/g, 'bold' );
+			const boldCallback = getCallbackFunctionForInlineAutoformat( this.editor, 'bold' );
+
+			new InlineAutoformatEditing( this.editor, /(\*\*)([^*]+)(\*\*)$/g, boldCallback );
+			new InlineAutoformatEditing( this.editor, /(__)([^_]+)(__)$/g, boldCallback );
 			/* eslint-enable no-new */
 		}
 
 		if ( commands.get( 'italic' ) ) {
+			/* eslint-disable no-new */
+			const italicCallback = getCallbackFunctionForInlineAutoformat( this.editor, 'italic' );
+
 			// The italic autoformatter cannot be triggered by the bold markers, so we need to check the
 			// text before the pattern (e.g. `(?:^|[^\*])`).
-
-			/* eslint-disable no-new */
-			new InlineAutoformatEditing( this.editor, /(?:^|[^*])(\*)([^*_]+)(\*)$/g, 'italic' );
-			new InlineAutoformatEditing( this.editor, /(?:^|[^_])(_)([^_]+)(_)$/g, 'italic' );
+			new InlineAutoformatEditing( this.editor, /(?:^|[^*])(\*)([^*_]+)(\*)$/g, italicCallback );
+			new InlineAutoformatEditing( this.editor, /(?:^|[^_])(_)([^_]+)(_)$/g, italicCallback );
 			/* eslint-enable no-new */
 		}
 
 		if ( commands.get( 'code' ) ) {
 			/* eslint-disable no-new */
-			new InlineAutoformatEditing( this.editor, /(`)([^`]+)(`)$/g, 'code' );
+			const codeCallback = getCallbackFunctionForInlineAutoformat( this.editor, 'code' );
+
+			new InlineAutoformatEditing( this.editor, /(`)([^`]+)(`)$/g, codeCallback );
 			/* eslint-enable no-new */
 		}
 	}
@@ -144,3 +149,28 @@ export default class Autoformat extends Plugin {
 		}
 	}
 }
+
+// Helper function for getting `InlineAutoformatEditing` callbacks that checks if command is enabled.
+//
+// @param {module:core/editor/editor~Editor} editor
+// @param {String} attributeKey
+// @returns {Function}
+function getCallbackFunctionForInlineAutoformat( editor, attributeKey ) {
+	return ( writer, rangesToFormat ) => {
+		const command = editor.commands.get( attributeKey );
+
+		if ( !command.isEnabled ) {
+			return false;
+		}
+
+		const validRanges = editor.model.schema.getValidRanges( rangesToFormat, attributeKey );
+
+		for ( const range of validRanges ) {
+			writer.setAttribute( attributeKey, true, range );
+		}
+
+		// After applying attribute to the text, remove given attribute from the selection.
+		// This way user is able to type a text without attribute used by auto formatter.
+		writer.removeSelectionAttribute( attributeKey );
+	};
+}

src/blockautoformatediting.js

@@ -26,7 +26,7 @@ export default class BlockAutoformatEditing {
 	 *
 	 * Examples of usage:
 	 *
-	 * To convert a paragraph to heading 1 when `- ` is typed, using just the commmand name:
+	 * To convert a paragraph to heading 1 when `- ` is typed, using just the command name:
 	 *
 	 *		new BlockAutoformatEditing( editor, /^\- $/, 'heading1' );
 	 *
@@ -49,19 +49,24 @@ export default class BlockAutoformatEditing {
 	 */
 	constructor( editor, pattern, callbackOrCommand ) {
 		let callback;
+		let command = null;
 
 		if ( typeof callbackOrCommand == 'function' ) {
 			callback = callbackOrCommand;
 		} else {
 			// We assume that the actual command name was provided.
-			const command = callbackOrCommand;
+			command = editor.commands.get( callbackOrCommand );
 
 			callback = () => {
-				editor.execute( command );
+				editor.execute( callbackOrCommand );
 			};
 		}
 
 		editor.model.document.on( 'change', ( evt, batch ) => {
+			if ( command && !command.isEnabled ) {
+				return;
+			}
+
 			if ( batch.type == 'transparent' ) {
 				return;
 			}

src/inlineautoformatediting.js

@@ -52,15 +52,23 @@ export default class InlineAutoformatEditing {
 	 *		}
 	 *
 	 * @param {Function|String} attributeOrCallback The name of attribute to apply on matching text or a callback for manual
-	 * formatting.
+	 * formatting. If callback is passed it should return `false` if changes should not be applied (e.g. if a command is disabled).
 	 *
 	 *		// Use attribute name:
 	 *		new InlineAutoformatEditing( editor, /(\*\*)([^\*]+?)(\*\*)$/g, 'bold' );
 	 *
 	 *		// Use formatting callback:
-	 *		new InlineAutoformatEditing( editor, /(\*\*)([^\*]+?)(\*\*)$/g, ( writer, validRanges ) => {
+	 *		new InlineAutoformatEditing( editor, /(\*\*)([^\*]+?)(\*\*)$/g, ( writer, rangesToFormat ) => {
+	 *			const command = editor.commands.get( 'bold' );
+	 *
+	 *			if ( !command.isEnabled ) {
+	 *				return false;
+	 *			}
+	 *
+	 *			const validRanges = editor.model.schema.getValidRanges( rangesToFormat, 'bold' );
+	 *
 	 *			for ( let range of validRanges ) {
-	 *				writer.setAttribute( command, true, range );
+	 *				writer.setAttribute( 'bold', true, range );
 	 *			}
 	 *		} );
 	 */
@@ -128,7 +136,9 @@ export default class InlineAutoformatEditing {
 		} );
 
 		// A format callback run on matched text.
-		formatCallback = formatCallback || ( ( writer, validRanges ) => {
+		formatCallback = formatCallback || ( ( writer, rangesToFormat ) => {
+			const validRanges = editor.model.schema.getValidRanges( rangesToFormat, attributeKey );
+
 			for ( const range of validRanges ) {
 				writer.setAttribute( attributeKey, true, range );
 			}
@@ -170,10 +180,13 @@ export default class InlineAutoformatEditing {
 
 			// Use enqueueChange to create new batch to separate typing batch from the auto-format changes.
 			editor.model.enqueueChange( writer => {
-				const validRanges = editor.model.schema.getValidRanges( rangesToFormat, attributeKey );
-
 				// Apply format.
-				formatCallback( writer, validRanges );
+				const hasChanged = formatCallback( writer, rangesToFormat );
+
+				// Strict check on `false` to have backward compatibility (when callbacks were returning `undefined`).
+				if ( hasChanged === false ) {
+					return;
+				}
 
 				// Remove delimiters - use reversed order to not mix the offsets while removing.
 				for ( const range of rangesToRemove.reverse() ) {

tests/autoformat.js

@@ -272,6 +272,22 @@ describe( 'Autoformat', () => {
 
 			expect( getData( model ) ).to.equal( '<paragraph>foo <$text bold="true">bar</$text>[] baz</paragraph>' );
 		} );
+
+		it( 'should not format if the command is not enabled', () => {
+			model.schema.addAttributeCheck( ( context, attributeName ) => {
+				if ( attributeName == 'bold' ) {
+					return false;
+				}
+			} );
+
+			setData( model, '<paragraph>**foobar*[]</paragraph>' );
+
+			model.change( writer => {
+				writer.insertText( '*', doc.selection.getFirstPosition() );
+			} );
+
+			expect( getData( model ) ).to.equal( '<paragraph>**foobar**[]</paragraph>' );
+		} );
 	} );
 
 	describe( 'without commands', () => {

tests/blockautoformatediting.js

@@ -3,6 +3,7 @@
  * For licensing, see LICENSE.md.
  */
 
+import Autoformat from '../src/autoformat';
 import BlockAutoformatEditing from '../src/blockautoformatediting';
 import Paragraph from '@ckeditor/ckeditor5-paragraph/src/paragraph';
 import VirtualTestEditor from '@ckeditor/ckeditor5-core/tests/_utils/virtualtesteditor';
@@ -19,7 +20,7 @@ describe( 'BlockAutoformatEditing', () => {
 	beforeEach( () => {
 		return VirtualTestEditor
 			.create( {
-				plugins: [ Enter, Paragraph ]
+				plugins: [ Enter, Paragraph, Autoformat ]
 			} )
 			.then( newEditor => {
 				editor = newEditor;
@@ -28,13 +29,17 @@ describe( 'BlockAutoformatEditing', () => {
 			} );
 	} );
 
-	describe( 'Command name', () => {
+	describe( 'command name', () => {
 		it( 'should run a command when the pattern is matched', () => {
 			const spy = testUtils.sinon.spy();
-			editor.commands.add( 'testCommand', new TestCommand( editor, spy ) );
+			const testCommand = new TestCommand( editor, spy );
+
+			editor.commands.add( 'testCommand', testCommand );
+
 			new BlockAutoformatEditing( editor, /^[*]\s$/, 'testCommand' ); // eslint-disable-line no-new
 
 			setData( model, '<paragraph>*[]</paragraph>' );
+
 			model.change( writer => {
 				writer.insertText( ' ', doc.selection.getFirstPosition() );
 			} );
@@ -44,20 +49,45 @@ describe( 'BlockAutoformatEditing', () => {
 
 		it( 'should remove found pattern', () => {
 			const spy = testUtils.sinon.spy();
-			editor.commands.add( 'testCommand', new TestCommand( editor, spy ) );
+			const testCommand = new TestCommand( editor, spy );
+
+			editor.commands.add( 'testCommand', testCommand );
+
 			new BlockAutoformatEditing( editor, /^[*]\s$/, 'testCommand' ); // eslint-disable-line no-new
 
 			setData( model, '<paragraph>*[]</paragraph>' );
+
 			model.change( writer => {
 				writer.insertText( ' ', doc.selection.getFirstPosition() );
 			} );
 
 			sinon.assert.calledOnce( spy );
 			expect( getData( model ) ).to.equal( '<paragraph>[]</paragraph>' );
 		} );
+
+		it( 'should not autoformat if command is disabled', () => {
+			const spy = testUtils.sinon.spy();
+			const testCommand = new TestCommand( editor, spy );
+
+			testCommand.refresh = function() {
+				this.isEnabled = false;
+			};
+
+			editor.commands.add( 'testCommand', testCommand );
+
+			new BlockAutoformatEditing( editor, /^[*]\s$/, 'testCommand' ); // eslint-disable-line no-new
+
+			setData( model, '<paragraph>*[]</paragraph>' );
+
+			model.change( writer => {
+				writer.insertText( ' ', doc.selection.getFirstPosition() );
+			} );
+
+			sinon.assert.notCalled( spy );
+		} );
 	} );
 
-	describe( 'Callback', () => {
+	describe( 'callback', () => {
 		it( 'should run callback when the pattern is matched', () => {
 			const spy = testUtils.sinon.spy();
 			new BlockAutoformatEditing( editor, /^[*]\s$/, spy ); // eslint-disable-line no-new

tests/inlineautoformatediting.js

@@ -3,6 +3,7 @@
  * For licensing, see LICENSE.md.
  */
 
+import Autoformat from '../src/autoformat';
 import InlineAutoformatEditing from '../src/inlineautoformatediting';
 import Paragraph from '@ckeditor/ckeditor5-paragraph/src/paragraph';
 import VirtualTestEditor from '@ckeditor/ckeditor5-core/tests/_utils/virtualtesteditor';
@@ -18,7 +19,7 @@ describe( 'InlineAutoformatEditing', () => {
 	beforeEach( () => {
 		return VirtualTestEditor
 			.create( {
-				plugins: [ Enter, Paragraph ]
+				plugins: [ Enter, Paragraph, Autoformat ]
 			} )
 			.then( newEditor => {
 				editor = newEditor;
@@ -64,7 +65,7 @@ describe( 'InlineAutoformatEditing', () => {
 		} );
 	} );
 
-	describe( 'Callback', () => {
+	describe( 'callback', () => {
 		it( 'should stop when there are no format ranges returned from testCallback', () => {
 			const formatSpy = testUtils.sinon.spy();
 			const testStub = testUtils.sinon.stub().returns( {
@@ -115,6 +116,27 @@ describe( 'InlineAutoformatEditing', () => {
 
 			sinon.assert.notCalled( formatSpy );
 		} );
+
+		it( 'should not autoformat if callback returned false', () => {
+			setData( model, '<paragraph>Foobar[]</paragraph>' );
+
+			const p = model.document.getRoot().getChild( 0 );
+
+			const testCallback = () => ( {
+				format: [ model.createRange( model.createPositionAt( p, 0 ), model.createPositionAt( p, 3 ) ) ],
+				remove: [ model.createRange( model.createPositionAt( p, 0 ), model.createPositionAt( p, 3 ) ) ]
+			} );
+
+			const formatCallback = () => false;
+
+			new InlineAutoformatEditing( editor, testCallback, formatCallback ); // eslint-disable-line no-new
+
+			model.change( writer => {
+				writer.insertText( ' ', doc.selection.getFirstPosition() );
+			} );
+
+			expect( getData( model ) ).to.equal( '<paragraph>Foobar []</paragraph>' );
+		} );
 	} );
 
 	it( 'should ignore transparent batches', () => {

tests/undointegration.js

@@ -20,7 +20,7 @@ import VirtualTestEditor from '@ckeditor/ckeditor5-core/tests/_utils/virtualtest
 import { setData, getData } from '@ckeditor/ckeditor5-engine/src/dev-utils/model';
 import testUtils from '@ckeditor/ckeditor5-core/tests/_utils/utils';
 
-describe( 'Autoformat', () => {
+describe( 'Autoformat undo integration', () => {
 	let editor, model, doc;
 
 	testUtils.createSinonSandbox();