From 6ed926b48c573c0ffd8acfb71d982e9c294c5f39 Mon Sep 17 00:00:00 2001 From: Szymon Cofalik Date: Mon, 8 Jul 2019 18:35:44 +0200 Subject: [PATCH 1/3] Fix: Text transformations will not remove existing formatting. --- src/texttransformation.js | 171 ++++++++++++++++++++++++++---------- src/textwatcher.js | 2 + tests/texttransformation.js | 59 ++++++++++++- 3 files changed, 182 insertions(+), 50 deletions(-) diff --git a/src/texttransformation.js b/src/texttransformation.js index f996c3a..ba306d8 100644 --- a/src/texttransformation.js +++ b/src/texttransformation.js @@ -9,6 +9,7 @@ import Plugin from '@ckeditor/ckeditor5-core/src/plugin'; import TextWatcher from './textwatcher'; +import { escapeRegExp } from 'lodash-es'; // All named transformations. const TRANSFORMATIONS = { @@ -23,8 +24,8 @@ const TRANSFORMATIONS = { twoThirds: { from: '2/3', to: '⅔' }, oneForth: { from: '1/4', to: '¼' }, threeQuarters: { from: '3/4', to: '¾' }, - lessThenOrEqual: { from: '<=', to: '≤' }, - greaterThenOrEqual: { from: '>=', to: '≥' }, + lessThanOrEqual: { from: '<=', to: '≤' }, + greaterThanOrEqual: { from: '>=', to: '≥' }, notEqual: { from: '!=', to: '≠' }, arrowLeft: { from: '<-', to: '←' }, arrowRight: { from: '->', to: '→' }, @@ -36,16 +37,16 @@ const TRANSFORMATIONS = { // Quotations: // English, US - quotesPrimary: { from: buildQuotesRegExp( '"' ), to: '$1“$2”' }, - quotesSecondary: { from: buildQuotesRegExp( '\'' ), to: '$1‘$2’' }, + quotesPrimary: { from: buildQuotesRegExp( '"' ), to: [ null, '“', null, '”' ] }, + quotesSecondary: { from: buildQuotesRegExp( '\'' ), to: [ null, '‘', null, '’' ] }, // English, UK - quotesPrimaryEnGb: { from: buildQuotesRegExp( '\'' ), to: '$1‘$2’' }, - quotesSecondaryEnGb: { from: buildQuotesRegExp( '"' ), to: '$1“$2”' }, + quotesPrimaryEnGb: { from: buildQuotesRegExp( '\'' ), to: [ null, '‘', null, '’' ] }, + quotesSecondaryEnGb: { from: buildQuotesRegExp( '"' ), to: [ null, '“', null, '”' ] }, // Polish - quotesPrimaryPl: { from: buildQuotesRegExp( '"' ), to: '$1„$2”' }, - quotesSecondaryPl: { from: buildQuotesRegExp( '\'' ), to: '$1‚$2’' } + quotesPrimaryPl: { from: buildQuotesRegExp( '"' ), to: [ null, '„', null, '”' ] }, + quotesSecondaryPl: { from: buildQuotesRegExp( '\'' ), to: [ null, '‚', null, '’' ] } }; // Transformation groups. @@ -53,7 +54,7 @@ const TRANSFORMATION_GROUPS = { symbols: [ 'copyright', 'registeredTrademark', 'trademark' ], mathematical: [ 'oneHalf', 'oneThird', 'twoThirds', 'oneForth', 'threeQuarters', - 'lessThenOrEqual', 'greaterThenOrEqual', 'notEqual', + 'lessThanOrEqual', 'greaterThanOrEqual', 'notEqual', 'arrowLeft', 'arrowRight' ], typography: [ 'horizontalEllipsis', 'enDash', 'emDash' ], @@ -104,43 +105,93 @@ export default class TextTransformation extends Plugin { const configuredTransformations = getConfiguredTransformations( editor.config.get( 'typing.transformations' ) ); for ( const transformation of configuredTransformations ) { - const { from, to } = transformation; + const from = normalizeFrom( transformation.from ); + const to = normalizeTo( transformation.to ); - let testCallback; - let textReplacer; + const watcher = new TextWatcher( editor.model, text => from.test( text ) ); - if ( from instanceof RegExp ) { - testCallback = text => from.test( text ); - textReplacer = message => message.replace( from, to ); - } else { - testCallback = text => text.endsWith( from ); - textReplacer = message => message.slice( 0, message.length - from.length ) + to; - } + watcher.on( 'matched:data', ( evt, data ) => { + const matches = from.exec( data.text ); + const replaces = to( matches.slice( 1 ) ); - const watcher = new TextWatcher( editor.model, testCallback ); + // Used `focus` to be in line with `TextWatcher#_getText()`. + const selectionParent = editor.model.document.selection.focus.parent; - watcher.on( 'matched:data', ( evt, data ) => { - const selection = editor.model.document.selection; - const focus = selection.focus; - const textToReplaceLength = data.text.length; - const textToInsert = textReplacer( data.text ); + let changeIndex = matches.index; + + 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; - model.enqueueChange( model.createBatch(), writer => { - const replaceRange = writer.createRange( focus.getShiftedBy( -textToReplaceLength ), focus ); + continue; + } - model.insertContent( writer.createText( textToInsert, selection.getAttributes() ), replaceRange ); + const replacePosition = model.createPositionAt( selectionParent, changeIndex ); + const replaceRange = model.createRange( replacePosition, replacePosition.getShiftedBy( match.length ) ); + const attributes = getTextAttributesAfterPosition( replacePosition ); + + model.insertContent( writer.createText( replaceWith, attributes ), replaceRange ); + + changeIndex += replaceWith.length; + } } ); } ); } } } +// Normalizes config `from` parameter value. +// The normalized value for `from` parameter is a RegExp instance. If passed `from` is already a RegExp instance it is returned unchanged. +// +// @param {String|RegExp} from +// @returns {RegExp} +function normalizeFrom( from ) { + if ( typeof from == 'string' ) { + return new RegExp( '(' + escapeRegExp( from ) + ')$' ); + } + + // `from` is already a regular expression. + return from; +} + +// Normalizes config `to` parameter value. +// The normalized value for `to` parameter is a function that takes an array and returns an array. See more in configuration description. +// If passed `to` is already a function it is returned unchanged. +// +// @param {String|Array.|Function} to +// @returns {Function} +function normalizeTo( to ) { + if ( typeof to == 'string' ) { + return () => [ to ]; + } else if ( to instanceof Array ) { + return () => to; + } + + // `to` is already a function. + return to; +} + +// For given `position` returns attributes for the text that is after that position. +// The text can be in the same text node as the position (`foo[]bar`) or in the next text node (`foo[]<$text bold="true">bar`). +// +// @param {module:engine/model/position~Position} position +// @returns {Iterable.<*>} +function getTextAttributesAfterPosition( position ) { + const textNode = position.textNode ? position.textNode : position.nodeAfter; + + return textNode.getAttributes(); +} + // Returns a RegExp pattern string that detects a sentence inside a quote. // -// @param {String} quoteCharacter a character to creat a pattern for. +// @param {String} quoteCharacter The character to create a pattern for. // @returns {String} function buildQuotesRegExp( quoteCharacter ) { - return new RegExp( `(^|\\s)${ quoteCharacter }([^${ quoteCharacter }]+)${ quoteCharacter }$` ); + return new RegExp( `(^|\\s)(${ quoteCharacter })([^${ quoteCharacter }]*)(${ quoteCharacter })$` ); } // Reads text transformation config and returns normalized array of transformations objects. @@ -182,20 +233,42 @@ function expandGroupsAndRemoveDuplicates( definitions ) { } /** - * Text transformation definition object. + * Text transformation definition object. Describes what should be replace with what. + * + * Input value ("replace from") can be passed either as a `String` or a `RegExp` instance. + * + * * If `String` is passed it will be simply checked if the end of the input matches it. + * * If `RegExp` is passed, all parts of it's parts have to be inside capturing groups. Also, since it is compared against the end of + * an input, it has to include `$` token to be correctly matched. See examples below. + * + * Output value ("replace to") can be passed either as a `String` or an `Array` or a `Function`. * - * const transformations = [ - * // Will replace foo with bar: - * { from: 'foo', to: 'bar' }, + * * If a `String` is passed, it will be used as a replacement value as-is. Note, that a `String` output value can be used only if + * the input value is a `String` too. + * * If an `Array` is passed it has to have the same number of elements as there are capturing groups in the input value `RegExp`. + * Each capture group will be replaced by a corresponding `String` from the passed `Array`. If given capturing group should not be replaced, + * use `null` instead of passing a `String`. + * * If a `Function` is used, it should return an array as described above. The function is passed one parameter - an array with matches + * for the input value `RegExp`. See examples below. * + * Simple string-to-string replacement: + * + * { from: '(c)', to: '©' } + * + * Change quotes styles using regular expression. Note how all the parts are in separate capturing groups and the space at the beginning and + * the text inside quotes are not replaced (`null` passed as the first and the third value in `to` parameter): + * + * { + * from: /(\\s)(")([^"]*)(")$/, + * to: [ null, '“', null, '”' ] + * } * - * // The following (naive) rule will remove @ from emails. - * // For example, user@example.com will become user.at.example.com. - * { from: /([a-z-]+)@([a-z]+\.[a-z]{2,})$/i, to: '$1.at.$2' } - * ] + * Automatic uppercase after a dot using a callback: * - * **Note:** The text watcher always evaluates the end of the input (the typed text). If you're passing a RegExp object you must - * include `$` token to match the end of string. + * { + * from: /(\. )([a-z])$/, + * to: matches => [ null, matches[ 1 ].toUpperCase() ] + * } * * @typedef {Object} module:typing/texttransformation~TextTransformationDescription * @property {String|RegExp} from The string or RegExp to transform. @@ -243,8 +316,8 @@ function expandGroupsAndRemoveDuplicates( definitions ) { * - `twoThirds`: transforms `2/3`, to: `⅔` * - `oneForth`: transforms `1/4`, to: `¼` * - `threeQuarters`: transforms `3/4`, to: `¾` - * - `lessThenOrEqual`: transforms `<=`, to: `≤` - * - `greaterThenOrEqual`: transforms `>=`, to: `≥` + * - `lessThanOrEqual`: transforms `<=`, to: `≤` + * - `greaterThanOrEqual`: transforms `>=`, to: `≥` * - `notEqual`: transforms `!=`, to: `≠` * - `arrowLeft`: transforms `<-`, to: `←` * - `arrowRight`: transforms `->`, to: `→` @@ -263,21 +336,23 @@ function expandGroupsAndRemoveDuplicates( definitions ) { * In order to completely override the supported transformations, use the * {@link module:typing/texttransformation~TextTransformationConfig#include `transformations.include` option}. * - * Example: + * Examples: * * const transformationsConfig = { * include: [ * // Use only the 'quotes' and 'typography' groups. * 'quotes', * 'typography', - + * * // Plus, some custom transformation. - * { from: 'CKE', to: 'CKEditor } - * ], + * { from: 'CKE', to: 'CKEditor' } + * ] + * }; * + * const transformationsConfig = { * // Remove the 'ellipsis' transformation loaded by the 'typography' group. * remove: [ 'ellipsis' ] - * }; + * } * * @interface TextTransformationConfig */ @@ -313,7 +388,7 @@ function expandGroupsAndRemoveDuplicates( definitions ) { * * const transformationsConfig = { * remove: [ - * 'ellipsis', // Remove only 'ellipsis' from 'typography group. + * 'ellipsis', // Remove only 'ellipsis' from 'typography' group. * 'mathematical' // Remove all transformations from 'mathematical' group. * ] * } diff --git a/src/textwatcher.js b/src/textwatcher.js index b650641..5bb4e03 100644 --- a/src/textwatcher.js +++ b/src/textwatcher.js @@ -78,6 +78,7 @@ export default class TextWatcher { * @fires unmatched * * @private + * @param {'data'|'selection'} suffix Suffix used for generating event name. */ _evaluateTextBeforeSelection( suffix ) { const text = this._getText(); @@ -101,6 +102,7 @@ export default class TextWatcher { * * @event matched:data */ + /** * Fired whenever the text watcher found a match for selection changes. * diff --git a/tests/texttransformation.js b/tests/texttransformation.js index d697f36..cfb4e3a 100644 --- a/tests/texttransformation.js +++ b/tests/texttransformation.js @@ -9,6 +9,7 @@ import global from '@ckeditor/ckeditor5-utils/src/dom/global'; import TextTransformation from '../src/texttransformation'; import { getData, setData } from '@ckeditor/ckeditor5-engine/src/dev-utils/model'; import Paragraph from '@ckeditor/ckeditor5-paragraph/src/paragraph'; +import Bold from '@ckeditor/ckeditor5-basic-styles/src/bold'; describe( 'Text transformation feature', () => { let editorElement, editor, model, doc; @@ -83,6 +84,40 @@ describe( 'Text transformation feature', () => { } ); } ); + // https://github.com/ckeditor/ckeditor5-typing/issues/203. + it( 'should replace only the parts of content which changed', () => { + setData( model, 'Foo "<$text bold="true">Bar[]' ); + + model.change( writer => { + writer.insertText( '"', doc.selection.focus ); + } ); + + expect( getData( model, { withoutSelection: true } ) ) + .to.equal( 'Foo “<$text bold="true">Bar”' ); + } ); + + it( 'should keep styles of the replaced text #1', () => { + setData( model, 'Foo <$text bold="true">"Bar[]' ); + + model.change( writer => { + writer.insertText( '"', { bold: true }, doc.selection.focus ); + } ); + + expect( getData( model, { withoutSelection: true } ) ) + .to.equal( 'Foo <$text bold="true">“Bar<$text bold="true">”' ); + } ); + + it( 'should keep styles of the replaced text #2', () => { + setData( model, 'F<$text bold="true">oo "Bar[]' ); + + model.change( writer => { + writer.insertText( '"', doc.selection.focus ); + } ); + + expect( getData( model, { withoutSelection: true } ) ) + .to.equal( 'F<$text bold="true">oo “Bar”' ); + } ); + function testTransformation( transformFrom, transformTo ) { it( `should transform "${ transformFrom }" to "${ transformTo }"`, () => { setData( model, 'A foo[]' ); @@ -142,7 +177,7 @@ describe( 'Text transformation feature', () => { typing: { transformations: { extra: [ - { from: /([a-z]+)@(example.com)$/, to: '$1.at.$2' } + { from: /([a-z]+)(@)(example.com)$/, to: [ null, '.at.', null ] } ] } } @@ -157,6 +192,26 @@ describe( 'Text transformation feature', () => { } ); } ); + it( 'should allow adding own rules with function as `to` value', () => { + return createEditorInstance( { + typing: { + transformations: { + extra: [ + { from: /(\. )([a-z])$/, to: matches => [ null, matches[ 1 ].toUpperCase() ] } + ] + } + } + } ).then( () => { + setData( model, 'Foo. []' ); + + model.enqueueChange( model.createBatch(), writer => { + writer.insertText( 'b', doc.selection.focus ); + } ); + + expect( getData( model, { withoutSelection: true } ) ).to.equal( 'Foo. B' ); + } ); + } ); + it( 'should not alter include rules adding own rules as extra', () => { return createEditorInstance( { typing: { @@ -291,7 +346,7 @@ describe( 'Text transformation feature', () => { function createEditorInstance( additionalConfig = {} ) { return ClassicTestEditor .create( editorElement, Object.assign( { - plugins: [ Paragraph, TextTransformation ] + plugins: [ Paragraph, Bold, TextTransformation ] }, additionalConfig ) ) .then( newEditor => { editor = newEditor; From d6a4f58569a09adae1d03f8c6f0be7f3bed92a49 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Piotrek=20Koszuli=C5=84ski?= Date: Tue, 9 Jul 2019 09:24:45 +0200 Subject: [PATCH 2/3] Added missing dependency. --- package.json | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/package.json b/package.json index c3d1912..93c5286 100644 --- a/package.json +++ b/package.json @@ -12,7 +12,8 @@ "dependencies": { "@ckeditor/ckeditor5-core": "^12.2.0", "@ckeditor/ckeditor5-engine": "^13.2.0", - "@ckeditor/ckeditor5-utils": "^13.0.0" + "@ckeditor/ckeditor5-utils": "^13.0.0", + "lodash-es": "^4.17.10" }, "devDependencies": { "@ckeditor/ckeditor5-basic-styles": "^11.1.2", From 2b9ad49b4035b6b2d8f92cc74443c70caf190a58 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Piotrek=20Koszuli=C5=84ski?= Date: Tue, 9 Jul 2019 12:44:42 +0200 Subject: [PATCH 3/3] Corrected tones of our mistakes :D. --- .../features/text-transformation-extended.js | 22 +++++++++---- docs/features/text-transformation.md | 26 +++++++++++----- src/texttransformation.js | 31 ++++++++++--------- 3 files changed, 51 insertions(+), 28 deletions(-) diff --git a/docs/_snippets/features/text-transformation-extended.js b/docs/_snippets/features/text-transformation-extended.js index f6e9eb5..8d24ff6 100644 --- a/docs/_snippets/features/text-transformation-extended.js +++ b/docs/_snippets/features/text-transformation-extended.js @@ -17,9 +17,9 @@ ClassicEditor transformations: { remove: [ // Don't use the transformations from the - // 'symbols' and 'mathematical' groups. + // 'symbols' and 'quotes' groups. 'symbols', - 'mathematical', + 'quotes', // As well as the following transformations. 'arrowLeft', @@ -33,10 +33,20 @@ ClassicEditor { from: ':tada:', to: '🎉' }, // You can also define patterns using regexp. - // Note: the pattern must end with `$`. - // The following (naive) rule will remove @ from emails. - // For example, user@example.com will become user.at.example.com. - { from: /([a-z-]+)@([a-z]+\.[a-z]{2,})$/i, to: '$1.at.$2' } + // Note: the pattern must end with `$` and all its fragments must be wrapped + // with capturing groups. + // The following rule replaces ` "foo"` with ` «foo»`. + { + from: /(^|\s)(")([^"]*)(")$/, + to: [ null, '«', null, '»' ] + }, + + // Finally, you can define `to` as a callback. + // This (naive) rule will auto-capitalize first word after a period. + { + from: /(\. )([a-z])$/, + to: matches => [ null, matches[ 1 ].toUpperCase() ] + } ], } } diff --git a/docs/features/text-transformation.md b/docs/features/text-transformation.md index f521467..be2356c 100644 --- a/docs/features/text-transformation.md +++ b/docs/features/text-transformation.md @@ -101,9 +101,9 @@ ClassicEditor transformations: { remove: [ // Don't use the transformations from the - // 'symbols' and 'mathematical' groups. + // 'symbols' and 'quotes' groups. 'symbols', - 'mathematical', + 'quotes', // As well as the following transformations. 'arrowLeft', @@ -117,10 +117,20 @@ ClassicEditor { from: ':tada:', to: '🎉' }, // You can also define patterns using regexp. - // Note: the pattern must end with `$`. - // The following (naive) rule will remove @ from emails. - // For example, user@example.com will become user.at.example.com. - { from: /([a-z-]+)@([a-z]+\.[a-z]{2,})$/i, to: '$1.at.$2' } + // Note: the pattern must end with `$` and all its fragments must be wrapped + // with capturing groups. + // The following rule replaces ` "foo"` with ` «foo»`. + { + from: /(^|\s)(")([^"]*)(")$/, + to: [ null, '«', null, '»' ] + }, + + // Finally, you can define `to` as a callback. + // This (naive) rule will auto-capitalize first word after a period. + { + from: /(\. )([a-z])$/, + to: matches => [ null, matches[ 1 ].toUpperCase() ] + } ], } } @@ -129,7 +139,9 @@ ClassicEditor .catch( ... ); ``` -You can test the above configuration here: +You can read more about the format of transformation rules in {@link module:typing/texttransformation~TextTransformationDescription}. + +Test the rules defined above: {@snippet features/text-transformation-extended} diff --git a/src/texttransformation.js b/src/texttransformation.js index ba306d8..f73ef5a 100644 --- a/src/texttransformation.js +++ b/src/texttransformation.js @@ -233,23 +233,24 @@ function expandGroupsAndRemoveDuplicates( definitions ) { } /** - * Text transformation definition object. Describes what should be replace with what. + * Text transformation definition object. Describes what should be replaced with what. * - * Input value ("replace from") can be passed either as a `String` or a `RegExp` instance. + * The input value (`from`) can be passed either as a string or a regexp. * - * * If `String` is passed it will be simply checked if the end of the input matches it. - * * If `RegExp` is passed, all parts of it's parts have to be inside capturing groups. Also, since it is compared against the end of - * an input, it has to include `$` token to be correctly matched. See examples below. + * * If a string is passed it will be simply checked if the end of the input matches it. + * * If a regexp is passed, its entire length must be covered with capturing groups (e.g. `/(foo)(bar)$/`). + * Also, since it is compared against the end of the input, it has to end with `$` to be correctly matched. + * See examples below. * - * Output value ("replace to") can be passed either as a `String` or an `Array` or a `Function`. + * The output value (`to`) can be passed either as a string or an array or a function. * - * * If a `String` is passed, it will be used as a replacement value as-is. Note, that a `String` output value can be used only if - * the input value is a `String` too. - * * If an `Array` is passed it has to have the same number of elements as there are capturing groups in the input value `RegExp`. - * Each capture group will be replaced by a corresponding `String` from the passed `Array`. If given capturing group should not be replaced, - * use `null` instead of passing a `String`. - * * If a `Function` is used, it should return an array as described above. The function is passed one parameter - an array with matches - * for the input value `RegExp`. See examples below. + * * If a string is passed, it will be used as a replacement value as-is. Note, that a string output value can be used only if + * the input value is a string too. + * * If an array is passed it has to have the same number of elements as there are capturing groups in the input value regexp. + * Each capture group will be replaced by a corresponding string from the passed array. If given capturing group should not be replaced, + * use `null` instead of passing a string. + * * If a function is used, it should return an array as described above. The function is passed one parameter — an array with matches + * by the regexp. See the examples below. * * Simple string-to-string replacement: * @@ -259,7 +260,7 @@ function expandGroupsAndRemoveDuplicates( definitions ) { * the text inside quotes are not replaced (`null` passed as the first and the third value in `to` parameter): * * { - * from: /(\\s)(")([^"]*)(")$/, + * from: /(^|\s)(")([^"]*)(")$/, * to: [ null, '“', null, '”' ] * } * @@ -303,7 +304,7 @@ function expandGroupsAndRemoveDuplicates( definitions ) { * - `ellipsis`: transforms `...` to `…` * - `enDash`: transforms ` -- ` to ` – ` * - `emDash`: transforms ` --- ` to ` — ` - * * Quotations (group name: `quotations`) + * * Quotations (group name: `quotes`) * - `quotesPrimary`: transforms `"Foo bar"` to `“Foo bar”` * - `quotesSecondary`: transforms `'Foo bar'` to `‘Foo bar’` * * Symbols (group name: `symbols`)