Repository navigation
Conversation
Bundle#indent decided whether a source starts a line from the last character of the source before it. An empty source has none, so the source after it was treated as continuing a line and its first line was left unindented, even at the very start of the bundle or after a newline. An empty source now leaves that state as it found it.
Sinkleberg
left a comment
Sinkleberg
left a comment
There was a problem hiding this comment.
An empty source with a non-newline separator now makes the next source indent in the middle of a line:
const b = new Bundle({ separator: '' });
b.addSource({ filename: 'previous.js', content: new MagicString('a\n') });
b.addSource({ filename: 'empty.js', separator: ';', content: new MagicString('') });
b.addSource({ filename: 'next.js', content: new MagicString('b\nc') });
console.log(JSON.stringify(b.indent('>').toString()));
// base: ">a\n;b\n>c"
// PR: ">a\n;>b\n>c"The ; has already started the line, so b continues it. At src/Bundle.ts:406, the empty-source branch carries indentStart forward even though the separator emitted visible text.
Codex-assisted review, checked against the PR head and base.
…ndent A source that follows a separator such as ';' continues the line that separator started, so it is not indented. Carrying the line state past an empty source made the next source indent after the ';' instead. The separator now decides the state when it ends in a newline or has visible text; empty and whitespace-only separators leave it as before.
|
Thanks for adding the regression test. The updated separator handling fixes the case I reported. |
|
Thanks, you're right. Carrying Fixed in 09c6bca. When there is a separator, it now sets the line state: one ending in a newline starts a line, and one with other visible text continues it. Empty and whitespace-only separators leave the state alone, same as on master. That also covers a case master already got wrong, where the source before the const b = new Bundle({ separator: '' })
b.addSource(new MagicString('a\n'))
b.addSource({ separator: ';', content: new MagicString('b\nc') })
b.indent('>').toString()
// master: '>a\n;>b\n>c', now: '>a\n;b\n>c'Your example gives |
Bundle#indent()leaves the first line of a source unindented when the source before it is empty:It happens wherever a source's output is empty: an empty string, a source whose content was removed, or one that
trim()emptied, at the start of the bundle or between two sources:Cause
indent()tracks whether the output so far ends a line, to decide whether each source starts one (indentStart). After each source it sets that fromsource.content.lastChar() === '\n'. An empty source has no last character, so the flag becomesfalse, as if the empty source had left text on the line. The next source is then indented as a continuation.Fix
An empty source adds nothing, so the flag after it is the one it started with: its own
indentStart, which already takes its separator into account. Non-empty sources are unchanged.Evidence
I compared
bundle.indent(str).toString()withnew MagicString(bundle.toString()).indent(str).toString()on random bundles: one to four sources (empty, removed, trimmed or plain, with and without newlines), randomprepend,appendand trims on the bundle, and the separators'\n','','\r\n'and'\n\n'. Over 100,000 bundles, 16,066 fail on master and all of them pass with this change. None fail before and after, and none newly fail.With a separator that contains visible text, such as
';\n', some bundles still differ from the reference before and after this change, for example when a trim leaves the;at the start of a line.indent()does not indent separators, since they are not part of any source, so that is a separate question and I left it out of this PR.Three tests are added: an empty source first and in the middle, a source emptied by
trim(), and a case pinning that a source after an empty one still continues the line when no newline came before. The first two fail on master. The suite is at 497 and passes, ESLint is clean, and typecheck reports no errors. The edit-model fuzz still passes at 20,000 sequences for seeds 1 and 2.