Merge pull request #2102 from topecongiro/soft-wrapping-comments

Soft wrapping for comments

src/comment.rs

@@ -287,12 +287,13 @@ fn rewrite_comment_inner(
         .checked_sub(closer.len() + opener.len())
         .unwrap_or(1);
     let indent_str = shape.indent.to_string(config);
-    let fmt = StringFormat {
+    let fmt_indent = shape.indent + (opener.len() - line_start.len());
+    let mut fmt = StringFormat {
         opener: "",
         closer: "",
         line_start: line_start,
         line_end: "",
-        shape: Shape::legacy(max_chars, shape.indent + (opener.len() - line_start.len())),
+        shape: Shape::legacy(max_chars, fmt_indent),
         trim_end: true,
         config: config,
     };
@@ -317,26 +318,69 @@ fn rewrite_comment_inner(
         });
 
     let mut result = opener.to_owned();
+    let mut is_prev_line_multi_line = false;
+    let comment_line_separator = format!("\n{}{}", indent_str, line_start);
     for line in lines {
         if result == opener {
             if line.is_empty() {
                 continue;
             }
         } else {
-            result.push('\n');
-            result.push_str(&indent_str);
-            result.push_str(line_start);
+            if is_prev_line_multi_line && !line.is_empty() {
+                result.push(' ')
+            } else {
+                result.push_str(&comment_line_separator);
+            }
         }
 
-        if config.wrap_comments() && line.len() > max_chars {
-            let rewrite = rewrite_string(line, &fmt).unwrap_or_else(|| line.to_owned());
-            result.push_str(&rewrite);
+        if config.wrap_comments() && line.len() > fmt.shape.width && !has_url(line) {
+            match rewrite_string(line, &fmt, Some(max_chars)) {
+                Some(ref s) => {
+                    is_prev_line_multi_line = s.contains('\n');
+                    result.push_str(s);
+                }
+                None if is_prev_line_multi_line => {
+                    // We failed to put the current `line` next to the previous `line`.
+                    // Remove the trailing space, then start rewrite on the next line.
+                    result.pop();
+                    result.push_str(&comment_line_separator);
+                    fmt.shape = Shape::legacy(max_chars, fmt_indent);
+                    match rewrite_string(line, &fmt, Some(max_chars)) {
+                        Some(ref s) => {
+                            is_prev_line_multi_line = s.contains('\n');
+                            result.push_str(s);
+                        }
+                        None => {
+                            is_prev_line_multi_line = false;
+                            result.push_str(line);
+                        }
+                    }
+                }
+                None => {
+                    is_prev_line_multi_line = false;
+                    result.push_str(line);
+                }
+            }
+
+            fmt.shape = if is_prev_line_multi_line {
+                // 1 = " "
+                let offset = 1 + last_line_width(&result) - line_start.len();
+                Shape {
+                    width: max_chars.checked_sub(offset).unwrap_or(0),
+                    indent: fmt_indent,
+                    offset: fmt.shape.offset + offset,
+                }
+            } else {
+                Shape::legacy(max_chars, fmt_indent)
+            };
         } else {
             if line.is_empty() && result.ends_with(' ') {
                 // Remove space if this is an empty comment or a doc comment.
                 result.pop();
             }
             result.push_str(line);
+            fmt.shape = Shape::legacy(max_chars, fmt_indent);
+            is_prev_line_multi_line = false;
         }
     }
 
@@ -349,6 +393,12 @@ fn rewrite_comment_inner(
     Some(result)
 }
 
+/// Returns true if the given string MAY include URLs or alike.
+fn has_url(s: &str) -> bool {
+    // This function may return false positive, but should get its job done in most cases.
+    s.contains("https://") || s.contains("http://") || s.contains("ftp://") || s.contains("file://")
+}
+
 /// Given the span, rewrite the missing comment inside it if available.
 /// Note that the given span must only include comments (or leading/trailing whitespaces).
 pub fn rewrite_missing_comment(

src/expr.rs

@@ -1955,7 +1955,11 @@ fn rewrite_string_lit(context: &RewriteContext, span: Span, shape: Shape) -> Opt
     // Remove the quote characters.
     let str_lit = &string_lit[1..string_lit.len() - 1];
 
-    rewrite_string(str_lit, &StringFormat::new(shape, context.config))
+    rewrite_string(
+        str_lit,
+        &StringFormat::new(shape.visual_indent(0), context.config),
+        None,
+    )
 }
 
 fn string_requires_rewrite(

src/string.rs

@@ -44,13 +44,17 @@ impl<'a> StringFormat<'a> {
 }
 
 // FIXME: simplify this!
-pub fn rewrite_string<'a>(orig: &str, fmt: &StringFormat<'a>) -> Option<String> {
+pub fn rewrite_string<'a>(
+    orig: &str,
+    fmt: &StringFormat<'a>,
+    max_width: Option<usize>,
+) -> Option<String> {
     // Strip line breaks.
     let re = Regex::new(r"([^\\](\\\\)*)\\[\n\r][[:space:]]*").unwrap();
     let stripped_str = re.replace_all(orig, "$1");
 
     let graphemes = UnicodeSegmentation::graphemes(&*stripped_str, false).collect::<Vec<&str>>();
-    let shape = fmt.shape.visual_indent(0);
+    let shape = fmt.shape;
     let indent = shape.indent.to_string(fmt.config);
     let punctuation = ":,;.";
 
@@ -67,7 +71,7 @@ pub fn rewrite_string<'a>(orig: &str, fmt: &StringFormat<'a>) -> Option<String>
     let ender_length = fmt.line_end.len();
     // If we cannot put at least a single character per line, the rewrite won't
     // succeed.
-    let max_chars = shape
+    let mut max_chars = shape
         .width
         .checked_sub(fmt.opener.len() + ender_length + 1)? + 1;
 
@@ -135,6 +139,10 @@ pub fn rewrite_string<'a>(orig: &str, fmt: &StringFormat<'a>) -> Option<String>
 
         // The next line starts where the current line ends.
         cur_start = cur_end;
+
+        if let Some(new_max_chars) = max_width {
+            max_chars = new_max_chars.checked_sub(fmt.opener.len() + ender_length + 1)? + 1;
+        }
     }
 
     result.push_str(fmt.closer);
@@ -150,6 +158,6 @@ mod test {
     fn issue343() {
         let config = Default::default();
         let fmt = StringFormat::new(Shape::legacy(2, Indent::empty()), &config);
-        rewrite_string("eq_", &fmt);
+        rewrite_string("eq_", &fmt, None);
     }
 }

tests/source/soft-wrapping.rs

@@ -0,0 +1,9 @@
+// rustfmt-wrap_comments: true
+// rustfmt-max_width: 80
+// Soft wrapping for comments.
+
+// #535, soft wrapping for comments
+// Compare the lowest `f32` of both inputs for greater than or equal. The
+// lowest 32 bits of the result will be `0xffffffff` if `a.extract(0)` is
+// ggreater than or equal `b.extract(0)`, or `0` otherwise. The upper 96 bits off
+// the result are the upper 96 bits of `a`.

tests/target/soft-wrapping.rs

@@ -0,0 +1,9 @@
+// rustfmt-wrap_comments: true
+// rustfmt-max_width: 80
+// Soft wrapping for comments.
+
+// #535, soft wrapping for comments
+// Compare the lowest `f32` of both inputs for greater than or equal. The
+// lowest 32 bits of the result will be `0xffffffff` if `a.extract(0)` is
+// ggreater than or equal `b.extract(0)`, or `0` otherwise. The upper 96 bits
+// off the result are the upper 96 bits of `a`.