From f9ced1911a6375345fd2e17e12609117d40e333d Mon Sep 17 00:00:00 2001 From: Lee Byron Date: Tue, 10 May 2016 19:49:52 -0700 Subject: [PATCH] FIX: slice considers intro/outro, no throw after removal 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`. --- src/MagicString.js | 39 ++++++++++++++++++----- test/index.js | 78 ++++++++++++++++++++++++++++++++++++++++++++-- 2 files changed, 106 insertions(+), 11 deletions(-) diff --git a/src/MagicString.js b/src/MagicString.js index bcda8f3..e845923 100644 --- a/src/MagicString.js +++ b/src/MagicString.js @@ -352,23 +352,46 @@ MagicString.prototype = { return this.overwrite( start, end, '', false ); }, - slice ( start, end = this.original.length ) { + slice ( start, end ) { 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; diff --git a/test/index.js b/test/index.js index fe51ef2..eb35e22 100644 --- a/test/index.js +++ b/test/index.js @@ -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 () {