Skip to content

fix: carry trimStart and trimEnd into the outro and intro - #348

Open
wahidrizka wants to merge 1 commit into
Rich-Harris:masterfrom
wahidrizka:fix-trim-intro-outro
Open

fix: carry trimStart and trimEnd into the outro and intro#348
wahidrizka wants to merge 1 commit into
Rich-Harris:masterfrom
wahidrizka:fix-trim-intro-outro

Conversation

@wahidrizka

Copy link
Copy Markdown
Contributor

trimStart() trims the intro and then walks the chunks, but once the last chunk is done it returns without looking at the outro. trimEnd() has the mirror gap: it never reaches the intro. So when everything before the outro (or after the intro) is blank, the whitespace at that end of the string survives:

import MagicString, { Bundle } from 'magic-string'

new MagicString('   ').append(' x ').trimStart().toString() // " x " before this fix, "x " after
new MagicString('   ').prepend(' x ').trimEnd().toString()  // " x " before this fix, " x" after

The return value is also wrong in that case: it reports that everything was trimmed away even though the outro or intro still has content. Bundle relies on it to decide whether to keep trimming into the next source, so it goes on to strip the separator and the next source too:

const b = new Bundle()
b.addSource(new MagicString('   ').append('X'))
b.addSource(new MagicString('  Y'))
b.toString()             // "   X\n  Y"
b.trimStart().toString() // "XY" before this fix, "X\n  Y" after

trimEnd() on a bundle does the same from the other side ("Y \nX " became "YX").

Chunk#trimStart already carries on into the chunk's outro once its intro and content are gone, and Chunk#trimEnd into its intro. This does the same one level up: after the last chunk, trimStart() trims the outro and trimEnd() trims the intro, and the return value reports whether anything is left there.

To check the result beyond these cases, I compared trimStart, trimEnd, trim and trimLines against plain string trimming of toString() on 30,000 random edit sequences for MagicString and 30,000 random bundles (whitespace, empty and non-whitespace separators, bundle-level prepend and append). Before the fix there were several thousand mismatches across all four methods on both classes; after it there are none. lastChar(), lastLine(), isEmpty() and clone() also stay consistent with toString() after trimming.

The four new tests (two for MagicString, two for Bundle) fail on the old code and pass now. The suite goes from 366 to 370, and tsc (both configs) and eslint are clean.

CI did not run here: since 77f0e57 the triggers in test.yml list only main, while this repo's default branch is master (they listed both before). I ran the suite, typecheck and lint locally.

trimStart() trimmed the intro and then each chunk, but stopped after the
last chunk without looking at the outro, and trimEnd() never reached the
intro. When everything before the outro (or after the intro) was blank,
the string kept its leading or trailing whitespace, and a Bundle, which
uses the result to decide whether to keep going, went on to trim the
next separator and source as well. Both now carry on into the outro or
intro, as Chunk#trimStart and Chunk#trimEnd already do for a chunk.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant