fix: keep multi-line interpolations from reporting false errors - #36
Conversation
🦋 Changeset detectedLatest commit: ef7bd42 The changes in this PR will be included in the next version bump. This PR includes changesets to release 1 package
Not sure what this means? Click here to learn what changesets are. Click here if you're a maintainer who wants to add another changeset to this PR |
There was a problem hiding this comment.
Important
The substitution fix itself looks right: I could not make it report a false error, and the Emmet view returns the same suggestions as the single-line shape in every layout I tried. One new object is internally inconsistent, though: the Emmet document shim hands a third-party library a TextDocument whose text does not match its own offsets, and the spec sentence that licenses it is factually wrong. Worth fixing in this PR rather than leaving as a trap for the next reader.
Reviewed changes — every fill now writes over the whole placeholder, plus the Emmet line view, the spec, and the tests that cover them.
- Solid fill for opaque positions — a placeholder that starts inside a
/* */or//comment, a string, or an unquotedurl()argument fills withxover its whole length, so the token the CSS service reads no longer ends early. The decision comes from one walk of the spans against the run list (findSolidSpans), and a backslash-started run is excluded so an escape that swallows the placeholder's first character keeps its branch fill. - No fill keeps a line terminator —
fillPlaceholder,wrapPlaceholder, andmaskSubstitutionsbecameString.repeat, the per-character loops are gone, and the hex fill's special case collapses intowrapPlaceholder(placeholderText, { open: HEX_FILL }). The masked text is built once (buildSyntaxText), so the runs are found against the final text. - Emmet line view —
withTemplateLineBreaks(src/features/completions.ts:360) gives Emmet a copy of the virtual document with\nwritten at the end of every template line, which restores Emmet after a multi-line fill, a lone\r, a line continuation, or a U+2028/U+2029 kept inside a string. - Spec and changesets —
docs/architecture.mdgains the solid-fill invariant and the Emmet line view, and the masked-text and cost bullets are reworded for the new fill; the multi-line and Emmet changesets are added and the TypeScript-floor changeset is reworded. - Tests and guardrails — diagnostics rows for each fixed shape (every one keeps a
colrpositive control), substitution rows per fill, an Emmet placement table, fourcompare-releasecases, and two scaling-guardrail shapes that alternate solid and classified placeholders.
What I ran: corepack yarn test:unit and corepack yarn test:scaling are green on the head. Checking out the pre-PR src/template/template-substitutions.ts and src/features/completions.ts against the new tests fails exactly the 53 rows this PR adds or rewrites and nothing else, so the helper rewrite does not move any single-line behavior. A 75-case differential (46 shapes I wrote, across styled, css, and keyframes tags, including CRLF, U+2028/U+2029, an emoji, unterminated comments and strings, and every at-rule and selector position) shows the multi-line variant never reports a diagnostic the single-line variant does not.
ℹ️ Nitpicks
getSubstitutionstill callsfillPlaceholder(placeholderText, 'x')in three branches, which is now character-for-characterSOLID_FILL.repeat(placeholderText.length). Using the constant the PR introduced keeps one name for "the fill character" and saves a reader from wondering whether the two spellings mean the same thing.- The new per-request work in the completions path has no scaling-guardrail case: every entry in
completionCasesends in a line that already has its\n, so the string-building branch ofwithTemplateLineBreaksis never timed. One large template with a multi-line interpolation in the middle would cover it (AGENTS.md, "Performance").
Space Bunny (free) | 𝕏
…late-start offset
There was a problem hiding this comment.
ℹ️ No blocking issues — the same-length view is sound. Two doc-and-test nits inline.
Reviewed changes — what landed since the last Pullfrog review: the Emmet line view rebuilt as a real TextDocument, the solid-fill constant, two new url(#…) diagnostic rows, a new view unit test, and the spec and changeset reword.
- Emmet line view is a consistent document —
withTemplateLineBreaksno longer returns a hand-rolled object mixingalignedwith the unaligned document's offsets andlineCount; it writes\nover each template line's last character, keeping the substituted text's length, and returnsTextDocument.create(...)over it.docs/architecture.md:86's "same length, positions unchanged" claim is now true, and the newtest/unit/completions.test.tspins length, per-offset agreement, and self-consistent line ranges. - Template start computed once —
toVirtualDocOffset(0, context)is read intotemplateStartand shared by the Emmet view and the at-keyword range. SOLID_FILLin the threegetSubstitutionbranches — the literal'x'is gone, so one name means "the fill character".- Two single-line
url(#…)rows —fill: url(#${id}-grad)andmask: url(#${p => p.id}_mask)join thecolr-control group, and the spec's block-position example is narrowed to the case that still produces$a:0. - Scaling guardrail for the completions path —
createManyMultilinePlaceholdersTemplateexercises the string-building branch, which every existing completion case skips. - Changeset — records that 1.0.1 reported these errors too, and that Emmet suggestions are back after a lone carriage return, a line continuation, or a line separator inside a string.
What I ran: corepack yarn test:unit (928 passed), corepack yarn test:scaling (all ratios inside threshold, the new completions case at 3.29), format:check, lint, and typecheck are clean. Checking out main's template-substitutions.ts and completions.ts against the new tests fails exactly the 58 rows this PR adds or rewrites and moves no pre-existing one. A fuzz over 824 templates (mixed \n / \r\n / lone \r / U+2028 / U+2029, 0–3 placeholders in 14 positions, 10 escape shapes) held both invariants across ~97k positions, and completion entry names match the single-line equivalent for 17 shape pairs.
ℹ️ CRLF is absent from the new table on purpose, and one offset really does differ
The fixture table has no CRLF row, and adding one would fail twice. The line-range assertion is the obvious half (inline); this is the other half. A position whose character falls inside a \r\n pair (the \n) maps one character earlier in the view than in the document, because vscode-languageserver-textdocument's offsetAt runs ensureBeforeEOL and never returns an offset between \r and \n, while the plugin's line map happily does. It needs a \r\n outside every placeholder plus one line that needs realignment, so the fresh view is built at all. It is unreachable in practice: the position the plugin receives comes from a client offset through TypeScript's own line map (relative(baseLC, getLineAndChar(...)) in the decorator), so character never exceeds the line's visible content. Every reachable position agrees, and the view's line starts match the plugin's line map even when a template mixes CRLF with a multi-line placeholder.
Space Bunny (free) | 𝕏
…pe its line-range test
There was a problem hiding this comment.
✅ No new issues found. Both nits from the last pass are addressed, and nothing in this delta needs a change.
Reviewed changes — what landed since the last Pullfrog review (920f315 → ef7bd42, one commit, two files).
- The JSDoc now says overwrite, not append —
withTemplateLineBreaksdocuments the\nas "written over the last character of every template line" and adds "An overwrite, not an insert, so the view keeps the document's length and offsets", then names what that character always is: the template's line terminator, a fill character, or a space an escape stand-in wrote. That matches the invariantdocs/architecture.md:86states, so the comment and the spec no longer read as two different mechanisms. - The line-range assertion is scoped to a realigned view — a new
notStrictEqual(view, document)guard keeps the block on the case the prior comment described, and the comparison drops the trailing\ra surviving\r\nleaves on asplit('\n')line, which is the one differencegetLineRangeand Emmet'sgetCurrentLinelegitimately have.
What I ran: corepack yarn test:unit (928 passed, including the three withTemplateLineBreaks rows), corepack yarn test:scaling (every ratio inside threshold; the new completions case at 3.65, the two new substitution cases at 4.40 and 3.89), lint at 0 warnings, and typecheck are clean.
I also checked the new assertion is true in general and not only for its three fixtures, since that was the substance of the prior comment. Over 6400 generated templates (24 fragments covering multi-line placeholders, strings, comments, url() and url(#…), line continuations, hex escapes and at-rules, combined one to three deep across \n, \r\n, lone \r, U+2028 and U+2029) and ~219k positions: every realigned view keeps the document's length, its line starts inside the template region are exactly templateStart + lineStarts, split('\n').length === lineCount always holds, and the replace(/\r$/, '') comparison holds on every line. The only disagreements were the already-documented offsetAt case where a position's character falls inside a \r\n pair, one per template ending in \r\n, unchanged from the last pass and unreachable from a client-derived position.
Space Bunny (free) | 𝕏

Fixes the remaining shapes from #13: an interpolation written across several lines is now checked the same as when written on one line. Prettier produces these layouts often, for example when it wraps
${({ theme }) => theme.breakpoints.md}inside@media, orurl(${props => props.image}), onto several lines.What was wrong
Before the CSS language service reads a template, the plugin swaps each
${...}for filler of the same length. The filler used to keep the interpolation's own line breaks. At runtime the interpolated value contains none of those source line breaks, and in several places a line break changes how CSS reads the text:url()argument, or a//comment, a raw line break ends the token early. The string's real closing quote then opened a new string, which led to errors like) expected, or silently stopped checking for the rest of the template.@media,@keyframes,@container, or@layer, a line break split one name into several tokens and produced errors likecolon expectedor{ expected.What changed
url()argument is filled with plainxover its whole length. This holds both in the text the plugin uses to classify later interpolations and in the text the CSS language service checks, so a mixin later on the same line is still read as a mixin.url()followed by more text, such asurl(#${id}-grad), no longer reports false errors, even on one line.Verification
Language-service tests cover each shape, and every case includes a positive-control typo that must still be reported:
url()andurl(#${id})//and/* */comments@media,@keyframes,@container, and@layerpreludesRows labeled Semi-colon expected after Prettier format #13 fail on
mainand pass here; the rest are regression guards.Two fuzz comparisons, each over roughly 19,000 generated templates, check that multi-line diagnostics now match the single-line version.
New scaling-guardrail shapes cover many multi-line interpolations alternating between strings and code, and completions on a large template with many multi-line interpolations.
A unit test checks that Emmet's line view has the same length and offsets as the virtual document, and that its lines agree with its text.
yarn verifypasses.A separate commit rewords a pending changeset. On TypeScript older than 5.0, 1.0.1 left an unexplained activation error in the TypeScript server log. This release logs that TypeScript 5.0 or newer is required.