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 #1194 from ckeditor/t/858
Browse files Browse the repository at this point in the history 
Refactoring: improve batch API. Make it possible to use batch API on detached nodes. Use batch API instead of elements API in view to model conversion.
  • Loading branch information
Piotr Jasiun authored Nov 28, 2017
2 parents 4a55a6c + f183da5 commit 98b984c
Show file tree
Hide file tree
Showing 75 changed files with 3,634 additions and 2,144 deletions.
18 changes: 10 additions & 8 deletions src/controller/datacontroller.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,6 @@ import { convertText, convertToModelFragment } from '../conversion/view-to-model
import ViewDocumentFragment from '../view/documentfragment';

import ModelRange from '../model/range';
import ModelPosition from '../model/position';
import ModelElement from '../model/element';

import insertContent from './insertcontent';
Expand Down Expand Up @@ -196,9 +195,10 @@ export default class DataController {
this.model.selection.clearAttributes();

// Initial batch should be ignored by features like undo, etc.
this.model.batch( 'transparent' )
.remove( ModelRange.createIn( modelRoot ) )
.insert( ModelPosition.createAt( modelRoot, 0 ), this.parse( data ) );
const batch = this.model.batch( 'transparent' );

batch.remove( ModelRange.createIn( modelRoot ) );
batch.insert( this.parse( data, batch ), modelRoot );
} );
}

