r101416 MediaWiki - Code Review archive

Repository:MediaWiki
Revision:< r101415‎ | r101416 | r101417 >
Date:22:06, 31 October 2011
Author:tparscal
Status:deferred
Tags:
Comment:
Fixed getRelativeContentOffset so that it considers non-structural offsets to be valid content-offsets, and renamed isContentOffset and isElementOffset to isContentData and isElementData respectively, to better represent what they actually do. In the former case isContentOffset was also simplified, since the actual purpose of the additional checks are better achieved by negating the result of isStructuralOffset.
Modified paths:
  • /trunk/parsers/wikidom/lib/hype/models/es.DocumentModel.js (modified) (history)
  • /trunk/parsers/wikidom/tests/hype/es.DocumentModel.test.js (modified) (history)

Diff [purge]

Index: trunk/parsers/wikidom/tests/hype/es.DocumentModel.test.js
——@@ -215,7 +215,7 @@
216216 deepEqual( documentModel.slice( 0 ), tree, 'Nodes in the model tree contain correct lengths' );
217217 } );
218218
219 -test( 'es.DocumentModel.getRelativeContentOffset', 6, function() {
 219+test( 'es.DocumentModel.getRelativeContentOffset', 7, function() {
220220 var documentModel = es.DocumentModel.newFromPlainObject( obj );
221221
222222 // Test 1
——@@ -232,26 +232,32 @@
233233 );
234234 // Test 3
235235 equal(
 236+ documentModel.getRelativeContentOffset( 3, 1 ),
 237+ 4,
 238+ 'getRelativeContentOffset uses the offset after the last character in an element'
 239+ );
 240+ // Test 3
 241+ equal(
236242 documentModel.getRelativeContentOffset( 1, -1 ),
237243 1,
238244 'getRelativeContentOffset treats the begining a document as a non-content offset'
239245 );
240246 // Test 4
241247 equal(
242 - documentModel.getRelativeContentOffset( 26, 1 ),
243 - 26,
 248+ documentModel.getRelativeContentOffset( 27, 1 ),
 249+ 27,
244250 'getRelativeContentOffset treats the end a document as a non-content offset'
245251 );
246252 // Test 5
247253 equal(
248 - documentModel.getRelativeContentOffset( 3, 1 ),
 254+ documentModel.getRelativeContentOffset( 4, 1 ),
249255 9,
250256 'getRelativeContentOffset advances forwards between elements'
251257 );
252258 // Test 6
253259 equal(
254260 documentModel.getRelativeContentOffset( 26, -1 ),
255 - 19,
 261+ 20,
256262 'getRelativeContentOffset advances backwards between elements'
257263 );
258264 } );
Index: trunk/parsers/wikidom/lib/hype/models/es.DocumentModel.js
——@@ -116,8 +116,8 @@
117117 }
118118
119119 function remove( op ) {
120 - var elementLeft = es.DocumentModel.isElementOffset( this.data, this.cursor ),
121 - elementRight = es.DocumentModel.isElementOffset( this.cursor + op.data.length );
 120+ var elementLeft = es.DocumentModel.isElementData( this.data, this.cursor ),
 121+ elementRight = es.DocumentModel.isElementData( this.cursor + op.data.length );
122122 if ( elementLeft && elementRight ) {
123123 // TODO: Support tree updates when removing whole elements
124124 } else {
——@@ -486,9 +486,7 @@
487487 /**
488488 * Checks if a data at a given offset is content.
489489 *
490 - * Content offsets are those which are between an opening and a closing element.
491 - *
492 - * @example Content offsets:
 490+ * @example Content data:
493491 * <paragraph> a b c </paragraph> <list> <listItem> d e f </listItem> </list>
494492 * ^ ^ ^ ^ ^ ^
495493 *
——@@ -498,30 +496,15 @@
499497 * @param {Integer} offset Offset in data to check
500498 * @returns {Boolean} If data at offset is content
501499 */
502 -es.DocumentModel.isContentOffset = function( data, offset ) {
503 - // Content can't exist at the edges
504 - if ( offset > 0 && offset < data.length ) {
505 - // Shortcut: if there's already content there, we will trust it's supposed to be there
506 - if ( typeof data[offset] === 'string' || es.isArray( data[offset] ) ) {
507 - return true;
508 - }
509 - // Empty elements will have an opening and a closing next to each other
510 - var openLeft = data[offset - 1].type !== undefined && data[offset - 1].type[0] !== '/',
511 - closeRight = data[offset].type !== undefined && data[offset].type[0] !== '/';
512 - // Check that there's an opening and a closing on the left and right, and the types match
513 - if ( openLeft && closeRight && ('/' + data[offset - 1].type === data[offset].type ) ) {
514 - return true;
515 - }
516 - }
517 - return false;
 500+es.DocumentModel.isContentData = function( data, offset ) {
 501+ // Shortcut: if there's already content there, we will trust it's supposed to be there
 502+ return typeof data[offset] === 'string' || es.isArray( data[offset] );
518503 };
519504
520505 /**
521506 * Checks if a data at a given offset is an element.
522507 *
523 - * Element offsets are those at which an element is present.
524 - *
525 - * @example Element offsets:
 508+ * @example Element data:
526509 * <paragraph> a b c </paragraph> <list> <listItem> d e f </listItem> </list>
527510 * ^ ^ ^ ^ ^ ^
528511 *
——@@ -531,7 +514,7 @@
532515 * @param {Integer} offset Offset in data to check
533516 * @returns {Boolean} If data at offset is an element
534517 */
535 -es.DocumentModel.isElementOffset = function( data, offset ) {
 518+es.DocumentModel.isElementData = function( data, offset ) {
536519 // TODO: Is there a safer way to check if it's a plain object without sacrificing speed?
537520 return offset >= 0 && offset < data.length && data[offset].type !== undefined;
538521 };
——@@ -716,7 +699,7 @@
717700 * @returns {es.Range|null} Range of content making up a whole word or null if offset is not content
718701 */
719702 es.DocumentModel.prototype.getWordBoundaries = function( offset ) {
720 - if ( !es.DocumentModel.isContentOffset( this.data, offset ) ) {
 703+ if ( es.DocumentModel.isStructuralOffset( this.data, offset ) ) {
721704 return null;
722705 }
723706 var start = offset,
——@@ -756,7 +739,7 @@
757740 * @param {Integer} Offset a given distance from the given offset
758741 */
759742 es.DocumentModel.prototype.getRelativeContentOffset = function( offset, distance ) {
760 - if ( !es.DocumentModel.isContentOffset( this.data, offset ) ) {
 743+ if ( es.DocumentModel.isStructuralOffset( this.data, offset ) ) {
761744 throw 'Invalid offset error. Can not get relative content offset from non-content offset.';
762745 }
763746 if ( distance === 0 ) {
——@@ -767,7 +750,7 @@
768751 steps = 0;
769752 distance = Math.abs( distance );
770753 while ( i > 0 && i < this.data.length - 1 ) {
771 - if ( typeof this.data[i] === 'string' || es.isArray( this.data[i] ) ) {
 754+ if ( !es.DocumentModel.isStructuralOffset( this.data, i ) ) {
772755 steps++;
773756 offset = i;
774757 if ( distance === steps ) {

Status & tagging log