Regression with wrap_comments=true

Hi all,

I've noticed a regression that happened between rustfmt 1.5.2-nightly (fe7454b 2023-06-19) and rustfmt 1.5.3-nightly (4651421 2023-06-20).

In my project [1], whenever I run cargo fmt, some comment lines are mangled unexpectedly. Here's the full diff: https://pastebin.com/raw/HwqgrCUz

If I disable wrap_comments=true in rustfmt.toml, the problem no longer occurs.

Interestingly, I couldn't reproduce the problem using a small demonstration program. It only happens when using rustfmt or cargo fmt in my project (which has around 75k LoC).

This is similar to #5835, but not quite the same problem.

Please let me know if you need more info to look into this.

[1] https://github.com/rwestphal/holo

~~This looks exactly the same as #5835~~ Oh, it's not. . is due to numbered list, but why + is also affected 🤔️

It seems #5423 also changed how unordered list works

In theory your graph contains markdown lists. You can change it to table characters or wrap it in "```", which will also make hovering doc nicer.

/// ```text
/// +-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+
/// |                                                               |
/// +                                                               +
/// |                     Mandatory Parameters                      |
/// +                                                               +
/// |                                                               |
/// +-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+
/// ``` 
fn a() {}

But you used comment // instead of doc comment ///. Not sure whether it's reasonable to assume // is markdown 🤔️

Rust style guide doesn't mention Markdown

https://doc.rust-lang.org/nightly/style-guide/index.html#comments

Rustdoc says it uses Markdown

https://doc.rust-lang.org/rustdoc/how-to-write-documentation.html?highlight=markdown#markdown

@xxchan

@xxchan thanks for helping to look into this and for providing a workaround for the issue.

@rwestphal Thanks for the report. Confirming I can reproduce this with rustfmt 1.6.0-nightly (a9ae7462 2023-07-19). Here's a minimal reproducible example.

input

// +-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+
// |                                                               |
// +                                                               +
// |                     Mandatory Parameters                      |
// +                                                               +
// |                                                               |
// +-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+

output (using wrap_comments=true):

// +-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+
// |                                                               |
// + +
// |                     Mandatory Parameters                      |
// + +
// |                                                               |
// +-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+

Basically #5423 will remove all spaces after item markers (+, 1., etc.), so I cannot tell it's a bug or feature, unlike #5835 which is definitely a bug.

#5836 is somewhat similar. i.e., the style of how to format a markdown list is controversial..

But you used comment // instead of doc comment ///. Not sure whether it's reasonable to assume // is markdown 🤔️

~~I believe we're incorrectly calling code that goes down the doc comment rewrite path, even when we're rewriting a normal comment. #5536 should solve the issue highlighted in #5862 (comment)~~

On further reflection I don't think that's the case.

@xxchan

@xxchan Do you have any interest in workin on the? Part of me thinks that the solution might be as simple as wrapping this in a check for is_doc_comment so we don't try to parse out code blocks or lists if we're not in a doc comment.

rustfmt/src/comment.rs

Lines 806 to 813 in f89cd3c

    
           if let Some(stripped) = line.strip_prefix("```") { 
        
               self.code_block_attr = Some(CodeBlockAttribute::new(stripped)) 
        
           } else if self.fmt.config.wrap_comments() { 
        
               if let Some(ib) = ItemizedBlock::new(line) { 
        
                   self.item_block = Some(ib); 
        
                   return false; 
        
               } 
        
           }

ci: add PR code formatting check

Navigation Menu

Repository navigation

Regression with `wrap_comments=true` #5862

Metadata

Assignees

Labels

Type

Projects

Milestone

Relationships

Development

	if let Some(stripped) = line.strip_prefix("```") {
	self.code_block_attr = Some(CodeBlockAttribute::new(stripped))
	} else if self.fmt.config.wrap_comments() {
	if let Some(ib) = ItemizedBlock::new(line) {
	self.item_block = Some(ib);
	return false;
	}
	}

Regression with wrap_comments=true #5862

Description

Activity

xxchan commented on Jul 29, 2023

xxchan commented on Jul 29, 2023

xxchan commented on Jul 29, 2023

xxchan commented on Jul 29, 2023

ytmimi commented on Aug 1, 2023

xxchan commented on Aug 4, 2023

ytmimi commented on Aug 25, 2023

ytmimi commented on Aug 30, 2023

Metadata

Metadata

Assignees

Labels

Type

Projects

Milestone

Relationships

Development

Issue actions

Regression with `wrap_comments=true` #5862