Repository navigation
Fix statement comment wrapping - #7170
Merged
Merged
Conversation
This naming was a bit confusing: I would expect a 'line start' of a
comment to include any leading indent etc., e.g. for the comments in:
// top level comment
fn f() {
// function comment
...
}
I would expect:
* `line_start`: `// `, and
* `line_start`: ` // `
Rather than both to be `// `.
By excluding the width of the indent when combining comments, since
`last_line_width` includes this. Otherwise e.g. when formatting:
V------V (indent) (end of line) ---V
// some comment with final word
// new line fits word
We'd would compute the offset using:
last_line_width = " // word".len()`
(i.e. indent+comment prefix+content), then subtract only the width of
the comment prefix (and `" "`) and incorrectly conclude that won't fit
if combined with the following comment.
References: rust-lang#7167
rustbot
added
the
S-waiting-on-author
matthewhughes934
marked this pull request as ready for review
October 10, 2026 08:10
rustbot
added
S-waiting-on-review
jieyouxu
added
UO-wrap_comments
jieyouxu
reviewed
Oct 13, 2026
Comment on lines
+886
to
+887
| - self.comment_prefix.len(); | ||
| - self.comment_prefix.len() | ||
| - self.fmt_indent.width(); |
Member
There was a problem hiding this comment.
Could we use saturing subtraction here? I suspect some kind of weird issue like tabs spaces can cause us to underflow here
Comment on lines
+5
to
+6
| // Note: off-by-one bug: the 100 width wide line should not be formatted | ||
| // https://github.com/rust-lang/rustfmt/issues/7167 |
Member
There was a problem hiding this comment.
Maybe just,
// FIXME(#7167): off-by-one bug: the 100 width wide line should not be formatted
jieyouxu
approved these changes
Oct 13, 2026
Member
There was a problem hiding this comment.
Thanks, just #7170 (comment), feel free to merge otherwise.
EDIT: maybe hold and rebase after #7165 since that is extremely conflict-prone.
ytmimi
approved these changes
Oct 13, 2026
Contributor
|
@matthewhughes934 thanks for working on this and @jieyouxu thanks for reviewing. I should have time to update #7165 later today, but I think I need to rebase that anyway so let's get this one merged first. |
rustbot
added
release-notes
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
4 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Rename
line_starttocomment_prefixThis naming was a bit confusing: I would expect a 'line start' of a
comment to include any leading indent etc., e.g. for the comments in:
I would expect:
line_start://, andline_start://Rather than both to be
//.Fix comments not taking full width when combined
By excluding the width of the indent when combining comments, since
last_line_widthincludes this. Otherwise e.g. when formatting:We'd would compute the offset using:
(i.e. indent+comment prefix+content), then subtract only the width of
the comment prefix (and
" ") and incorrectly conclude that won't fitif combined with the following comment.
References: rustfmt gives legal length comments a ragged edge in argument lists #7167
I did not use an LLM to create a change in this PR.
I used an LLM to create a change in this PR, and I have explained below how it was used.