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

I/6020: Run only one instance of the TextWatcher for all text transformations #223

Merged
merged 20 commits into from
Mar 10, 2020
Merged
Show file tree
Hide file tree
Changes from 12 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
102 changes: 62 additions & 40 deletions src/texttransformation.js
Original file line number Diff line number Diff line change
Expand Up @@ -93,14 +93,20 @@ export default class TextTransformation extends Plugin {
}
} );

this.editor = editor;
/**
* The current editor instance.
*
* @private
* @type {module:core/editor/editor~Editor}
*/
this._editor = editor;
}

/**
* @inheritDoc
*/
init() {
const model = this.editor.model;
const model = this._editor.model;
const modelSelection = model.document.selection;

modelSelection.on( 'change:range', () => {
Expand All @@ -112,60 +118,72 @@ 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 editor = this._editor;
const model = editor.model;
const input = editor.plugins.get( 'Input' );
const normalizedTransformations = normalizeTransformations( editor.config.get( 'typing.transformations' ) );

const testCallback = text => {
for ( const normalizedTransformation of normalizedTransformations ) {
const from = normalizedTransformation.from;
const match = from.test( text );

if ( match ) {
return {
match,
data: {
normalizedTransformation
}
};
}
}
};

const configuredTransformations = getConfiguredTransformations( editor.config.get( 'typing.transformations' ) );

for ( const transformation of configuredTransformations ) {
const from = normalizeFrom( transformation.from );
const to = normalizeTo( transformation.to );
const watcherCallback = ( evt, data ) => {
if ( !input.isInput( data.batch ) ) {
return;
}

const watcher = new TextWatcher( editor.model, text => from.test( text ) );
const { from, to } = data.normalizedTransformation;

const watcherCallback = ( evt, data ) => {
if ( !input.isInput( data.batch ) ) {
return;
}
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;
}
} );
};
const watcher = new TextWatcher( editor.model, testCallback );

watcher.on( 'matched:data', watcherCallback );
watcher.bind( 'isEnabled' ).to( this );
}
watcher.on( 'matched:data', watcherCallback );
watcher.bind( 'isEnabled' ).to( this );
}
}

Expand Down Expand Up @@ -223,8 +241,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 +251,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
12 changes: 7 additions & 5 deletions src/textwatcher.js
Original file line number Diff line number Diff line change
Expand Up @@ -119,9 +119,9 @@ export default class TextWatcher {

const { text, range } = getLastTextLine( rangeBeforeSelection, model );

const textHasMatch = this.testCallback( text );
const textMatcher = this.testCallback( text );

if ( !textHasMatch && this.hasMatch ) {
if ( !textMatcher && this.hasMatch ) {
/**
* Fired whenever the text does not match anymore. Fired only when the text watcher found a match.
*
Expand All @@ -130,10 +130,12 @@ export default class TextWatcher {
this.fire( 'unmatched' );
}

this.hasMatch = textHasMatch;
this.hasMatch = textMatcher && textMatcher.match;

if ( textHasMatch ) {
const eventData = Object.assign( data, { text, range } );
if ( this.hasMatch ) {
// If text matches and testCallback() returns additional data, pass the data to the eventData object.
const additionalData = textMatcher.data;
const eventData = Object.assign( data, { text, range, ...additionalData } );

/**
* Fired whenever the text watcher found a match for data changes.
Expand Down
8 changes: 4 additions & 4 deletions tests/manual/texttransformation.md
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.
47 changes: 32 additions & 15 deletions tests/textwatcher.js
Original file line number Diff line number Diff line change
Expand Up @@ -141,24 +141,41 @@ 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" should fired when test callback returns true for model data changes', () => {
it( 'without additional data', () => {
panr marked this conversation as resolved.
Show resolved Hide resolved
testCallbackStub.returns( { match: 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( 'with additional data', () => {
const additionalData = { abc: 'xyz' };
testCallbackStub.returns( { match: true, data: additionalData } );

model.change( writer => {
writer.insertText( '@', doc.selection.getFirstPosition() );
} );

sinon.assert.calledOnce( testCallbackStub );
sinon.assert.calledOnce( matchedDataSpy );
sinon.assert.notCalled( unmatchedSpy );

expect( matchedDataSpy.firstCall.args[ 1 ] ).to.deep.include( additionalData );
} );
} );

it( 'should not fire "matched:data" event when watcher is disabled' +
' (even when test callback returns true for model data changes)', () => {
watcher.isEnabled = false;

testCallbackStub.returns( true );
testCallbackStub.returns( { match: true } );

model.change( writer => {
writer.insertText( '@', doc.selection.getFirstPosition() );
Expand All @@ -171,7 +188,7 @@ describe( 'TextWatcher', () => {
} );

it( 'should fire "matched:selection" event when test callback returns true for model data changes', () => {
testCallbackStub.returns( true );
testCallbackStub.returns( { match: true } );

model.enqueueChange( 'transparent', writer => {
writer.insertText( '@', doc.selection.getFirstPosition() );
Expand All @@ -191,7 +208,7 @@ describe( 'TextWatcher', () => {
' (even when test callback returns true for model data changes)', () => {
watcher.isEnabled = false;

testCallbackStub.returns( true );
testCallbackStub.returns( { match: true } );

model.enqueueChange( 'transparent', writer => {
writer.insertText( '@', doc.selection.getFirstPosition() );
Expand All @@ -208,7 +225,7 @@ describe( 'TextWatcher', () => {
} );

it( 'should not fire "matched" event when test callback returns false', () => {
testCallbackStub.returns( false );
testCallbackStub.returns( { match: false } );

model.change( writer => {
writer.insertText( '@', doc.selection.getFirstPosition() );
Expand All @@ -221,7 +238,7 @@ describe( 'TextWatcher', () => {
} );

it( 'should fire "unmatched" event when test callback returns false when it was previously matched', () => {
testCallbackStub.returns( true );
testCallbackStub.returns( { match: true } );

model.change( writer => {
writer.insertText( '@', doc.selection.getFirstPosition() );
Expand All @@ -243,7 +260,7 @@ describe( 'TextWatcher', () => {
} );

it( 'should fire "umatched" event when selection is expanded', () => {
testCallbackStub.returns( true );
testCallbackStub.returns( { match: true } );

model.change( writer => {
writer.insertText( '@', doc.selection.getFirstPosition() );
Expand Down