Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Partition wrap_comments into normal or doc settings #5943

Open
wants to merge 4 commits into
base: master
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
12 changes: 4 additions & 8 deletions Configurations.md
Original file line number Diff line number Diff line change
Expand Up @@ -3096,11 +3096,11 @@ Note that no wrapping will happen if:
1. The comment is the start of a markdown header doc comment
2. An URL was found in the comment

- **Default value**: `false`
- **Possible values**: `true`, `false`
- **Default value**: `"off"`
Copy link
Contributor

Choose a reason for hiding this comment

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

nit: Let's stay consistent with the capitalization.

Suggested change
- **Default value**: `"off"`
- **Default value**: `"Off"`

- **Possible values**: `"doc"`, `"normal"`, `"all"` (alias `true`), `"off"` (alias `false`)
ytmimi marked this conversation as resolved.
Show resolved Hide resolved
- **Stable**: No (tracking issue: [#3347](https://github.com/rust-lang/rustfmt/issues/3347))

#### `false` (default):
#### `"off"` (default):
ytmimi marked this conversation as resolved.
Show resolved Hide resolved

```rust
// Lorem ipsum dolor sit amet, consectetur adipiscing elit,
Expand All @@ -3112,12 +3112,10 @@ Note that no wrapping will happen if:
// Lorem ipsum dolor sit amet, consectetur adipiscing elit, sed do eiusmod tempor incididunt ut labore et dolore magna aliqua. Ut enim ad minim veniam, quis nostrud exercitation ullamco laboris nisi ut aliquip ex ea commodo consequat.

// Information on the lorem ipsum can be found at the following url: https://en.wikipedia.org/wiki/Lorem_ipsum. Its text is: lorem ipsum dolor sit amet, consectetur adipiscing elit, sed do eiusmod tempor incididunt ut labore et dolore magna aliqua. Ut enim ad minim veniam, quis nostrud exercitation ullamco laboris nisi ut aliquip ex ea commodo consequat.

/// # This doc comment is a very long header (it starts with a '#'). Had it not been a header it would have been wrapped. But because it is a header, it will not be. That is because wrapping a markdown header breaks it.
Copy link
Contributor

Choose a reason for hiding this comment

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

I'm fine with removing this from the example code, but I'd like to preserve this information by adding it to the note above. I'm thinking something like this:

Break comments to fit on the line

Note that no wrapping will happen if:
1. The comment is the start of a markdown header doc comment. For example:
   ```rust
   /// # This doc comment is a markdown header
   ```
3. An URL was found in the comment

struct Foo {}
```

#### `true`:
#### `"all"`:

```rust
// Lorem ipsum dolor sit amet, consectetur adipiscing elit,
Expand All @@ -3133,8 +3131,6 @@ struct Foo {}
// commodo consequat.

// Information on the lorem ipsum can be found at the following url: https://en.wikipedia.org/wiki/Lorem_ipsum. Its text is: lorem ipsum dolor sit amet, consectetur adipiscing elit, sed do eiusmod tempor incididunt ut labore et dolore magna aliqua. Ut enim ad minim veniam, quis nostrud exercitation ullamco laboris nisi ut aliquip ex ea commodo consequat.

/// # This doc comment is a very long header (it starts with a '#'). Had it not been a header it would have been wrapped. But because it is a header, it will not be. That is because wrapping a markdown header breaks it.
struct Foo {}
```

Expand Down
31 changes: 18 additions & 13 deletions src/comment.rs
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,7 @@ use lazy_static::lazy_static;
use regex::Regex;
use rustc_span::Span;

use crate::config::Config;
use crate::config::{Config, WrapComments};
use crate::rewrite::RewriteContext;
use crate::shape::{Indent, Shape};
use crate::string::{rewrite_string, StringFormat};
Expand Down Expand Up @@ -361,12 +361,11 @@ fn identify_comment(
if !config.normalize_comments() && has_bare_lines && style.is_block_comment() {
trim_left_preserve_layout(first_group, shape.indent, config)?
} else if !config.normalize_comments()
&& !config.wrap_comments()
&& !(
// `format_code_in_doc_comments` should only take effect on doc comments,
// so we only consider it when this comment block is a doc comment block.
is_doc_comment && config.format_code_in_doc_comments()
)
&& (if is_doc_comment {
!config.wrap_comments().is_doc() && !config.format_code_in_doc_comments()
} else {
!config.wrap_comments().is_normal()
})
{
light_rewrite_comment(first_group, shape.indent, config, is_doc_comment)
} else {
Expand Down Expand Up @@ -770,7 +769,7 @@ impl<'a> CommentRewrite<'a> {
&& !self.code_block_buffer.trim().is_empty() =>
{
let mut config = self.fmt.config.clone();
config.set().wrap_comments(false);
config.set().wrap_comments(WrapComments::Off);
let comment_max_width = config
.doc_comment_code_block_width()
.min(config.max_width());
Expand Down Expand Up @@ -802,11 +801,17 @@ impl<'a> CommentRewrite<'a> {
return false;
}

let config_wrap_comments = if is_doc_comment {
self.fmt.config.wrap_comments().is_doc()
} else {
self.fmt.config.wrap_comments().is_normal()
};

self.code_block_attr = None;
self.item_block = None;
if let Some(stripped) = line.strip_prefix("```") {
self.code_block_attr = Some(CodeBlockAttribute::new(stripped))
} else if self.fmt.config.wrap_comments() {
} else if config_wrap_comments {
if let Some(ib) = ItemizedBlock::new(line) {
self.item_block = Some(ib);
return false;
Expand Down Expand Up @@ -845,7 +850,7 @@ impl<'a> CommentRewrite<'a> {
// 4) No URLS were found in the comment
// If this changes, the documentation in ../Configurations.md#wrap_comments
// should be changed accordingly.
let should_wrap_comment = self.fmt.config.wrap_comments()
let should_wrap_comment = config_wrap_comments
&& !is_markdown_header_doc_comment
&& unicode_str_width(line) > self.fmt.shape.width
&& !has_url(line)
Expand Down Expand Up @@ -1903,13 +1908,13 @@ mod test {

#[test]
#[rustfmt::skip]
fn format_doc_comments() {
fn format_comments() {
let mut wrap_normalize_config: crate::config::Config = Default::default();
wrap_normalize_config.set().wrap_comments(true);
wrap_normalize_config.set().wrap_comments(WrapComments::All);
wrap_normalize_config.set().normalize_comments(true);

let mut wrap_config: crate::config::Config = Default::default();
wrap_config.set().wrap_comments(true);
wrap_config.set().wrap_comments(WrapComments::All);

let comment = rewrite_comment(" //test",
true,
Expand Down
4 changes: 2 additions & 2 deletions src/config/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -63,7 +63,7 @@ create_config! {
over multiple lines.";

// Comments. macros, and strings
wrap_comments: bool, false, false, "Break comments to fit on the line";
wrap_comments: WrapComments, WrapComments::Off, false, "Break comments to fit on the line";
format_code_in_doc_comments: bool, false, false, "Format the code snippet in doc comments.";
doc_comment_code_block_width: usize, 100, false, "Maximum width for code snippets in doc \
comments. No effect unless format_code_in_doc_comments = true";
Expand Down Expand Up @@ -634,7 +634,7 @@ array_width = 60
chain_width = 60
single_line_if_else_max_width = 50
single_line_let_else_max_width = 50
wrap_comments = false
wrap_comments = "Off"
format_code_in_doc_comments = false
doc_comment_code_block_width = 100
comment_width = 80
Expand Down
91 changes: 91 additions & 0 deletions src/config/options.rs
Original file line number Diff line number Diff line change
Expand Up @@ -216,6 +216,97 @@ pub enum Verbosity {
Quiet,
}

/// Which comments to wrap
#[derive(Copy, Clone, Debug, Eq, PartialEq, Serialize)]
pub enum WrapComments {
/// Don't wrap comments
Off,
/// Wrap all kinds of comments
All,
/// Only wrap doc comments
Doc,
/// Only wrap normal comments
Normal,
}
ytmimi marked this conversation as resolved.
Show resolved Hide resolved

impl WrapComments {
pub(crate) fn is_normal(self) -> bool {
matches!(self, WrapComments::All | WrapComments::Normal)
}

pub(crate) fn is_doc(self) -> bool {
matches!(self, WrapComments::All | WrapComments::Doc)
}
}

impl fmt::Display for WrapComments {
fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result {
match self {
WrapComments::Off => f.write_str("Off"),
WrapComments::All => f.write_str("All"),
WrapComments::Doc => f.write_str("Doc"),
WrapComments::Normal => f.write_str("Normal"),
}
}
}

impl super::ConfigType for WrapComments {
fn doc_hint() -> String {
"[Off|All|Doc|Normal]".to_owned()
}

fn stable_variant(&self) -> bool {
true
}
}
ytmimi marked this conversation as resolved.
Show resolved Hide resolved

impl std::str::FromStr for WrapComments {
type Err = &'static str;

fn from_str(s: &str) -> Result<Self, Self::Err> {
match s.to_lowercase().as_str() {
"off" | "false" => Ok(WrapComments::Off),
"all" | "true" => Ok(WrapComments::All),
"doc" => Ok(WrapComments::Doc),
"normal" => Ok(WrapComments::Normal),
_ => Err("Bad variant, expected one of: `Off` `All` `Doc` `Normal`"),
Copy link
Contributor

Choose a reason for hiding this comment

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

Do you think It also makes sense to mention true and false here?

}
}
}

impl<'de> serde::de::Deserialize<'de> for WrapComments {
fn deserialize<D>(d: D) -> Result<Self, D::Error>
where
D: serde::Deserializer<'de>,
{
use serde::de::Error;

struct StringOrBoolVisitor;

impl<'de> Visitor<'de> for StringOrBoolVisitor {
type Value = String;

fn expecting(&self, formatter: &mut fmt::Formatter<'_>) -> fmt::Result {
formatter.write_str("string")
}

fn visit_str<E>(self, value: &str) -> Result<String, E> {
Ok(String::from(value))
}

fn visit_bool<E>(self, value: bool) -> Result<String, E> {
Ok(value.to_string())
}
}

let s = d.deserialize_string(StringOrBoolVisitor)?;
s.parse().map_err(|_| {
static ALLOWED: &'static [&str] = &["Off", "All", "Doc", "Normal"];
ytmimi marked this conversation as resolved.
Show resolved Hide resolved
D::Error::unknown_variant(&s, ALLOWED)
})
}
}

#[derive(Deserialize, Serialize, Clone, Debug, PartialEq)]
pub struct WidthHeuristics {
// Maximum width of the args of a function call before falling back
Expand Down
17 changes: 17 additions & 0 deletions tests/source/configs/wrap_comments/all.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,17 @@
// rustfmt-wrap_comments: all
ytmimi marked this conversation as resolved.
Show resolved Hide resolved
// rustfmt-max_width: 50
// Wrap comments

fn main() {
//! Lorem ipsum dolor sit amet, consectetur adipiscing elit, sed do eiusmod tempor incididunt ut labore et dolore magna aliqua. Ut enim ad minim veniam, quis nostrud exercitation ullamco laboris nisi ut aliquip ex ea commodo consequat.
// Lorem ipsum dolor sit amet, consectetur adipiscing elit, sed do eiusmod tempor incididunt ut labore et dolore magna aliqua. Ut enim ad minim veniam, quis nostrud exercitation ullamco laboris nisi ut aliquip ex ea commodo consequat.
}

fn code_block() {
// ```rust
// let x = 3;
//
// println!("x = {}", x);
// ```
}
17 changes: 17 additions & 0 deletions tests/source/configs/wrap_comments/doc.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,17 @@
// rustfmt-wrap_comments: doc
// rustfmt-max_width: 50
/// Wrap comments
fn main() {
//! Lorem ipsum dolor sit amet, consectetur adipiscing elit, sed do eiusmod tempor incididunt ut labore et dolore magna aliqua. Ut enim ad minim veniam, quis nostrud exercitation ullamco laboris nisi ut aliquip ex ea commodo consequat.
// Lorem ipsum dolor sit amet, consectetur adipiscing elit, sed do eiusmod tempor incididunt ut labore et dolore magna aliqua. Ut enim ad minim veniam, quis nostrud exercitation ullamco laboris nisi ut aliquip ex ea commodo consequat.
}

fn code_block() {
// ```rust
// let x = 3;
//
// println!("x = {}", x);
// ```
}
10 changes: 10 additions & 0 deletions tests/source/configs/wrap_comments/false.rs
Original file line number Diff line number Diff line change
Expand Up @@ -4,5 +4,15 @@
// Wrap comments

fn main() {
//! Lorem ipsum dolor sit amet, consectetur adipiscing elit, sed do eiusmod tempor incididunt ut labore et dolore magna aliqua. Ut enim ad minim veniam, quis nostrud exercitation ullamco laboris nisi ut aliquip ex ea commodo consequat.
// Lorem ipsum dolor sit amet, consectetur adipiscing elit, sed do eiusmod tempor incididunt ut labore et dolore magna aliqua. Ut enim ad minim veniam, quis nostrud exercitation ullamco laboris nisi ut aliquip ex ea commodo consequat.
}

fn code_block() {
//! ```rust
//! let x = 3;
//!
//! println!("x = {}", x);
//! ```
}
18 changes: 18 additions & 0 deletions tests/source/configs/wrap_comments/normal.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,18 @@
// rustfmt-wrap_comments: normal
// rustfmt-max_width: 50
// rustfmt-error_on_line_overflow: false
// Wrap comments

fn main() {
//! Lorem ipsum dolor sit amet, consectetur adipiscing elit, sed do eiusmod tempor incididunt ut labore et dolore magna aliqua. Ut enim ad minim veniam, quis nostrud exercitation ullamco laboris nisi ut aliquip ex ea commodo consequat.
// Lorem ipsum dolor sit amet, consectetur adipiscing elit, sed do eiusmod tempor incididunt ut labore et dolore magna aliqua. Ut enim ad minim veniam, quis nostrud exercitation ullamco laboris nisi ut aliquip ex ea commodo consequat.
}

fn code_block() {
//! ```rust
//! let x = 3;
//!
//! println!("x = {}", x);
//! ```
}
18 changes: 18 additions & 0 deletions tests/source/configs/wrap_comments/off.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,18 @@
// rustfmt-wrap_comments: off
// rustfmt-max_width: 50
// rustfmt-error_on_line_overflow: false
// Wrap comments

fn main() {
//! Lorem ipsum dolor sit amet, consectetur adipiscing elit, sed do eiusmod tempor incididunt ut labore et dolore magna aliqua. Ut enim ad minim veniam, quis nostrud exercitation ullamco laboris nisi ut aliquip ex ea commodo consequat.
// Lorem ipsum dolor sit amet, consectetur adipiscing elit, sed do eiusmod tempor incididunt ut labore et dolore magna aliqua. Ut enim ad minim veniam, quis nostrud exercitation ullamco laboris nisi ut aliquip ex ea commodo consequat.
}

fn code_block() {
//! ```rust
//! let x = 3;
//!
//! println!("x = {}", x);
//! ```
}
2 changes: 2 additions & 0 deletions tests/source/configs/wrap_comments/true.rs
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,8 @@
// Wrap comments

fn main() {
//! Lorem ipsum dolor sit amet, consectetur adipiscing elit, sed do eiusmod tempor incididunt ut labore et dolore magna aliqua. Ut enim ad minim veniam, quis nostrud exercitation ullamco laboris nisi ut aliquip ex ea commodo consequat.
// Lorem ipsum dolor sit amet, consectetur adipiscing elit, sed do eiusmod tempor incididunt ut labore et dolore magna aliqua. Ut enim ad minim veniam, quis nostrud exercitation ullamco laboris nisi ut aliquip ex ea commodo consequat.
}

Expand Down
27 changes: 27 additions & 0 deletions tests/target/configs/wrap_comments/all.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,27 @@
// rustfmt-wrap_comments: all
// rustfmt-max_width: 50
// Wrap comments

fn main() {
//! Lorem ipsum dolor sit amet, consectetur
//! adipiscing elit, sed do eiusmod tempor
//! incididunt ut labore et dolore magna
//! aliqua. Ut enim ad minim veniam, quis
//! nostrud exercitation ullamco laboris nisi
//! ut aliquip ex ea commodo consequat.

// Lorem ipsum dolor sit amet, consectetur
// adipiscing elit, sed do eiusmod tempor
// incididunt ut labore et dolore magna
// aliqua. Ut enim ad minim veniam, quis
// nostrud exercitation ullamco laboris nisi
// ut aliquip ex ea commodo consequat.
}

fn code_block() {
// ```rust
// let x = 3;
//
// println!("x = {}", x);
// ```
}
22 changes: 22 additions & 0 deletions tests/target/configs/wrap_comments/doc.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,22 @@
// rustfmt-wrap_comments: doc
// rustfmt-max_width: 50
/// Wrap comments

fn main() {
//! Lorem ipsum dolor sit amet, consectetur
//! adipiscing elit, sed do eiusmod tempor
//! incididunt ut labore et dolore magna
//! aliqua. Ut enim ad minim veniam, quis
//! nostrud exercitation ullamco laboris nisi
//! ut aliquip ex ea commodo consequat.

// Lorem ipsum dolor sit amet, consectetur adipiscing elit, sed do eiusmod tempor incididunt ut labore et dolore magna aliqua. Ut enim ad minim veniam, quis nostrud exercitation ullamco laboris nisi ut aliquip ex ea commodo consequat.
}

fn code_block() {
// ```rust
// let x = 3;
//
// println!("x = {}", x);
// ```
}
Loading
Loading