fix: carry trimStart and trimEnd into the outro and intro - #348
Open
wahidrizka wants to merge 1 commit into
Open
fix: carry trimStart and trimEnd into the outro and intro#348wahidrizka wants to merge 1 commit into
wahidrizka wants to merge 1 commit into
Conversation
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.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
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: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.
Bundlerelies 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:trimEnd()on a bundle does the same from the other side ("Y \nX "became"YX").Chunk#trimStartalready carries on into the chunk's outro once its intro and content are gone, andChunk#trimEndinto its intro. This does the same one level up: after the last chunk,trimStart()trims the outro andtrimEnd()trims the intro, and the return value reports whether anything is left there.To check the result beyond these cases, I compared
trimStart,trimEnd,trimandtrimLinesagainst plain string trimming oftoString()on 30,000 random edit sequences forMagicStringand 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()andclone()also stay consistent withtoString()after trimming.The four new tests (two for
MagicString, two forBundle) fail on the old code and pass now. The suite goes from 366 to 370, andtsc(both configs) andeslintare clean.CI did not run here: since 77f0e57 the triggers in
test.ymllist onlymain, while this repo's default branch ismaster(they listed both before). I ran the suite, typecheck and lint locally.