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

Commit

Permalink
Make Position.createAt() offset parameter required.
Browse files Browse the repository at this point in the history
  • Loading branch information
jodator committed Oct 4, 2018
1 parent 3cc4834 commit 7999456
Show file tree
Hide file tree
Showing 20 changed files with 61 additions and 52 deletions.
4 changes: 2 additions & 2 deletions src/controller/datacontroller.js
Original file line number Diff line number Diff line change
Expand Up @@ -201,7 +201,7 @@ export default class DataController {
const modelRoot = this.model.document.getRoot( rootName );

this.model.enqueueChange( 'transparent', writer => {
writer.insert( this.parse( data, modelRoot ), modelRoot );
writer.insert( this.parse( data, modelRoot ), modelRoot, 0 );
} );

return Promise.resolve();
Expand All @@ -228,7 +228,7 @@ export default class DataController {
writer.removeSelectionAttribute( this.model.document.selection.getAttributeKeys() );

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

Expand Down
4 changes: 2 additions & 2 deletions src/conversion/upcast-converters.js
Original file line number Diff line number Diff line change
Expand Up @@ -358,7 +358,7 @@ function _prepareToElementConverter( config ) {
conversionApi.writer.insert( modelElement, splitResult.position );

// Convert children and insert to element.
const childrenResult = conversionApi.convertChildren( data.viewItem, ModelPosition.createAt( modelElement ) );
const childrenResult = conversionApi.convertChildren( data.viewItem, ModelPosition.createAt( modelElement, 0 ) );

// Consume appropriate value from consumable values list.
conversionApi.consumable.consume( data.viewItem, match.match );
Expand All @@ -380,7 +380,7 @@ function _prepareToElementConverter( config ) {
// before: <allowed><notAllowed>[]</notAllowed></allowed>
// after: <allowed><notAllowed></notAllowed><converted></converted><notAllowed>[]</notAllowed></allowed>
if ( splitResult.cursorParent ) {
data.modelCursor = ModelPosition.createAt( splitResult.cursorParent );
data.modelCursor = ModelPosition.createAt( splitResult.cursorParent, 0 );

// Otherwise just continue after inserted element.
} else {
Expand Down
4 changes: 2 additions & 2 deletions src/conversion/upcastdispatcher.js
Original file line number Diff line number Diff line change
Expand Up @@ -71,7 +71,7 @@ import mix from '@ckeditor/ckeditor5-utils/src/mix';
* conversionApi.writer.insert( paragraph, splitResult.position );
*
* // Convert children to paragraph.
* const { modelRange } = conversionApi.convertChildren( data.viewItem, Position.createAt( paragraph ) );
* const { modelRange } = conversionApi.convertChildren( data.viewItem, Position.createAt( paragraph, 0 ) );
*
* // Set as conversion result, attribute converters may use this property.
* data.modelRange = new Range( Position.createBefore( paragraph ), modelRange.end );
Expand Down Expand Up @@ -419,7 +419,7 @@ function createContextTree( contextDefinition, writer ) {
writer.append( current, position );
}

position = ModelPosition.createAt( current );
position = ModelPosition.createAt( current, 0 );
}

return position;
Expand Down
2 changes: 1 addition & 1 deletion src/dev-utils/model.js
Original file line number Diff line number Diff line change
Expand Up @@ -391,7 +391,7 @@ function convertToModelElement() {

conversionApi.mapper.bindElements( element, data.viewItem );

conversionApi.convertChildren( data.viewItem, ModelPosition.createAt( element ) );
conversionApi.convertChildren( data.viewItem, ModelPosition.createAt( element, 0 ) );

data.modelRange = ModelRange.createOn( element );
data.modelCursor = data.modelRange.end;
Expand Down
8 changes: 5 additions & 3 deletions src/model/position.js
Original file line number Diff line number Diff line change
Expand Up @@ -816,7 +816,7 @@ export default class Position {
* * {@link module:engine/model/position~Position.createFromPosition}.
*
* @param {module:engine/model/item~Item|module:engine/model/position~Position} itemOrPosition
* @param {Number|'end'|'before'|'after'} [offset=0] Offset or one of the flags. Used only when
* @param {Number|'end'|'before'|'after'} offset Offset or one of the flags. Used only when
* first parameter is a {@link module:engine/model/item~Item model item}.
*/
static createAt( itemOrPosition, offset ) {
Expand All @@ -831,8 +831,10 @@ export default class Position {
return this.createBefore( node );
} else if ( offset == 'after' ) {
return this.createAfter( node );
} else if ( !offset ) {
offset = 0;
} else if ( offset !== 0 && !offset ) {
throw new CKEditorError(
'model-position-createAt-required-second-parameter: ' +
'Position.createAt requires the second parameter offset.' );
}

return this.createFromParentAndOffset( node, offset );
Expand Down
2 changes: 1 addition & 1 deletion src/model/schema.js
Original file line number Diff line number Diff line change
Expand Up @@ -1162,7 +1162,7 @@ export class SchemaContext {
* schema.checkChild( contextDefinition, childToCheck );
*
* // Also check in [ rootElement, blockQuoteElement, paragraphElement ].
* schema.checkChild( Position.createAt( paragraphElement ), 'foo' );
* schema.checkChild( Position.createAt( paragraphElement, 0 ), 'foo' );
*
* // Check in [ rootElement, paragraphElement ].
* schema.checkChild( [ rootElement, paragraphElement ], 'foo' );
Expand Down
4 changes: 2 additions & 2 deletions src/model/selection.js
Original file line number Diff line number Diff line change
Expand Up @@ -665,7 +665,7 @@ export default class Selection {
const endBlock = getParentBlock( range.end, visited );

// #984. Don't return the end block if the range ends right at its beginning.
if ( endBlock && !range.end.isTouching( Position.createAt( endBlock ) ) ) {
if ( endBlock && !range.end.isTouching( Position.createAt( endBlock, 0 ) ) ) {
yield endBlock;
}
}
Expand All @@ -683,7 +683,7 @@ export default class Selection {
* @returns {Boolean}
*/
containsEntireContent( element = this.anchor.root ) {
const limitStartPosition = Position.createAt( element );
const limitStartPosition = Position.createAt( element, 0 );
const limitEndPosition = Position.createAt( element, 'end' );

return limitStartPosition.isTouching( this.getFirstPosition() ) &&
Expand Down
2 changes: 1 addition & 1 deletion src/model/utils/deletecontent.js
Original file line number Diff line number Diff line change
Expand Up @@ -210,7 +210,7 @@ function replaceEntireContentWithParagraph( writer, selection ) {
const limitElement = writer.model.schema.getLimitElement( selection );

writer.remove( Range.createIn( limitElement ) );
insertParagraph( writer, Position.createAt( limitElement ), selection );
insertParagraph( writer, Position.createAt( limitElement, 0 ), selection );
}

// We want to replace the entire content with a paragraph when:
Expand Down
2 changes: 1 addition & 1 deletion src/model/utils/getselectedcontent.js
Original file line number Diff line number Diff line change
Expand Up @@ -99,7 +99,7 @@ export default function getSelectedContent( model, selection ) {
// Find the position of the original range in the cloned fragment.
const newRange = range._getTransformedByMove( flatSubtreeRange.start, Position.createAt( frag, 0 ), howMany )[ 0 ];

const leftExcessRange = new Range( Position.createAt( frag ), newRange.start );
const leftExcessRange = new Range( Position.createAt( frag, 0 ), newRange.start );
const rightExcessRange = new Range( newRange.end, Position.createAt( frag, 'end' ) );

removeRangeContent( rightExcessRange, writer );
Expand Down
7 changes: 5 additions & 2 deletions src/model/utils/selection-post-fixer.js
Original file line number Diff line number Diff line change
Expand Up @@ -199,8 +199,11 @@ function tryFixingNonCollapsedRage( range, schema ) {
if ( isStartInLimit || isEndInLimit ) {
// Although we've already found limit element on start/end positions we must find the outer-most limit element.
// as limit elements might be nested directly inside (ie table > tableRow > tableCell).
const fixedStart = isStartInLimit ? expandSelectionOnIsLimitNode( Position.createAt( startLimitElement ), schema, 'start' ) : start;
const fixedEnd = isEndInLimit ? expandSelectionOnIsLimitNode( Position.createAt( endLimitElement ), schema, 'end' ) : end;
const startPosition = Position.createAt( startLimitElement, 0 );
const endPosition = Position.createAt( endLimitElement, 0 );

const fixedStart = isStartInLimit ? expandSelectionOnIsLimitNode( startPosition, schema, 'start' ) : start;
const fixedEnd = isEndInLimit ? expandSelectionOnIsLimitNode( endPosition, schema, 'end' ) : end;

return new Range( fixedStart, fixedEnd );
}
Expand Down
8 changes: 4 additions & 4 deletions src/model/writer.js
Original file line number Diff line number Diff line change
Expand Up @@ -156,7 +156,7 @@ export default class Writer {
* @param {Number|'end'|'before'|'after'} [offset=0] Offset or one of the flags. Used only when
* second parameter is a {@link module:engine/model/item~Item model item}.
*/
insert( item, itemOrPosition, offset ) {
insert( item, itemOrPosition, offset = 0 ) {
this._assertWriterUsedCorrectly();

const position = Position.createAt( itemOrPosition, offset );
Expand Down Expand Up @@ -198,7 +198,7 @@ export default class Writer {
if ( item instanceof DocumentFragment ) {
for ( const [ markerName, markerRange ] of item.markers ) {
// We need to migrate marker range from DocumentFragment to Document.
const rangeRootPosition = Position.createAt( markerRange.root );
const rangeRootPosition = Position.createAt( markerRange.root, 0 );
const range = new Range(
markerRange.start._getCombined( rangeRootPosition, position ),
markerRange.end._getCombined( rangeRootPosition, position )
Expand Down Expand Up @@ -668,7 +668,7 @@ export default class Writer {

return {
position,
range: new Range( Position.createAt( firstSplitElement, 'end' ), Position.createAt( firstCopyElement ) )
range: new Range( Position.createAt( firstSplitElement, 'end' ), Position.createAt( firstCopyElement, 0 ) )
};
}

Expand Down Expand Up @@ -1043,7 +1043,7 @@ export default class Writer {
* {@link module:engine/model/position~Position.createAt `Position.createAt()`} parameters.
*
* @param {module:engine/model/item~Item|module:engine/model/position~Position} itemOrPosition
* @param {Number|'end'|'before'|'after'} [offset=0] Offset or one of the flags. Used only when
* @param {Number|'end'|'before'|'after'} [of~fset=0] Offset or one of the flags. Used only when
* first parameter is a {@link module:engine/model/item~Item model item}.
*/
setSelectionFocus( itemOrPosition, offset ) {
Expand Down
2 changes: 1 addition & 1 deletion src/view/downcastwriter.js
Original file line number Diff line number Diff line change
Expand Up @@ -915,7 +915,7 @@ export default class DowncastWriter {
const newElement = new ContainerElement( newName, viewElement.getAttributes() );

this.insert( Position.createAfter( viewElement ), newElement );
this.move( Range.createIn( viewElement ), Position.createAt( newElement ) );
this.move( Range.createIn( viewElement ), Position.createAt( newElement, 0 ) );
this.remove( Range.createOn( viewElement ) );

return newElement;
Expand Down
4 changes: 2 additions & 2 deletions tests/conversion/upcast-converters.js
Original file line number Diff line number Diff line change
Expand Up @@ -841,7 +841,7 @@ describe( 'upcast-converters', () => {
const paragraph = conversionApi.writer.createElement( 'paragraph' );

conversionApi.writer.insert( paragraph, data.modelCursor );
conversionApi.convertChildren( data.viewItem, ModelPosition.createAt( paragraph ) );
conversionApi.convertChildren( data.viewItem, ModelPosition.createAt( paragraph, 0 ) );

data.modelRange = ModelRange.createOn( paragraph );
data.modelCursor = data.modelRange.end;
Expand All @@ -863,7 +863,7 @@ describe( 'upcast-converters', () => {
new ViewContainerElement( 'div', null, [ new ViewText( 'abc' ), new ViewContainerElement( 'foo' ) ] ),
new ViewContainerElement( 'bar' )
] );
const position = ModelPosition.createAt( new ModelElement( 'element' ) );
const position = ModelPosition.createAt( new ModelElement( 'element' ), 0 );

dispatcher.on( 'documentFragment', convertToModelFragment() );
dispatcher.on( 'element', convertToModelFragment(), { priority: 'lowest' } );
Expand Down
8 changes: 4 additions & 4 deletions tests/conversion/upcastdispatcher.js
Original file line number Diff line number Diff line change
Expand Up @@ -501,7 +501,7 @@ describe( 'UpcastDispatcher', () => {
dispatcher.on( 'documentFragment', ( evt, data, conversionApi ) => {
spy();

const result = conversionApi.convertChildren( data.viewItem, ModelPosition.createAt( rootMock ) );
const result = conversionApi.convertChildren( data.viewItem, ModelPosition.createAt( rootMock, 0 ) );

expect( result.modelRange ).to.be.instanceof( ModelRange );
expect( result.modelRange.start.path ).to.deep.equal( [ 0 ] );
Expand Down Expand Up @@ -540,7 +540,7 @@ describe( 'UpcastDispatcher', () => {
dispatcher.on( 'documentFragment', ( evt, data, conversionApi ) => {
const paragraph = conversionApi.writer.createElement( 'paragraph' );
const span = conversionApi.writer.createElement( 'span' );
const position = ModelPosition.createAt( paragraph );
const position = ModelPosition.createAt( paragraph, 0 );

const result = conversionApi.splitToAllowedParent( span, position );

Expand Down Expand Up @@ -572,7 +572,7 @@ describe( 'UpcastDispatcher', () => {
conversionApi.writer.insert( paragraph, section );
conversionApi.writer.insert( span, paragraph );

const position = ModelPosition.createAt( span );
const position = ModelPosition.createAt( span, 0 );

const paragraph2 = conversionApi.writer.createElement( 'paragraph' );
const result = conversionApi.splitToAllowedParent( paragraph2, position );
Expand All @@ -597,7 +597,7 @@ describe( 'UpcastDispatcher', () => {
dispatcher.on( 'documentFragment', ( evt, data, conversionApi ) => {
const paragraph = conversionApi.writer.createElement( 'paragraph' );
const span = conversionApi.writer.createElement( 'span' );
const position = ModelPosition.createAt( paragraph );
const position = ModelPosition.createAt( paragraph, 0 );

const result = conversionApi.splitToAllowedParent( span, position );

Expand Down
10 changes: 7 additions & 3 deletions tests/model/position.js
Original file line number Diff line number Diff line change
Expand Up @@ -158,17 +158,21 @@ describe( 'Position', () => {
} );

describe( 'createAt()', () => {
it( 'should throw if uknown offset is passed', () => {
expect( () => Position.createAt( ul ) ).to.throw( CKEditorError, /model-position-createAt-required-second-parameter/ );
} );

it( 'should create positions from positions', () => {
const spy = testUtils.sinon.spy( Position, 'createFromPosition' );

expect( Position.createAt( Position.createAt( ul ) ) ).to.have.property( 'path' ).that.deep.equals( [ 1, 0 ] );
expect( Position.createAt( Position.createAt( ul, 0 ) ) ).to.have.property( 'path' ).that.deep.equals( [ 1, 0 ] );

expect( spy.calledOnce ).to.be.true;
} );

it( 'should create positions from node and offset', () => {
expect( Position.createAt( ul ) ).to.have.property( 'path' ).that.deep.equals( [ 1, 0 ] );
expect( Position.createAt( li1 ) ).to.have.property( 'path' ).that.deep.equals( [ 1, 0, 0 ] );
expect( Position.createAt( ul, 0 ) ).to.have.property( 'path' ).that.deep.equals( [ 1, 0 ] );
expect( Position.createAt( li1, 0 ) ).to.have.property( 'path' ).that.deep.equals( [ 1, 0, 0 ] );
expect( Position.createAt( ul, 1 ) ).to.have.property( 'path' ).that.deep.equals( [ 1, 1 ] );
} );

Expand Down
2 changes: 1 addition & 1 deletion tests/model/range.js
Original file line number Diff line number Diff line change
Expand Up @@ -176,7 +176,7 @@ describe( 'Range', () => {
describe( 'createCollapsedAt()', () => {
it( 'should return new collapsed range at the given item position', () => {
const item = new Element( 'p', null, new Text( 'foo' ) );
const range = Range.createCollapsedAt( item );
const range = Range.createCollapsedAt( item, 0 );

expect( range.start.parent ).to.equal( item );
expect( range.start.offset ).to.equal( 0 );
Expand Down
28 changes: 14 additions & 14 deletions tests/model/schema.js
Original file line number Diff line number Diff line change
Expand Up @@ -394,8 +394,8 @@ describe( 'Schema', () => {
} );

it( 'accepts a schemaContext instance as a context', () => {
const rootContext = new SchemaContext( Position.createAt( root1 ) );
const paragraphContext = new SchemaContext( Position.createAt( r1p1 ) );
const rootContext = new SchemaContext( Position.createAt( root1, 0 ) );
const paragraphContext = new SchemaContext( Position.createAt( r1p1, 0 ) );

expect( schema.checkChild( rootContext, 'paragraph' ) ).to.be.true;
expect( schema.checkChild( rootContext, '$text' ) ).to.be.false;
Expand All @@ -405,8 +405,8 @@ describe( 'Schema', () => {
} );

it( 'accepts a position as a context', () => {
const posInRoot = Position.createAt( root1 );
const posInParagraph = Position.createAt( r1p1 );
const posInRoot = Position.createAt( root1, 0 );
const posInParagraph = Position.createAt( r1p1, 0 );

expect( schema.checkChild( posInRoot, 'paragraph' ) ).to.be.true;
expect( schema.checkChild( posInRoot, '$text' ) ).to.be.false;
Expand Down Expand Up @@ -486,16 +486,16 @@ describe( 'Schema', () => {
} );

it( 'accepts a position as a context', () => {
const posInRoot = Position.createAt( root1 );
const posInParagraph = Position.createAt( r1p1 );
const posInRoot = Position.createAt( root1, 0 );
const posInParagraph = Position.createAt( r1p1, 0 );

expect( schema.checkAttribute( posInRoot, 'align' ) ).to.be.false;
expect( schema.checkAttribute( posInParagraph, 'align' ) ).to.be.true;
} );

it( 'accepts a schemaContext instance as a context', () => {
const rootContext = new SchemaContext( Position.createAt( root1 ) );
const paragraphContext = new SchemaContext( Position.createAt( r1p1 ) );
const rootContext = new SchemaContext( Position.createAt( root1, 0 ) );
const paragraphContext = new SchemaContext( Position.createAt( r1p1, 0 ) );

expect( schema.checkAttribute( rootContext, 'align' ) ).to.be.false;
expect( schema.checkAttribute( paragraphContext, 'align' ) ).to.be.true;
Expand Down Expand Up @@ -1575,15 +1575,15 @@ describe( 'Schema', () => {
it( 'should return position ancestor that allows to insert given node to it', () => {
const node = new Element( 'paragraph' );

const allowedParent = schema.findAllowedParent( node, Position.createAt( r1bQp ) );
const allowedParent = schema.findAllowedParent( node, Position.createAt( r1bQp, 0 ) );

expect( allowedParent ).to.equal( r1bQ );
} );

it( 'should return position ancestor that allows to insert given node to it when position is already i such an element', () => {
const node = new Text( 'text' );

const parent = schema.findAllowedParent( node, Position.createAt( r1bQp ) );
const parent = schema.findAllowedParent( node, Position.createAt( r1bQp, 0 ) );

expect( parent ).to.equal( r1bQp );
} );
Expand All @@ -1597,7 +1597,7 @@ describe( 'Schema', () => {
} );
const node = new Element( 'div' );

const parent = schema.findAllowedParent( node, Position.createAt( r1bQp ) );
const parent = schema.findAllowedParent( node, Position.createAt( r1bQp, 0 ) );

expect( parent ).to.null;
} );
Expand All @@ -1611,15 +1611,15 @@ describe( 'Schema', () => {
} );
const node = new Element( 'div' );

const parent = schema.findAllowedParent( node, Position.createAt( r1bQp ) );
const parent = schema.findAllowedParent( node, Position.createAt( r1bQp, 0 ) );

expect( parent ).to.null;
} );

it( 'should return null when there is no allowed ancestor for given position', () => {
const node = new Element( 'section' );

const parent = schema.findAllowedParent( node, Position.createAt( r1bQp ) );
const parent = schema.findAllowedParent( node, Position.createAt( r1bQp, 0 ) );

expect( parent ).to.null;
} );
Expand Down Expand Up @@ -2866,7 +2866,7 @@ describe( 'SchemaContext', () => {
} );

it( 'creates context based on a position', () => {
const pos = Position.createAt( root.getChild( 0 ).getChild( 0 ) );
const pos = Position.createAt( root.getChild( 0 ).getChild( 0 ), 0 );
const ctx = new SchemaContext( pos );

expect( ctx.length ).to.equal( 3 );
Expand Down
4 changes: 2 additions & 2 deletions tests/model/writer.js
Original file line number Diff line number Diff line change
Expand Up @@ -1473,7 +1473,7 @@ describe( 'Writer', () => {
const docFrag = createDocumentFragment();

expect( () => {
move( range, docFrag );
move( range, docFrag, 0 );
} ).to.throw( CKEditorError, /^writer-move-different-document/ );
} );

Expand Down Expand Up @@ -2382,7 +2382,7 @@ describe( 'Writer', () => {
const firstParagraph = root.getNodeByPath( [ 1 ] );

const setFocusSpy = sinon.spy( DocumentSelection.prototype, '_setFocus' );
setSelectionFocus( firstParagraph );
setSelectionFocus( firstParagraph, 0 );

expect( setFocusSpy.calledOnce ).to.be.true;
setFocusSpy.restore();
Expand Down
Loading

0 comments on commit 7999456

Please sign in to comment.