8
0
Просмотр исходного кода

Merge pull request #223 from ckeditor/i/6020

Other: Run only one instance of the `TextWatcher` for all text transformations. Closes ckeditor/ckeditor5#6020.
Piotrek Koszuliński 5 лет назад
Родитель
Сommit
ceb2ead427

+ 47 - 38
packages/ckeditor5-typing/src/texttransformation.js

@@ -92,8 +92,6 @@ export default class TextTransformation extends Plugin {
 				include: DEFAULT_TRANSFORMATIONS
 			}
 		} );
-
-		this.editor = editor;
 	}
 
 	/**
@@ -112,7 +110,7 @@ export default class TextTransformation extends Plugin {
 	}
 
 	/**
-	 * Create new set of TextWatchers listening to the editor for typing and selection events.
+	 * Create new TextWatcher listening to the editor for typing and selection events.
 	 *
 	 * @private
 	 */
@@ -120,52 +118,59 @@ export default class TextTransformation extends Plugin {
 		const editor = this.editor;
 		const model = editor.model;
 		const input = editor.plugins.get( 'Input' );
+		const normalizedTransformations = normalizeTransformations( editor.config.get( 'typing.transformations' ) );
 
-		const configuredTransformations = getConfiguredTransformations( editor.config.get( 'typing.transformations' ) );
+		const testCallback = text => {
+			for ( const normalizedTransformation of normalizedTransformations ) {
+				const from = normalizedTransformation.from;
+				const match = from.test( text );
 
-		for ( const transformation of configuredTransformations ) {
-			const from = normalizeFrom( transformation.from );
-			const to = normalizeTo( transformation.to );
+				if ( match ) {
+					return { normalizedTransformation };
+				}
+			}
+		};
 
-			const watcher = new TextWatcher( editor.model, text => from.test( text ) );
+		const watcherCallback = ( evt, data ) => {
+			if ( !input.isInput( data.batch ) ) {
+				return;
+			}
 
-			const watcherCallback = ( evt, data ) => {
-				if ( !input.isInput( data.batch ) ) {
-					return;
-				}
+			const { from, to } = data.normalizedTransformation;
 
-				const matches = from.exec( data.text );
-				const replaces = to( matches.slice( 1 ) );
+			const matches = from.exec( data.text );
+			const replaces = to( matches.slice( 1 ) );
 
-				const matchedRange = data.range;
+			const matchedRange = data.range;
 
-				let changeIndex = matches.index;
+			let changeIndex = matches.index;
 
-				model.enqueueChange( writer => {
-					for ( let i = 1; i < matches.length; i++ ) {
-						const match = matches[ i ];
-						const replaceWith = replaces[ i - 1 ];
+			model.enqueueChange( writer => {
+				for ( let i = 1; i < matches.length; i++ ) {
+					const match = matches[ i ];
+					const replaceWith = replaces[ i - 1 ];
 
-						if ( replaceWith == null ) {
-							changeIndex += match.length;
+					if ( replaceWith == null ) {
+						changeIndex += match.length;
 
-							continue;
-						}
+						continue;
+					}
 
-						const replacePosition = matchedRange.start.getShiftedBy( changeIndex );
-						const replaceRange = model.createRange( replacePosition, replacePosition.getShiftedBy( match.length ) );
-						const attributes = getTextAttributesAfterPosition( replacePosition );
+					const replacePosition = matchedRange.start.getShiftedBy( changeIndex );
+					const replaceRange = model.createRange( replacePosition, replacePosition.getShiftedBy( match.length ) );
+					const attributes = getTextAttributesAfterPosition( replacePosition );
 
-						model.insertContent( writer.createText( replaceWith, attributes ), replaceRange );
+					model.insertContent( writer.createText( replaceWith, attributes ), replaceRange );
 
-						changeIndex += replaceWith.length;
-					}
-				} );
-			};
+					changeIndex += replaceWith.length;
+				}
+			} );
+		};
 
-			watcher.on( 'matched:data', watcherCallback );
-			watcher.bind( 'isEnabled' ).to( this );
-		}
+		const watcher = new TextWatcher( editor.model, testCallback );
+
+		watcher.on( 'matched:data', watcherCallback );
+		watcher.bind( 'isEnabled' ).to( this );
 	}
 }
 
