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.

Sign up for GitHub

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

do not force comments to be indented with a comment trailing a line of code #3833

Merged
merged 1 commit into from
Oct 4, 2019
Merged

do not force comments to be indented with a comment trailing a line of code #3833

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
42 changes: 38 additions & 4 deletions src/missed_spans.rs
View file
Open in desktop
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
View file
Open in desktop
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
View file
Open in desktop
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
View file
Open in desktop
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
View file
Open in desktop
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].
}