Skip to content
This repository has been archived by the owner on Jun 26, 2020. It is now read-only.

Commit

Permalink
Merge pull request #223 from ckeditor/i/6020
Browse files Browse the repository at this point in the history 
Other: Run only one instance of the `TextWatcher` for all text transformations. Closes ckeditor/ckeditor5#6020.
  • Loading branch information
@Reinmar
Reinmar authored Mar 10, 2020
2 parents 64daf31 + b713427 commit 550426d
Show file tree
Hide file tree
Showing 4 changed files with 141 additions and 76 deletions.
85 changes: 47 additions & 38 deletions src/texttransformation.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -92,8 +92,6 @@ export default class TextTransformation extends Plugin {
include: DEFAULT_TRANSFORMATIONS
}
} );

this.editor = editor;
}

/**
Expand All @@ -112,60 +110,67 @@ 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
*/
_enableTransformationWatchers() {
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 );
}
}

Expand Down Expand Up @@ -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 );
Expand All @@ -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.
Expand Down
87 changes: 62 additions & 25 deletions src/textwatcher.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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;

/**
Expand Down Expand Up @@ -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
*/
8 changes: 4 additions & 4 deletions tests/manual/texttransformation.md
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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.
37 changes: 28 additions & 9 deletions tests/textwatcher.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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' +
Expand Down

0 comments on commit 550426d

Please sign in to comment.