@@ -223,8 +228,8 @@ function buildQuotesRegExp( quoteCharacter ) {
 // Reads text transformation config and returns normalized array of transformations objects.
 //
 // @param {module:typing/texttransformation~TextTransformationDescription} config
-// @returns {Array.<module:typing/texttransformation~TextTransformationDescription>}
-function getConfiguredTransformations( config ) {
+// @returns {Array.<{from:String,to:Function}>}
+function normalizeTransformations( config ) {
 	const extra = config.extra || [];
 	const remove = config.remove || [];
 	const isNotRemoved = transformation => !remove.includes( transformation );
@@ -233,7 +238,11 @@ function getConfiguredTransformations( config ) {
 
 	return expandGroupsAndRemoveDuplicates( configured )
 		.filter( isNotRemoved ) // Filter out 'remove' transformations as they might be set in group
-		.map( transformation => TRANSFORMATIONS[ transformation ] || transformation );
+		.map( transformation => TRANSFORMATIONS[ transformation ] || transformation )
+		.map( transformation => ( {
+			from: normalizeFrom( transformation.from ),
+			to: normalizeTo( transformation.to )
+		} ) );
 }
 
 // Reads definitions and expands named groups if needed to transformation names.

+ 62 - 25
packages/ckeditor5-typing/src/textwatcher.js

@@ -26,11 +26,37 @@ export default class TextWatcher {
 	 * Creates a text watcher instance.
 	 *
 	 * @param {module:engine/model/model~Model} model
-	 * @param {Function} testCallback The function used to match the text.
+	 * @param {module:typing/textwatcher~TextWatcher#testCallback} testCallback
 	 */
 	constructor( model, testCallback ) {
+		/**
+		 * The editor's model.
+		 *
+		 * @readonly
+		 * @member {module:engine/model/model~Model}
+		 */
 		this.model = model;
+
+		/**
+		 * The function used to match the text.
+		 *
+		 * The test callback can return 3 values:
+		 *
+		 * * `false` if there is no match,
+		 * * `true` if there is a match,
+		 * * an object if there is a match and we want to pass some additional information to the {@link #matched:data} event.
+		 *
+		 * @member {Function} #testCallback
+		 * @returns {Object} testResult
+		 */
 		this.testCallback = testCallback;
+
+		/**
+		 * Whether there is a match currently.
+		 *
+		 * @readonly
+		 * @member {Boolean}
+		 */
 		this.hasMatch = false;
 
 		/**
@@ -119,40 +145,51 @@ export default class TextWatcher {
 
 		const { text, range } = getLastTextLine( rangeBeforeSelection, model );
 
-		const textHasMatch = this.testCallback( text );
+		const testResult = this.testCallback( text );
 
-		if ( !textHasMatch && this.hasMatch ) {
-			/**
-			 * Fired whenever the text does not match anymore. Fired only when the text watcher found a match.
-			 *
-			 * @event unmatched
-			 */
+		if ( !testResult && this.hasMatch ) {
 			this.fire( 'unmatched' );
 		}
 
-		this.hasMatch = textHasMatch;
+		this.hasMatch = !!testResult;
 
-		if ( textHasMatch ) {
+		if ( testResult ) {
 			const eventData = Object.assign( data, { text, range } );
 
-			/**
-			 * Fired whenever the text watcher found a match for data changes.
-			 *
-			 * @event matched:data
-			 * @param {Object} data Event data.
-			 * @param {String} data.text The full text before selection.
-			 * @param {module:engine/model/batch~Batch} data.batch A batch associated with a change.
-			 */
-			/**
-			 * Fired whenever the text watcher found a match for selection changes.
-			 *
-			 * @event matched:selection
-			 * @param {Object} data Event data.
-			 * @param {String} data.text The full text before selection.
-			 */
+			// If the test callback returns an object with additional data, assign the data as well.
+			if ( typeof testResult == 'object' ) {
+				Object.assign( eventData, testResult );
+			}
+
 			this.fire( `matched:${ suffix }`, eventData );
 		}
 	}
 }
 
 mix( TextWatcher, ObservableMixin );
+
+/**
+ * Fired whenever the text watcher found a match for data changes.
+ *
+ * @event matched:data
+ * @param {Object} data Event data.
+ * @param {String} data.text The full text before selection to which the regexp was applied.
+ * @param {module:engine/model/range~Range} data.range The range representing the position of the `data.text`.
+ * @param {Object} [data.testResult] The additional data returned from the {module:typing/textwatcher~TextWatcher#testCallback}.
+ */
+
+/**
+ * Fired whenever the text watcher found a match for selection changes.
+ *
+ * @event matched:selection
+ * @param {Object} data Event data.
+ * @param {String} data.text The full text before selection.
+ * @param {module:engine/model/range~Range} data.range The range representing the position of the `data.text`.
+ * @param {Object} [data.testResult] The additional data returned from the {module:typing/textwatcher~TextWatcher#testCallback}.
+ */
+
+/**
+ * Fired whenever the text does not match anymore. Fired only when the text watcher found a match.
+ *
+ * @event unmatched
+ */

+ 4 - 4
packages/ckeditor5-typing/tests/manual/texttransformation.md

@@ -17,17 +17,17 @@ Some of the transformations are:
     * Operators: `<=` to `≤`, `>=` to `≥`, `!=` to `≠`.
 
 1. Typography:
-    
+
     * Dashes: ` -- `, ` --- `.
     * Ellipsis: `...` to `…`
-    
+
 1. Quotes:
 
-    * Primary quotes (english): `'Foo bar'` to `‘Foo bar’` 
+    * Primary quotes (english): `'Foo bar'` to `‘Foo bar’`
     * Secondary quotes (english): `"Foo bar's"` to `“Foo bar's”`
 
 ### Testing
 
 * Check if the transformation works. Note that some might need a space to trigger (dashes).
 * Undo a text transformation and type - it should not re-transform it.
-* Change selection - the not transformed elements should stay. 
+* Change selection - the not transformed elements should stay.

+ 28 - 9
packages/ckeditor5-typing/tests/textwatcher.js

@@ -141,17 +141,36 @@ describe( 'TextWatcher', () => {
 	} );
 
 	describe( 'events', () => {
-		it( 'should fire "matched:data" event when test callback returns true for model data changes', () => {
-			testCallbackStub.returns( true );
-
-			model.change( writer => {
-				writer.insertText( '@', doc.selection.getFirstPosition() );
+		describe( '"matched:data"', () => {
+			it( 'should be fired when test callback returns true for model data changes', () => {
+				testCallbackStub.returns( true );
+
+				model.change( writer => {
+					writer.insertText( '@', doc.selection.getFirstPosition() );
+				} );
+
+				sinon.assert.calledOnce( testCallbackStub );
+				sinon.assert.calledOnce( matchedDataSpy );
+				sinon.assert.notCalled( matchedSelectionSpy );
+				sinon.assert.notCalled( unmatchedSpy );
 			} );
 
-			sinon.assert.calledOnce( testCallbackStub );
-			sinon.assert.calledOnce( matchedDataSpy );
-			sinon.assert.notCalled( matchedSelectionSpy );
-			sinon.assert.notCalled( unmatchedSpy );
+			it( 'should be fired with additional data when test callback returns true for model data changes', () => {
+				const additionalData = { abc: 'xyz' };
+
+				testCallbackStub.returns( additionalData );
+
+				model.change( writer => {
+					writer.insertText( '@', doc.selection.getFirstPosition() );
+				} );
+
+				sinon.assert.calledOnce( testCallbackStub );
+				sinon.assert.calledOnce( matchedDataSpy );
+				sinon.assert.notCalled( matchedSelectionSpy );
+				sinon.assert.notCalled( unmatchedSpy );
+
+				expect( matchedDataSpy.firstCall.args[ 1 ] ).to.deep.include( additionalData );
+			} );
 		} );
 
 		it( 'should not fire "matched:data" event when watcher is disabled' +