Expand All @@ -208,16 +208,17 @@ export default class DataController {
*
* @see #set
* @param {String} data Data to parse.
* @param {module:engine/model/batch~Batch} batch Batch to which the deltas will be added.
* @param {String} [context='$root'] Base context in which the view will be converted to the model. See:
* {@link module:engine/conversion/viewconversiondispatcher~ViewConversionDispatcher#convert}.
* @returns {module:engine/model/documentfragment~DocumentFragment} Parsed data.
*/
parse( data, context = '$root' ) {
parse( data, batch, context = '$root' ) {
// data -> view
const viewDocumentFragment = this.processor.toView( data );

// view -> model
return this.toModel( viewDocumentFragment, context );
return this.toModel( viewDocumentFragment, batch, context );
}

/**
Expand All @@ -231,12 +232,13 @@ export default class DataController {
*
* @param {module:engine/view/element~Element|module:engine/view/documentfragment~DocumentFragment} viewElementOrFragment
* Element or document fragment which content will be converted.
* @param {module:engine/model/batch~Batch} batch Batch to which the deltas will be added.
* @param {String} [context='$root'] Base context in which the view will be converted to the model. See:
* {@link module:engine/conversion/viewconversiondispatcher~ViewConversionDispatcher#convert}.
* @returns {module:engine/model/documentfragment~DocumentFragment} Output document fragment.
*/
toModel( viewElementOrFragment, context = '$root' ) {
return this.viewToModel.convert( viewElementOrFragment, { context: [ context ] } );
toModel( viewElementOrFragment, batch, context = '$root' ) {
return this.viewToModel.convert( viewElementOrFragment, batch, { context: [ context ] } );
}

/**
Expand Down
4 changes: 2 additions & 2 deletions src/controller/deletecontent.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -128,7 +128,7 @@ function mergeBranches( batch, startPos, endPos ) {
// <a><b>x[]</b></a><c><d>{}y</d></c>
// becomes:
// <a><b>x</b>[]<d>y</d></a><c>{}</c>
batch.move( endParent, startPos );
batch.insert( endParent, startPos );
}

// Merge two siblings:
Expand Down Expand Up @@ -181,7 +181,7 @@ function checkCanBeMerged( leftPos, rightPos ) {

function insertParagraph( batch, position, selection ) {
const paragraph = new Element( 'paragraph' );
batch.insert( position, paragraph );
batch.insert( paragraph, position );

selection.setCollapsedAt( paragraph );
}
Expand Down
8 changes: 4 additions & 4 deletions src/controller/insertcontent.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -229,7 +229,7 @@ class Insertion {
// If the node is a text and bare text is allowed in current position it means that the node
// contains disallowed attributes and we have to remove them.
else if ( this.schema.check( { name: '$text', inside: this.position } ) ) {
this.schema.removeDisallowedAttributes( [ node ], this.position );
this.schema.removeDisallowedAttributes( [ node ], this.position, this.batch );
this._handleNode( node, context );
}
// If text is not allowed, try autoparagraphing.
Expand All @@ -256,7 +256,7 @@ class Insertion {

const livePos = LivePosition.createFromPosition( this.position );

this.batch.insert( this.position, node );
this.batch.insert( node, this.position );

this.position = Position.createFromPosition( livePos );
livePos.detach();
Expand Down Expand Up @@ -341,7 +341,7 @@ class Insertion {
* @param {Object} context
*/
_tryAutoparagraphing( node, context ) {
const paragraph = new Element( 'paragraph' );
const paragraph = this.batch.createElement( 'paragraph' );

// Do not autoparagraph if the paragraph won't be allowed there,
// cause that would lead to an infinite loop. The paragraph would be rejected in
Expand All @@ -350,7 +350,7 @@ class Insertion {
// When node is a text and is disallowed by schema it means that contains disallowed attributes
// and we need to remove them.
if ( node.is( 'text' ) && !this._checkIsAllowed( node, [ paragraph ] ) ) {
this.schema.removeDisallowedAttributes( [ node ], [ paragraph ] );
this.schema.removeDisallowedAttributes( [ node ], [ paragraph ], this.batch );
}

if ( this._checkIsAllowed( node, [ paragraph ] ) ) {
Expand Down
23 changes: 13 additions & 10 deletions src/conversion/buildviewconverter.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -8,9 +8,6 @@
*/

import Matcher from '../view/matcher';
import ModelElement from '../model/element';
import ModelPosition from '../model/position';
import modelWriter from '../model/writer';
import CKEditorError from '@ckeditor/ckeditor5-utils/src/ckeditorerror';
import isIterable from '@ckeditor/ckeditor5-utils/src/isiterable';

Expand Down Expand Up @@ -266,13 +263,15 @@ class ViewConverterBuilder {
* buildViewConverter().for( dispatcher ).fromElement( 'p' ).toElement( 'paragraph' );
* buildViewConverter().for( dispatcher )
* .fromElement( 'img' )
* .toElement( ( viewElement ) => new ModelElement( 'image', { src: viewElement.getAttribute( 'src' ) } );
* .toElement( ( viewElement, batch ) => batch.createElement( 'image', { src: viewElement.getAttribute( 'src' ) } ) );
*
* @param {String|Function} element Model element name or model element creator function.
*/
toElement( element ) {
function eventCallbackGen( from ) {
return ( evt, data, consumable, conversionApi ) => {
const batch = conversionApi.batch;

// There is one callback for all patterns in the matcher.
// This will be usually just one pattern but we support matchers with many patterns too.
const matchAll = from.matcher.matchAll( data.input );
Expand All @@ -285,7 +284,7 @@ class ViewConverterBuilder {
// Now, for every match between matcher and actual element, we will try to consume the match.
for ( const match of matchAll ) {
// Create model element basing on creator function or element name.
const modelElement = element instanceof Function ? element( data.input ) : new ModelElement( element );
const modelElement = element instanceof Function ? element( data.input, batch ) : batch.createElement( element );

// Do not convert if element building function returned falsy value.
if ( !modelElement ) {
Expand All @@ -310,8 +309,10 @@ class ViewConverterBuilder {

// Convert children of converted view element and append them to `modelElement`.
const modelChildren = conversionApi.convertChildren( data.input, consumable, data );
const insertPosition = ModelPosition.createAt( modelElement, 'end' );
modelWriter.insert( insertPosition, modelChildren );

for ( const child of Array.from( modelChildren ) ) {
batch.append( child, modelElement );
}

// Remove created `modelElement` from the parents stack.
data.context.pop();
Expand Down Expand Up @@ -433,7 +434,9 @@ class ViewConverterBuilder {
*/
toMarker( creator ) {
function eventCallbackGen( from ) {
return ( evt, data, consumable ) => {
return ( evt, data, consumable, conversionApi ) => {
const batch = conversionApi.batch;

// There is one callback for all patterns in the matcher.
// This will be usually just one pattern but we support matchers with many patterns too.
const matchAll = from.matcher.matchAll( data.input );
Expand All @@ -450,7 +453,7 @@ class ViewConverterBuilder {
modelElement = creator( data.input );
// When there is no creator then create model element basing on data from view element.
} else {
modelElement = new ModelElement( '$marker', { 'data-name': data.input.getAttribute( 'data-name' ) } );
modelElement = batch.createElement( '$marker', { 'data-name': data.input.getAttribute( 'data-name' ) } );
}

// Check if model element is correct (has proper name and property).
Expand Down Expand Up @@ -525,7 +528,7 @@ function setAttributeOn( toChange, attribute, data, conversionApi ) {
};

if ( conversionApi.schema.check( schemaQuery ) ) {
toChange.setAttribute( attribute.key, attribute.value );
conversionApi.batch.setAttribute( attribute.key, attribute.value, toChange );
}
}

Expand Down
10 changes: 2 additions & 8 deletions src/conversion/view-to-model-converters.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -3,10 +3,6 @@
* For licensing, see LICENSE.md.
*/

import ModelDocumentFragment from '../model/documentfragment';
import ModelText from '../model/text';
import { normalizeNodes } from '../model/writer';

/**
* Contains {@link module:engine/view/view view} to {@link module:engine/model/model model} converters for
* {@link module:engine/conversion/viewconversiondispatcher~ViewConversionDispatcher}.
Expand All @@ -33,9 +29,7 @@ export function convertToModelFragment() {
return ( evt, data, consumable, conversionApi ) => {
// Second argument in `consumable.consume` is discarded for ViewDocumentFragment but is needed for ViewElement.
if ( !data.output && consumable.consume( data.input, { name: true } ) ) {
const convertedChildren = conversionApi.convertChildren( data.input, consumable, data );

data.output = new ModelDocumentFragment( normalizeNodes( convertedChildren ) );
data.output = conversionApi.convertChildren( data.input, consumable, data );
}
};
}
Expand All @@ -54,7 +48,7 @@ export function convertText() {

if ( conversionApi.schema.check( schemaQuery ) ) {
if ( consumable.consume( data.input ) ) {
data.output = new ModelText( data.input.data );
data.output = conversionApi.batch.createText( data.input.data );
}
}
};
Expand Down
63 changes: 33 additions & 30 deletions src/conversion/viewconversiondispatcher.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -13,11 +13,9 @@ import ModelPosition from '../model/position';
import ModelTreeWalker from '../model/treewalker';
import ModelNode from '../model/node';
import ModelDocumentFragment from '../model/documentfragment';
import { remove } from '../model/writer';

import EmitterMixin from '@ckeditor/ckeditor5-utils/src/emittermixin';
import mix from '@ckeditor/ckeditor5-utils/src/mix';
import extend from '@ckeditor/ckeditor5-utils/src/lib/lodash/extend';
import log from '@ckeditor/ckeditor5-utils/src/log';

/**
Expand Down Expand Up @@ -93,7 +91,7 @@ import log from '@ckeditor/ckeditor5-utils/src/log';
* // Fire conversion.
* // Always take care where the converted model structure will be appended to. If this `viewDocumentFragment`
* // is going to be appended directly to a '$root' element, use that in `context`.
* viewDispatcher.convert( viewDocumentFragment, { context: [ '$root' ] } );
* viewDispatcher.convert( viewDocumentFragment, batch, { context: [ '$root' ] } );
*
* Before each conversion process, `ViewConversionDispatcher` fires {@link ~ViewConversionDispatcher#event:viewCleanup}
* event which can be used to prepare tree view for conversion.
Expand All @@ -118,12 +116,15 @@ export default class ViewConversionDispatcher {
*
* @member {module:engine/conversion/viewconversiondispatcher~ViewConversionApi}
*/
this.conversionApi = extend( {}, conversionApi );
this.conversionApi = Object.assign( {}, conversionApi );

// `convertItem` and `convertChildren` are bound to this `ViewConversionDispatcher` instance and
// set on `conversionApi`. This way only a part of `ViewConversionDispatcher` API is exposed.
this.conversionApi.convertItem = this._convertItem.bind( this );
this.conversionApi.convertChildren = this._convertChildren.bind( this );

// Batch used for conversion. Is passed to #convert method and removed at the and of the conversion.
this.conversionApi.batch = null;
}

/**
Expand All @@ -134,13 +135,17 @@ export default class ViewConversionDispatcher {
* @fires documentFragment
* @param {module:engine/view/documentfragment~DocumentFragment|module:engine/view/element~Element} viewItem
* Part of the view to be converted.
* @param {Object} [additionalData] Additional data to be passed in `data` argument when firing `ViewConversionDispatcher`
* @param {module:engine/model/batch~Batch} batch Batch to which the deltas will be added.
* @param {Object} additionalData Additional data to be passed in `data` argument when firing `ViewConversionDispatcher`
* events. See also {@link ~ViewConversionDispatcher#event:element element event}.
* @returns {module:engine/model/documentfragment~DocumentFragment} Model data that is a result of the conversion process
* wrapped in `DocumentFragment`. Converted marker elements will be set as that document fragment's
* {@link module:engine/model/documentfragment~DocumentFragment#markers static markers map}.
*/
convert( viewItem, additionalData = {} ) {
convert( viewItem, batch, additionalData ) {
// Store batch in current conversion as conversionApi, will be removed at the end of this conversion.
this.conversionApi.batch = batch;

this.fire( 'viewCleanup', viewItem );

const consumable = ViewConsumable.createFrom( viewItem );
Expand All @@ -149,16 +154,19 @@ export default class ViewConversionDispatcher {
// We can get a null here if conversion failed (see _convertItem())
// or simply if an item could not be converted (e.g. due to the schema).
if ( !conversionResult ) {
return new ModelDocumentFragment();
return batch.createDocumentFragment();
}

// When conversion result is not a document fragment we need to wrap it in document fragment.
if ( !conversionResult.is( 'documentFragment' ) ) {
conversionResult = new ModelDocumentFragment( [ conversionResult ] );
const docFrag = batch.createDocumentFragment();

batch.append( conversionResult, docFrag );
conversionResult = docFrag;
}

// Extract temporary markers elements from model and set as static markers collection.
conversionResult.markers = extractMarkersFromModelFragment( conversionResult );
conversionResult.markers = extractMarkersFromModelFragment( conversionResult, batch );

return conversionResult;
}
Expand All @@ -168,7 +176,7 @@ export default class ViewConversionDispatcher {
* @see module:engine/conversion/viewconversiondispatcher~ViewConversionApi#convertItem
*/
_convertItem( input, consumable, additionalData = {} ) {
const data = extend( {}, additionalData, {
const data = Object.assign( {}, additionalData, {
input,
output: null
} );
Expand Down Expand Up @@ -203,24 +211,19 @@ export default class ViewConversionDispatcher {
* @private
* @see module:engine/conversion/viewconversiondispatcher~ViewConversionApi#convertChildren
*/
_convertChildren( input, consumable, additionalData = {} ) {
// Get all children of view input item.
const viewChildren = Array.from( input.getChildren() );

// 1. Map those children to model.
// 2. Filter out items that has not been converted or for which conversion returned wrong result (for those warning is logged).
// 3. Extract children from document fragments to flatten results.
const convertedChildren = viewChildren
.map( viewChild => this._convertItem( viewChild, consumable, additionalData ) )
.filter( converted => converted instanceof ModelNode || converted instanceof ModelDocumentFragment )
.reduce( ( result, filtered ) => {
return result.concat(
filtered.is( 'documentFragment' ) ? Array.from( filtered.getChildren() ) : filtered
);
}, [] );

// Normalize array to model document fragment.
return new ModelDocumentFragment( convertedChildren );
_convertChildren( input, consumable, additionalData ) {
const batch = this.conversionApi.batch;
const documentFragment = batch.createDocumentFragment();

for ( const viewChild of Array.from( input.getChildren() ) ) {
const modelChild = this._convertItem( viewChild, consumable, additionalData );

if ( modelChild instanceof ModelNode || modelChild instanceof ModelDocumentFragment ) {
batch.append( modelChild, documentFragment );
}
}

return documentFragment;
}

/**
Expand Down Expand Up @@ -278,7 +281,7 @@ mix( ViewConversionDispatcher, EmitterMixin );
//
// @param {module:engine/view/documentfragment~DocumentFragment|module:engine/view/node~Node} modelItem Fragment of model.
// @returns {Map<String, module:engine/model/range~Range>} List of static markers.
function extractMarkersFromModelFragment( modelItem ) {
function extractMarkersFromModelFragment( modelItem, batch ) {
const markerElements = new Set();
const markers = new Map();

Expand Down Expand Up @@ -310,7 +313,7 @@ function extractMarkersFromModelFragment( modelItem ) {
}

// Remove marker element from DocumentFragment.
remove( ModelRange.createOn( markerElement ) );
batch.remove( markerElement );
}

return markers;
Expand Down
Loading

0 comments on commit 98b984c

Please sign in to comment.