Visitar URL original
Place template annotation on a separate line by siaw23 · Pull Request #53731 · rails/rails · GitHub
Skip to content

Place template annotation on a separate line - #53731

Merged
rafaelfranca merged 1 commit into
rails:mainfrom
siaw23:template-annotation-comment
Oct 7, 2025
Merged

rafaelfranca merged 1 commit into
rails:mainfrom
siaw23:template-annotation-comment

Conversation

@siaw23

@siaw23 siaw23 commented Nov 25, 2024 •

Copy link
Copy Markdown
Member

Motivation / Background

The BEGIN template annotation/comment is printed on the same line with other element tags. This PR fixes that by moving the comment to its own line.

Before:

<!-- BEGIN /Users/siaw23/Desktop/rails/actionview/test/fixtures/actionpack/test/greeting.html.erb --><p>This is grand!</p>

After:

<!-- BEGIN /Users/siaw23/Desktop/rails/actionview/test/fixtures/actionpack/test/greeting.html.erb -->
<p>This is grand!</p>

Detail

This Pull Request changes [REPLACE ME]

Additional information

Checklist

Before submitting the PR make sure the following are checked:

  • This Pull Request is related to one change. Unrelated changes should be opened in separate PRs.
  • Commit message has a detailed description of what changed and why. If this PR fixes a related issue include it in the commit message. Ex: [Fix #issue-number]
  • Tests are added or updated if you fix a bug or add a feature.
  • CHANGELOG files are updated for the changed libraries if there is a behavior change or additional feature. Minor bug fixes and documentation changes should not be included.

@rails-bot rails-bot Bot added the actionview label Nov 25, 2024
@siaw23
siaw23 force-pushed the template-annotation-comment branch 2 times, most recently from c82c796 to 419f680 Compare November 25, 2024 00:28
@siaw23 siaw23 changed the title Put template annotation on its own line Place template annotation on a separate line Nov 25, 2024
@matthewd

Copy link
Copy Markdown
Member

It's not always safe to add whitespace in HTML. If we want to do this, I think the newline would need to be inside the comment.

@siaw23
siaw23 force-pushed the template-annotation-comment branch 2 times, most recently from dd0b121 to 980e392 Compare October 5, 2025 13:23
@siaw23

siaw23 commented Oct 5, 2025

Copy link
Copy Markdown
Member Author

Got it. Made some changes @matthewd

The BEGIN template annotation/comment was previously printed on the same line as the following element. We now insert a newline inside the comment so it spans two lines without adding visible whitespace to the HTML output to enhance readability.
@rafaelfranca
rafaelfranca force-pushed the template-annotation-comment branch from 980e392 to 4ae5bfc Compare October 6, 2025 23:40
@rafaelfranca
rafaelfranca merged commit 2ea1d9b into rails:main Oct 7, 2025
3 checks passed
@chaadow

chaadow commented Oct 10, 2025

Copy link
Copy Markdown
Contributor

@rafaelfranca This is weird I usually follow merges to main on Discord in the #rails-commits channel, and this PR/commit was not displayed there

I only stumbled upon this PR on @zzak "This week in rails" e-mail

Was there something different that changed, or the discord integration does not work all the time?

image

@zzak

zzak commented Oct 12, 2025

Copy link
Copy Markdown
Member

^ Fixed, thanks for reporting!

mizoR added a commit to mizoR/view_component that referenced this pull request Sep 13, 2026
Rails 8.1 added a newline to compiled ERB output (rails/rails#53731), and
ViewComponent compensates with a lineno of -1 so backtraces point at the
right line. That newline lives inside the `<!-- BEGIN ... -->` annotation,
which Rails only prepends when `annotate_rendered_view_with_filenames` is
enabled *and* the template's format is HTML.

Compensating for every ERB template shifted backtraces by one line for
non-HTML templates (`.text.erb`, `.css.erb`) and for every ERB template
when annotations are disabled, such as in production. When coverage was
running, the same mismatch made `strip_annotation_line?` drop everything
up to the first `;` of a preamble that was never emitted, so non-HTML
templates rendered empty.

Move the annotation conditions into `erb_newline_compensation_needed?` on
`Template` so every call site fires exactly when Rails emits the
annotation. The negative lineno is still avoided under coverage
(bugs.ruby-lang.org/issues/19363).

`Template::Inline` had the same off-by-one, and decided the compensation
when the component class was defined rather than when it compiled. It now
shares the predicate and resolves it at compile time, matching
`Template::File`.

Also add the missing `ensure` to the `without_template_annotations` test
helper, so an exception escaping the block no longer leaves annotations
disabled for the rest of the process.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants