Skip to content

Commit

Permalink
FIX: slice considers intro/outro, no throw after removal
Browse files Browse the repository at this point in the history
This fixes a few issues with slice introduced recently. Tests are added which fail before this patch.

* slice now considers `intro` and `outro`: the recent change from `insert` to `insertLeft`/`insertRight` changed implementations to use `intro`/`outro` on a chunk, but slice wasn't updated to this new impl. Now, any `intro`/`outro` contained within the slice bounds is included in the result.

* slice was broken if called after a `removed` or otherwise `edited` chunk: Because all chunks were being considered, regardless of if they overlapped the sliced range, a false-positive error was thrown indicating slicing a removed range. Now chunks which end before the sliced range are quickly skipped, removing the issue. As a bonus this adds a clarification of *which* bound was problematic.

* slice was broken if chunks were moved such that chunks between the moved chunks would otherwise not be considered "included" in the slice range despite being between the start and end chunks.

* performance of a slice of a small range suffers when in a large document: Previously all chunks were being considered, however the while loop can cease early as soon as a chunk is encountered which starts after the sliced range.

* slice API behavior different slightly from Array#slice, that if a `start` is not provided, the empty string is provided, instead of the expected default `0` being used as `start`.
  • Loading branch information
leebyron committed May 13, 2016
1 parent e2cd263 commit 7d280ef
Show file tree
Hide file tree
Showing 2 changed files with 106 additions and 11 deletions.
39 changes: 31 additions & 8 deletions src/MagicString.js
Expand Up @@ -352,23 +352,46 @@ MagicString.prototype = {
return this.overwrite( start, end, '', false );
},

slice ( start, end = this.original.length ) {
slice ( start = 0, end = this.original.length ) {
while ( start < 0 ) start += this.original.length;
while ( end < 0 ) end += this.original.length;

let result = '';

// find start chunk
let chunk = this.firstChunk;
while ( chunk && ( chunk.start > start || chunk.end <= start ) ) {

// found end chunk before start
if ( chunk.start < end && chunk.end >= end ) {
return result;
}

chunk = chunk.next;
}

if ( chunk && chunk.edited && chunk.start !== start ) throw new Error(`Cannot use replaced character ${start} as slice start anchor.`);

let startChunk = chunk;
while ( chunk ) {
if ( chunk.start < start || chunk.end > end ) {
if ( chunk.edited ) throw new Error( `Cannot use replaced characters (${start}, ${end}) as slice anchors` );
if ( chunk.intro && ( startChunk !== chunk || chunk.start === start ) ) {
result += chunk.intro;
}

const containsEnd = chunk.start < end && chunk.end >= end;
if ( containsEnd && chunk.edited && chunk.end !== end ) throw new Error(`Cannot use replaced character ${end} as slice end anchor.`);

const sliceStart = Math.max( start - chunk.start, 0 );
const sliceEnd = Math.min( chunk.content.length - ( chunk.end - end ), chunk.content.length );
const sliceStart = startChunk === chunk ? start - chunk.start : 0;
const sliceEnd = containsEnd ? chunk.content.length + end - chunk.end : chunk.content.length;

result += chunk.content.slice( sliceStart, sliceEnd );

if ( chunk.outro && ( !containsEnd || chunk.end === end ) ) {
result += chunk.outro;
}

result += chunk.content.slice( sliceStart, sliceEnd );
} else if ( chunk.end > start && chunk.start < end ) {
result += chunk.content;
if ( containsEnd ) {
break;
}

chunk = chunk.next;
Expand Down
78 changes: 75 additions & 3 deletions test/index.js
Expand Up @@ -845,24 +845,96 @@ describe( 'MagicString', function () {
assert.equal( s.slice( 0, -3 ), 'abcdefghi' );
});

it( 'includes inserted characters, respecting insertion direction', function () {
var s = new MagicString( 'abefij' );

s.insertRight( 2, 'cd' );
s.insertLeft( 4, 'gh' );

assert.equal( s.slice(), 'abcdefghij' );
assert.equal( s.slice( 1, 5 ), 'bcdefghi' );
assert.equal( s.slice( 2, 4 ), 'cdefgh' );
assert.equal( s.slice( 3, 4 ), 'fgh' );
assert.equal( s.slice( 0, 2 ), 'ab' );
assert.equal( s.slice( 0, 3 ), 'abcde' );
assert.equal( s.slice( 4, 6 ), 'ij' );
assert.equal( s.slice( 3, 6 ), 'fghij' );
});

it( 'supports characters moved outward', function () {
var s = new MagicString( 'abcdEFghIJklmn' );

s.move( 4, 6, 2 );
s.move( 8, 10, 12 );
assert.equal( s.toString(), 'abEFcdghklIJmn' );

assert.equal( s.slice( 1, -1 ), 'bEFcdghklIJm' );
assert.equal( s.slice( 2, -2 ), 'cdghkl' );
assert.equal( s.slice( 3, -3 ), 'dghk' );
assert.equal( s.slice( 4, -4 ), 'EFcdghklIJ' );
assert.equal( s.slice( 5, -5 ), 'FcdghklI' );
assert.equal( s.slice( 6, -6 ), 'gh' );
});

it( 'supports characters moved inward', function () {
var s = new MagicString( 'abCDefghijKLmn' );

s.move( 2, 4, 6 );
s.move( 10, 12, 8 );
assert.equal( s.toString(), 'abefCDghKLijmn' );

assert.equal( s.slice( 1, -1 ), 'befCDghKLijm' );
assert.equal( s.slice( 2, -2 ), 'CDghKL' );
assert.equal( s.slice( 3, -3 ), 'DghK' );
assert.equal( s.slice( 4, -4 ), 'efCDghKLij' );
assert.equal( s.slice( 5, -5 ), 'fCDghKLi' );
assert.equal( s.slice( 6, -6 ), 'gh' );
});

it( 'supports characters moved opposing', function () {
var s = new MagicString( 'abCDefghIJkl' );

s.move( 2, 4, 8 );
s.move( 8, 10, 4 );
assert.equal( s.toString(), 'abIJefghCDkl' );

assert.equal( s.slice( 1, -1 ), 'bIJefghCDk' );
assert.equal( s.slice( 2, -2 ), '' );
assert.equal( s.slice( 3, -3 ), '' );
assert.equal( s.slice( -3, 3 ), 'JefghC' );
assert.equal( s.slice( 4, -4 ), 'efgh' );
assert.equal( s.slice( 0, 3 ), 'abIJefghC' );
assert.equal( s.slice( 3 ), 'Dkl' );
assert.equal( s.slice( 0, -3 ), 'abI' );
assert.equal( s.slice( -3 ), 'JefghCDkl' );
});

it( 'errors if replaced characters are used as slice anchors', function () {
var s = new MagicString( 'abcdef' );
s.overwrite( 2, 4, 'CD' );

assert.throws( function () {
s.slice( 2, 3 );
}, /slice anchors/ );
}, /slice end anchor/ );

assert.throws( function () {
s.slice( 3, 4 );
}, /slice anchors/ );
}, /slice start anchor/ );

assert.throws( function () {
s.slice( 3, 5 );
}, /slice anchors/ );
}, /slice start anchor/ );

assert.equal( s.slice( 1, 5 ), 'bCDe' );
});

it( 'does not error if slice is after removed characters', function () {
var s = new MagicString( 'abcdef' );

s.remove( 0, 2 );

assert.equal( s.slice( 2, 4 ), 'cd' );
});
});

describe( 'snip', function () {
Expand Down

0 comments on commit 7d280ef

Please sign in to comment.