From fb01dc857c2c073eedfa02ce37f6189a9972f838 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?St=C3=A9phane=20Campinas?= Date: Fri, 4 Oct 2019 17:22:01 +0200 Subject: [PATCH] do not force comments to be indented with a comment trailing a line of code (#3833) --- src/missed_spans.rs | 42 ++++++++++++++++-- src/utils.rs | 13 ++++-- src/visitor.rs | 73 +++++++++++++++++++++---------- tests/source/trailing_comments.rs | 21 +++++++++ tests/target/trailing_comments.rs | 30 +++++++++++++ 5 files changed, 150 insertions(+), 29 deletions(-) create mode 100644 tests/source/trailing_comments.rs create mode 100644 tests/target/trailing_comments.rs diff --git a/src/missed_spans.rs b/src/missed_spans.rs index 7bc81fdc844..a6800b15e61 100644 --- a/src/missed_spans.rs +++ b/src/missed_spans.rs @@ -3,10 +3,11 @@ use syntax::source_map::{BytePos, Pos, Span}; use crate::comment::{is_last_comment_block, rewrite_comment, CodeCharKind, CommentCodeSlices}; use crate::config::file_lines::FileLines; use crate::config::FileName; +use crate::config::Version; use crate::coverage::transform_missing_snippet; use crate::shape::{Indent, Shape}; use crate::source_map::LineRangeUtils; -use crate::utils::{count_lf_crlf, count_newlines, last_line_width, mk_sp}; +use crate::utils::{count_lf_crlf, count_newlines, last_line_indent, last_line_width, mk_sp}; use crate::visitor::FmtVisitor; struct SnippetStatus { @@ -238,6 +239,7 @@ impl<'a> FmtVisitor<'a> { .next(); let fix_indent = last_char.map_or(true, |rev_c| ['{', '\n'].contains(&rev_c)); + let mut on_same_line = false; let comment_indent = if fix_indent { if let Some('{') = last_char { @@ -246,6 +248,13 @@ impl<'a> FmtVisitor<'a> { let indent_str = self.block_indent.to_string(self.config); self.push_str(&indent_str); self.block_indent + } else if self.config.version() == Version::Two && !snippet.starts_with('\n') { + // The comment appears on the same line as the previous formatted code. + // Assuming that comment is logically associated with that code, we want to keep it on + // the same level and avoid mixing it with possible other comment. + on_same_line = true; + self.push_str(" "); + Indent::from_width(self.config, last_line_indent(&self.buffer)) } else { self.push_str(" "); Indent::from_width(self.config, last_line_width(&self.buffer)) @@ -256,9 +265,34 @@ impl<'a> FmtVisitor<'a> { self.config.max_width() - self.block_indent.width(), ); let comment_shape = Shape::legacy(comment_width, comment_indent); - let comment_str = rewrite_comment(subslice, false, comment_shape, self.config) - .unwrap_or_else(|| String::from(subslice)); - self.push_str(&comment_str); + + if on_same_line { + match subslice.find("\n") { + None => { + self.push_str(subslice); + } + Some(offset) if offset + 1 == subslice.len() => { + self.push_str(&subslice[..offset]); + } + Some(offset) => { + // keep first line as is: if it were too long and wrapped, it may get mixed + // with the other lines. + let first_line = &subslice[..offset]; + self.push_str(first_line); + self.push_str(&comment_indent.to_string_with_newline(self.config)); + + let other_lines = &subslice[offset + 1..]; + let comment_str = + rewrite_comment(other_lines, false, comment_shape, self.config) + .unwrap_or_else(|| String::from(other_lines)); + self.push_str(&comment_str); + } + } + } else { + let comment_str = rewrite_comment(subslice, false, comment_shape, self.config) + .unwrap_or_else(|| String::from(subslice)); + self.push_str(&comment_str); + } status.last_wspace = None; status.line_start = offset + subslice.len(); diff --git a/src/utils.rs b/src/utils.rs index bad9bc1d788..76af95a8c7f 100644 --- a/src/utils.rs +++ b/src/utils.rs @@ -193,19 +193,26 @@ pub(crate) fn is_attributes_extendable(attrs_str: &str) -> bool { !attrs_str.contains('\n') && !last_line_contains_single_line_comment(attrs_str) } -// The width of the first line in s. +/// The width of the first line in s. #[inline] pub(crate) fn first_line_width(s: &str) -> usize { unicode_str_width(s.splitn(2, '\n').next().unwrap_or("")) } -// The width of the last line in s. +/// The width of the last line in s. #[inline] pub(crate) fn last_line_width(s: &str) -> usize { unicode_str_width(s.rsplitn(2, '\n').next().unwrap_or("")) } -// The total used width of the last line. +/// The indent width of the last line in s. +#[inline] +pub(crate) fn last_line_indent(s: &str) -> usize { + let last_line = s.rsplitn(2, '\n').next().unwrap_or(""); + last_line.chars().take_while(|c| c.is_whitespace()).count() +} + +/// The total used width of the last line. #[inline] pub(crate) fn last_line_used_width(s: &str, offset: usize) -> usize { if s.contains('\n') { diff --git a/src/visitor.rs b/src/visitor.rs index 49197682c14..8a1b3cd71fe 100644 --- a/src/visitor.rs +++ b/src/visitor.rs @@ -7,6 +7,7 @@ use syntax::{ast, visit}; use crate::attr::*; use crate::comment::{rewrite_comment, CodeCharKind, CommentCodeSlices}; +use crate::config::Version; use crate::config::{BraceStyle, Config}; use crate::coverage::transform_missing_snippet; use crate::items::{ @@ -252,32 +253,60 @@ impl<'b, 'a: 'b> FmtVisitor<'a> { let mut comment_shape = Shape::indented(self.block_indent, config).comment(config); - if comment_on_same_line { - // 1 = a space before `//` - let offset_len = 1 + last_line_width(&self.buffer) - .saturating_sub(self.block_indent.width()); - match comment_shape - .visual_indent(offset_len) - .sub_width(offset_len) - { - Some(shp) => comment_shape = shp, - None => comment_on_same_line = false, - } - }; - - if comment_on_same_line { + if self.config.version() == Version::Two && comment_on_same_line { self.push_str(" "); + // put the first line of the comment on the same line as the + // block's last line + match sub_slice.find("\n") { + None => { + self.push_str(&sub_slice); + } + Some(offset) if offset + 1 == sub_slice.len() => { + self.push_str(&sub_slice[..offset]); + } + Some(offset) => { + let first_line = &sub_slice[..offset]; + self.push_str(first_line); + self.push_str(&self.block_indent.to_string_with_newline(config)); + + // put the other lines below it, shaping it as needed + let other_lines = &sub_slice[offset + 1..]; + let comment_str = + rewrite_comment(other_lines, false, comment_shape, config); + match comment_str { + Some(ref s) => self.push_str(s), + None => self.push_str(other_lines), + } + } + } } else { - if count_newlines(snippet_in_between) >= 2 || extra_newline { - self.push_str("\n"); + if comment_on_same_line { + // 1 = a space before `//` + let offset_len = 1 + last_line_width(&self.buffer) + .saturating_sub(self.block_indent.width()); + match comment_shape + .visual_indent(offset_len) + .sub_width(offset_len) + { + Some(shp) => comment_shape = shp, + None => comment_on_same_line = false, + } + }; + + if comment_on_same_line { + self.push_str(" "); + } else { + if count_newlines(snippet_in_between) >= 2 || extra_newline { + self.push_str("\n"); + } + self.push_str(&self.block_indent.to_string_with_newline(config)); } - self.push_str(&self.block_indent.to_string_with_newline(config)); - } - let comment_str = rewrite_comment(&sub_slice, false, comment_shape, config); - match comment_str { - Some(ref s) => self.push_str(s), - None => self.push_str(&sub_slice), + let comment_str = rewrite_comment(&sub_slice, false, comment_shape, config); + match comment_str { + Some(ref s) => self.push_str(s), + None => self.push_str(&sub_slice), + } } } CodeCharKind::Normal if skip_normal(&sub_slice) => { diff --git a/tests/source/trailing_comments.rs b/tests/source/trailing_comments.rs new file mode 100644 index 00000000000..7845f713b8a --- /dev/null +++ b/tests/source/trailing_comments.rs @@ -0,0 +1,21 @@ +// rustfmt-version: Two +// rustfmt-wrap_comments: true + +pub const IFF_MULTICAST: ::c_int = 0x0000000800; // Supports multicast +// Multicast using broadcst. add. + +pub const SQ_CRETAB: u16 = 0x000e; // CREATE TABLE +pub const SQ_DRPTAB: u16 = 0x000f; // DROP TABLE +pub const SQ_CREIDX: u16 = 0x0010; // CREATE INDEX +//const SQ_DRPIDX: u16 = 0x0011; // DROP INDEX +//const SQ_GRANT: u16 = 0x0012; // GRANT +//const SQ_REVOKE: u16 = 0x0013; // REVOKE + +fn foo() { + let f = bar(); // Donec consequat mi. Quisque vitae dolor. Integer lobortis. Maecenas id nulla. Lorem. + // Id turpis. Nam posuere lectus vitae nibh. Etiam tortor orci, sagittis malesuada, rhoncus quis, hendrerit eget, libero. Quisque commodo nulla at nunc. Mauris consequat, enim vitae venenatis sollicitudin, dolor orci bibendum enim, a sagittis nulla nunc quis elit. Phasellus augue. Nunc suscipit, magna tincidunt lacinia faucibus, lacus tellus ornare purus, a pulvinar lacus orci eget nibh. Maecenas sed nibh non lacus tempor faucibus. In hac habitasse platea dictumst. Vivamus a orci at nulla tristique condimentum. Donec arcu quam, dictum accumsan, convallis accumsan, cursus sit amet, ipsum. In pharetra sagittis nunc. + let b = baz(); + + let normalized = self.ctfont.all_traits().normalized_weight(); // [-1.0, 1.0] + // TODO(emilio): It may make sense to make this range [.01, 10.0], to align with css-fonts-4's range of [1, 1000]. +} diff --git a/tests/target/trailing_comments.rs b/tests/target/trailing_comments.rs new file mode 100644 index 00000000000..eba943042ad --- /dev/null +++ b/tests/target/trailing_comments.rs @@ -0,0 +1,30 @@ +// rustfmt-version: Two +// rustfmt-wrap_comments: true + +pub const IFF_MULTICAST: ::c_int = 0x0000000800; // Supports multicast +// Multicast using broadcst. add. + +pub const SQ_CRETAB: u16 = 0x000e; // CREATE TABLE +pub const SQ_DRPTAB: u16 = 0x000f; // DROP TABLE +pub const SQ_CREIDX: u16 = 0x0010; // CREATE INDEX +//const SQ_DRPIDX: u16 = 0x0011; // DROP INDEX +//const SQ_GRANT: u16 = 0x0012; // GRANT +//const SQ_REVOKE: u16 = 0x0013; // REVOKE + +fn foo() { + let f = bar(); // Donec consequat mi. Quisque vitae dolor. Integer lobortis. Maecenas id nulla. Lorem. + // Id turpis. Nam posuere lectus vitae nibh. Etiam tortor orci, sagittis + // malesuada, rhoncus quis, hendrerit eget, libero. Quisque commodo nulla at + // nunc. Mauris consequat, enim vitae venenatis sollicitudin, dolor orci + // bibendum enim, a sagittis nulla nunc quis elit. Phasellus augue. Nunc + // suscipit, magna tincidunt lacinia faucibus, lacus tellus ornare purus, a + // pulvinar lacus orci eget nibh. Maecenas sed nibh non lacus tempor faucibus. + // In hac habitasse platea dictumst. Vivamus a orci at nulla tristique + // condimentum. Donec arcu quam, dictum accumsan, convallis accumsan, cursus sit + // amet, ipsum. In pharetra sagittis nunc. + let b = baz(); + + let normalized = self.ctfont.all_traits().normalized_weight(); // [-1.0, 1.0] + // TODO(emilio): It may make sense to make this range [.01, 10.0], to align + // with css-fonts-4's range of [1, 1000]. +}