Skip to content

Fix trailing whitespace before code block in wrapped doc comments - #7083

Open
siddhardh-codemonk wants to merge 1 commit into
rust-lang:mainfrom
siddhardh-codemonk:fix-6695-trailing-whitespace
Open

Fix trailing whitespace before code block in wrapped doc comments#7083
siddhardh-codemonk wants to merge 1 commit into
rust-lang:mainfrom
siddhardh-codemonk:fix-6695-trailing-whitespace

Conversation

@siddhardh-codemonk

Copy link
Copy Markdown

What this fixes

When a doc comment paragraph gets word-wrapped across multiple lines
(wrap_comments = true) and is immediately followed by a fenced code
block (```), rustfmt was leaving a stray trailing space on the
line right before the code block starts.

Root cause

In CommentRewrite::handle_line, when the previous line was wrapped
into multiple lines, a single space gets pushed to self.result for
potential same-line continuation. If the current line turns out to be
the start of a code block, a separate branch then pushes a newline
separator (comment_line_separator) right after — but the earlier
space was never removed, leaving it as trailing whitespace right
before the inserted newline.

Fix

Pop the trailing space (if present) immediately before pushing the
code-block separator, following the same pattern already used
elsewhere in this file (e.g. the existing .pop() calls nearby).

Testing

Verified against the repro in #6695:

Before:

error[internal]: left behind trailing whitespace
--> issue_6695.rs:3:3:4

After: that error no longer occurs. cargo test comment:: still
passes (15 passed, 0 failed).

Note: #6695 also reports a second, separate issue (a line-width
overflow caused by MIN_STRING in break_string rejecting a valid
break point). That one is more involved since it touches shared
wrapping logic used elsewhere in the codebase — I've written up my
findings in a comment on the issue and left it open for discussion
before attempting a fix there.

@rustbot rustbot added the S-waiting-on-review Status: awaiting review from the assignee but also interested parties. label Aug 29, 2026
@ytmimi

ytmimi commented Aug 29, 2026

Copy link
Copy Markdown
Contributor

@siddhardh-codemonk please disclose how much of this PR was generated using an LLM. Also, was the PR description generated using an LLM?

@ytmimi ytmimi 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.

I think we need to understand why we're adding whitespace incorrectly and prevent that from happening instead of just simply popping off the whitespace after it's been added.

Also, this PR doesn't have any tests to make sure we've solved the underlying issues.

View changes since this review

Comment thread src/comment.rs
Comment on lines +897 to +899
if self.result.ends_with(' ') {
self.result.pop();
}

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.

I don't want to just pop off extra whitespace. I want us to understand why we're adding the erroneous whitespace in the first place.

@rustbot rustbot added S-waiting-on-author Status: awaiting some action (such as code changes or more information) from the author. and removed S-waiting-on-review Status: awaiting review from the assignee but also interested parties. labels Aug 29, 2026
@rustbot

rustbot commented Aug 29, 2026

Copy link
Copy Markdown
Collaborator

Reminder, once the PR becomes ready for a review, use @rustbot ready.

@siddhardh-codemonk

Copy link
Copy Markdown
Author

Thanks for the review — that makes sense, I'll rework it to fix the root cause instead of popping the whitespace after the fact.

On the LLM question: I used ChatGPT to help verify how the variables get passed through (tracing values like is_prev_line_multi_line and code_block_attr across the function), and Claude to help me understand the overall flow and reasoning through the bug. The PR description was also drafted with Claude's help based on my own findings and testing. I'm still learning to read a codebase this size, so I leaned on both for understanding rather than having either generate the fix outright — I ran and verified everything locally myself.

@siddhardh-codemonk

Copy link
Copy Markdown
Author

On the root cause: when the previous line gets word-wrapped into multiple lines, handle_line pushes a single space onto self.result — this branch:
Screenshot 2026-08-30 113133

This assumes the current line should be joined onto the wrapped text with just a space (e.g. continuing a wrapped sentence). But it doesn't check whether the current line is actually the start of a fenced code block. When it is, a separate branch further down (around line 896) then pushes a real newline separator right after, because a code block needs to start on its own line — but the space from the earlier branch was already written and never gets cleaned up, so it ends up sitting there as trailing whitespace right before the inserted newline.

Basically: two branches both react to the same line, one assuming "same line continuation" and one assuming "new line needed," and they don't coordinate — so the output gets both a space AND a newline instead of just the newline.

@siddhardh-codemonk

siddhardh-codemonk commented Aug 30, 2026

Copy link
Copy Markdown
Author

For the fix, instead of popping the space after it's added, I prefer to stop the space from being added in the first place — adding && self.code_block_attr.is_none() to that same condition. This way no space gets added at the start of a code fence, and there's nothing left to clean up afterward.

While testing this, I also found that the block I'd added earlier (lines ~896-902) is now redundant — it was pushing a separator even though the normal flow already handles adding one correctly once the space isn't pushed in the first place. I removed it.

If that looks right to you, I'll finalize this PR for bug 2 and add a regression test, then move on to bug 1 — focusing on the MIN_STRING and break_string issues.

@ytmimi

ytmimi commented Aug 31, 2026

Copy link
Copy Markdown
Contributor

Thanks for the review — that makes sense, I'll rework it to fix the root cause instead of popping the whitespace after the fact.

On the LLM question: I used ChatGPT to help verify how the variables get passed through (tracing values like is_prev_line_multi_line and code_block_attr across the function), and Claude to help me understand the overall flow and reasoning through the bug. The PR description was also drafted with Claude's help based on my own findings and testing. I'm still learning to read a codebase this size, so I leaned on both for understanding rather than having either generate the fix outright — I ran and verified everything locally myself.

Thank you for disclosing that. Per the LLM policy, generating / drafting a PR description isn't allowed. They are fine to use when you're trying to navigate the codebase or better understand an issue, but please don't use LLMs to draft PRs, open issues, or leave comments when interacting with rust-lang projects, which includes rustfmt. Almost every comment you've left on this PR feels to me like it's been generated with an LLM.

@ytmimi

ytmimi commented Aug 31, 2026

Copy link
Copy Markdown
Contributor

For the fix, instead of popping the space after it's added, I prefer to stop the space from being added in the first place — adding && self.code_block_attr.is_none() to that same condition. This way no space gets added at the start of a code fence, and there's nothing left to clean up afterward.

While testing this, I also found that the block I'd added earlier (lines ~896-902) is now redundant — it was pushing a separator even though the normal flow already handles adding one correctly once the space isn't pushed in the first place. I removed it.

If that looks right to you, I'll finalize this PR for bug 2 and add a regression test, then move on to bug 1 — focusing on the MIN_STRING and break_string issues.

This feels like it was LLM generated (See slop grenade). I have no idea what you're talking about here. The only code in this PR pops whitespace off and never pushes anything.

@ytmimi
ytmimi removed their request for review August 31, 2026 15:09
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

S-waiting-on-author Status: awaiting some action (such as code changes or more information) from the author.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants