From 1957eb3e281c5fe8d8241815525c28cc50ad47ba Mon Sep 17 00:00:00 2001 From: Ed Sanders Date: Thu, 3 Oct 2013 10:52:21 +0100 Subject: [PATCH] Rename getDocumentSlice to cloneFromRange We already getSlice which returns a ve.dm.DocumentSlice, so using the word slice in this method is very confusing. What we are actually doing is creating a ve.dm.Document from a range. Also remove argument overloading as it's not particularly helpful and would make the new name a lie. Change-Id: I93da3419510410b170396e6765fbe2a87f9795be --- modules/ve-mw/test/dm/ve.dm.Transaction.test.js | 2 +- .../ve-mw/ui/dialogs/ve.ui.MWMediaEditDialog.js | 2 +- .../ve-mw/ui/dialogs/ve.ui.MWReferenceDialog.js | 2 +- modules/ve/dm/ve.dm.Document.js | 17 +++++------------ modules/ve/test/dm/ve.dm.Document.test.js | 16 ++++++++-------- modules/ve/test/dm/ve.dm.Transaction.test.js | 2 +- 6 files changed, 17 insertions(+), 24 deletions(-) diff --git a/modules/ve-mw/test/dm/ve.dm.Transaction.test.js b/modules/ve-mw/test/dm/ve.dm.Transaction.test.js index c38b7c1624..5435092f9e 100644 --- a/modules/ve-mw/test/dm/ve.dm.Transaction.test.js +++ b/modules/ve-mw/test/dm/ve.dm.Transaction.test.js @@ -127,7 +127,7 @@ QUnit.test( 'newFromDocumentReplace with references', function ( assert ) { if ( cases[i].newDocData ) { doc2 = new ve.dm.Document( cases[i].newDocData ); } else { - doc2 = doc.getDocumentSlice( cases[i].range ); + doc2 = doc.cloneFromRange( cases[i].range instanceof ve.Range ? cases[i].range : cases[i].range.getRange() ); cases[i].modify( doc2 ); } tx = ve.dm.Transaction.newFromDocumentReplace( doc, cases[i].range, doc2 ); diff --git a/modules/ve-mw/ui/dialogs/ve.ui.MWMediaEditDialog.js b/modules/ve-mw/ui/dialogs/ve.ui.MWMediaEditDialog.js index b3324630e3..c0bcf6f7d9 100644 --- a/modules/ve-mw/ui/dialogs/ve.ui.MWMediaEditDialog.js +++ b/modules/ve-mw/ui/dialogs/ve.ui.MWMediaEditDialog.js @@ -97,7 +97,7 @@ ve.ui.MWMediaEditDialog.prototype.onOpen = function () { this.mediaNode = this.surface.getView().getFocusedNode().getModel(); this.captionNode = this.mediaNode.getCaptionNode(); if ( this.captionNode && this.captionNode.getLength() > 0 ) { - newDoc = doc.getDocumentSlice( this.captionNode ); + newDoc = doc.cloneFromRange( this.captionNode.getRange() ); } else { newDoc = [ { 'type': 'paragraph', 'internal': { 'generated': 'wrapper' } }, diff --git a/modules/ve-mw/ui/dialogs/ve.ui.MWReferenceDialog.js b/modules/ve-mw/ui/dialogs/ve.ui.MWReferenceDialog.js index 12c0f8717e..2720cd13d4 100644 --- a/modules/ve-mw/ui/dialogs/ve.ui.MWReferenceDialog.js +++ b/modules/ve-mw/ui/dialogs/ve.ui.MWReferenceDialog.js @@ -279,7 +279,7 @@ ve.ui.MWReferenceDialog.prototype.useReference = function ( ref ) { 'refGroup': ref.refGroup, 'listIndex': ref.listIndex }; - newDoc = doc.getDocumentSlice( doc.getInternalList().getItemNode( ref.listIndex ) ); + newDoc = doc.cloneFromRange( doc.getInternalList().getItemNode( ref.listIndex ).getRange() ); refGroup = ref.refGroup; } else { // Create a new reference diff --git a/modules/ve/dm/ve.dm.Document.js b/modules/ve/dm/ve.dm.Document.js index d71e9c1ee7..e5b372462a 100644 --- a/modules/ve/dm/ve.dm.Document.js +++ b/modules/ve/dm/ve.dm.Document.js @@ -304,24 +304,17 @@ ve.dm.Document.prototype.getInternalList = function () { }; /** - * Get a document from a slice of this document. The new document's store and internal list will be + * Clone a sub-document from a range in this document. The new document's store and internal list will be * clones of the ones in this document. * - * @param {ve.Range|ve.dm.Node} rangeOrNode Range of data to clone, or node whose contents should be cloned + * @param {ve.Range} range Range of data to clone * @returns {ve.dm.Document} New document - * @throws {Error} rangeOrNode must be a ve.Range or a ve.dm.Node */ -ve.dm.Document.prototype.getDocumentSlice = function ( rangeOrNode ) { - var data, range, newDoc, +ve.dm.Document.prototype.cloneFromRange = function ( range ) { + var data, newDoc, store = this.store.clone(), listRange = this.internalList.getListNode().getOuterRange(); - if ( rangeOrNode instanceof ve.dm.Node ) { - range = rangeOrNode.getRange(); - } else if ( rangeOrNode instanceof ve.Range ) { - range = rangeOrNode; - } else { - throw new Error( 'rangeOrNode must be a ve.Range or a ve.dm.Node' ); - } + data = ve.copy( this.getFullData( range, true ) ); if ( range.start > listRange.start || range.end < listRange.end ) { // The range does not include the entire internal list, so add it diff --git a/modules/ve/test/dm/ve.dm.Document.test.js b/modules/ve/test/dm/ve.dm.Document.test.js index 0efe710dcd..eef892050b 100644 --- a/modules/ve/test/dm/ve.dm.Document.test.js +++ b/modules/ve/test/dm/ve.dm.Document.test.js @@ -69,38 +69,38 @@ QUnit.test( 'getFullData', 1, function ( assert ) { assert.deepEqualWithDomElements( doc.getFullData(), ve.dm.example.withMeta ); } ); -QUnit.test( 'getDocumentSlice', function ( assert ) { +QUnit.test( 'cloneFromRange', function ( assert ) { var i, doc2, doc = ve.dm.example.createExampleDocument( 'internalData' ), cases = [ { - 'msg': 'with range', + 'msg': 'first internal item', 'doc': 'internalData', - 'arg': new ve.Range( 7, 12 ), + 'range': new ve.Range( 7, 12 ), 'expectedData': doc.data.slice( 7, 12 ).concat( doc.data.slice( 5, 21 ) ) }, { - 'msg': 'with node', + 'msg': 'second internal item', 'doc': 'internalData', - 'arg': doc.getInternalList().getItemNode( 1 ), + 'range': doc.getInternalList().getItemNode( 1 ).getRange(), 'expectedData': doc.data.slice( 14, 19 ).concat( doc.data.slice( 5, 21 ) ) }, { 'msg': 'paragraph at the start', 'doc': 'internalData', - 'arg': new ve.Range( 0, 5 ), + 'range': new ve.Range( 0, 5 ), 'expectedData': doc.data.slice( 0, 21 ) }, { 'msg': 'paragraph at the end', 'doc': 'internalData', - 'arg': new ve.Range( 21, 27 ), + 'range': new ve.Range( 21, 27 ), 'expectedData': doc.data.slice( 21, 27 ).concat( doc.data.slice( 5, 21 ) ) } ]; QUnit.expect( 4*cases.length ); for ( i = 0; i < cases.length; i++ ) { doc = ve.dm.example.createExampleDocument( cases[i].doc ); - doc2 = doc.getDocumentSlice( cases[i].arg ); + doc2 = doc.cloneFromRange( cases[i].range ); assert.deepEqual( doc2.data.data, cases[i].expectedData, cases[i].msg + ': sliced data' ); assert.notStrictEqual( doc2.data[0], cases[i].expectedData[0], diff --git a/modules/ve/test/dm/ve.dm.Transaction.test.js b/modules/ve/test/dm/ve.dm.Transaction.test.js index 463b646d2e..6cb7a0ae92 100644 --- a/modules/ve/test/dm/ve.dm.Transaction.test.js +++ b/modules/ve/test/dm/ve.dm.Transaction.test.js @@ -797,7 +797,7 @@ QUnit.test( 'newFromDocumentReplace', function ( assert ) { if ( cases[i].newDocData ) { doc2 = new ve.dm.Document( cases[i].newDocData ); } else { - doc2 = doc.getDocumentSlice( cases[i].range ); + doc2 = doc.cloneFromRange( cases[i].range instanceof ve.Range ? cases[i].range : cases[i].range.getRange() ); cases[i].modify( doc2 ); } tx = ve.dm.Transaction.newFromDocumentReplace( doc, cases[i].range, doc2 );