Skip to content

Keep line numbers after a multi-line self-closing component - #62

Merged
johanrd merged 1 commit into
mainfrom
fix/self-closing-newlines
Sep 15, 2026
Merged

johanrd merged 1 commit into
mainfrom
fix/self-closing-newlines

Conversation

@johanrd

@johanrd johanrd commented Sep 15, 2026

Copy link
Copy Markdown
Owner

A self-closing call that resolves to a non-void element becomes <div …></div> in the blanked output. The blanker wrote this text as one line over the source span, so the newlines of the span were lost. As a result, each error after the call had a line number that was too low.

Change: the newlines of the span now go in the whitespace between the open tag and the close tag. The output length does not change.

Test: multiline-self-closing-consumer.gts, with Glint on and off. Before the fix, the error on line 12 showed on line 8.

Cowritten by Claude

A self-closing call that becomes a non-void element was written as one
line over the source span. This removed the newlines of the span, so
errors on later lines reported line numbers that were too low.
@johanrd johanrd added the bug Something isn't working label Sep 15, 2026
@johanrd
johanrd requested a balanced review from Copilot September 15, 2026 15:24

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🟢 Approval recommended

The change is small, length-preserving, correctly re-embeds newlines (strictly better than the prior all-spaces behavior), and is covered by a new test in both Glint modes that follows existing conventions.

Pull request overview

This PR fixes a line-number regression in the blanking transform. When a self-closing component call (e.g. <InfoPopover … />) resolves to a non-void native element, substituteSelfClosingComponent rewrites the source span to <div …></div>. Because the whole span is replaced via a renames entry (which overwrites the original characters, unlike blankRanges that preserve \r\n), the newlines of a multi-line call were collapsed to spaces, shifting every subsequent error to a lower line number. The fix re-embeds the span's newlines inside the substituted element while keeping the replacement byte-length identical, so downstream line numbers stay correct.

Changes:

  • Preserve the span's newline characters inside the substituted element instead of padding solely with spaces, keeping output length unchanged.
  • Add an integration test asserting a later no-implicit-button-type error reports its true source line (12:6) with Glint on and off.
  • Add info-popover-leaf.gts (non-void <div> root) and multiline-self-closing-consumer.gts fixtures exercising a five-line self-closing call.
File summaries
File Description
blank.ts Re-embeds the source span's \r\n into the substituted element's inner padding, preserving both length and newline count.
test/integration.test.ts New parameterized (Glint on/off) test verifying line numbers after a multi-line self-closing substitution.
examples/multiline-self-closing-consumer.gts Consumer fixture with a five-line self-closing call followed by a <button> on a known line.
test/glint-fixtures/info-popover-leaf.gts Leaf component whose non-void <div> root produces a long substituted open tag.
Review details
  • Files reviewed: 4/4 changed files
  • Comments generated: 0
  • Review effort level: Balanced

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

@johanrd
johanrd merged commit 6b248d4 into main Sep 15, 2026
9 checks passed
@github-actions github-actions Bot mentioned this pull request Sep 15, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

bug Something isn't working

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants