Skip to content

Commit

Permalink
do not force comments to be indented with a comment trailing a line o…
Browse files Browse the repository at this point in the history
…f code (rust-lang#3833)
  • Loading branch information
scampi authored and topecongiro committed Oct 4, 2019
1 parent 7926851 commit fb01dc8
Show file tree
Hide file tree
Showing 5 changed files with 150 additions and 29 deletions.
42 changes: 38 additions & 4 deletions src/missed_spans.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand Down Expand Up @@ -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 {
Expand All @@ -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))
Expand All @@ -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();
Expand Down
13 changes: 10 additions & 3 deletions src/utils.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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') {
Expand Down
73 changes: 51 additions & 22 deletions src/visitor.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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::{
Expand Down Expand Up @@ -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) => {
Expand Down
21 changes: 21 additions & 0 deletions tests/source/trailing_comments.rs
Original file line number Diff line number Diff line change
@@ -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].
}
30 changes: 30 additions & 0 deletions tests/target/trailing_comments.rs
Original file line number Diff line number Diff line change
@@ -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].
}

0 comments on commit fb01dc8

Please sign in to comment.