Skip to content

Commit fb01dc8

Browse files
scampitopecongiro
scampi
authored and
topecongiro
committed
do not force comments to be indented with a comment trailing a line of code (#3833)
1 parent 7926851 commit fb01dc8

File tree

5 files changed

+150
-29
lines changed

5 files changed

+150
-29
lines changed

‎src/missed_spans.rs‎

Lines changed: 38 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -3,10 +3,11 @@ use syntax::source_map::{BytePos, Pos, Span};
33
use crate::comment::{is_last_comment_block, rewrite_comment, CodeCharKind, CommentCodeSlices};
44
use crate::config::file_lines::FileLines;
55
use crate::config::FileName;
6+
use crate::config::Version;
67
use crate::coverage::transform_missing_snippet;
78
use crate::shape::{Indent, Shape};
89
use crate::source_map::LineRangeUtils;
9-
use crate::utils::{count_lf_crlf, count_newlines, last_line_width, mk_sp};
10+
use crate::utils::{count_lf_crlf, count_newlines, last_line_indent, last_line_width, mk_sp};
1011
use crate::visitor::FmtVisitor;
1112

1213
struct SnippetStatus {
@@ -238,6 +239,7 @@ impl<'a> FmtVisitor<'a> {
238239
.next();
239240

240241
let fix_indent = last_char.map_or(true, |rev_c| ['{', '\n'].contains(&rev_c));
242+
let mut on_same_line = false;
241243

242244
let comment_indent = if fix_indent {
243245
if let Some('{') = last_char {
@@ -246,6 +248,13 @@ impl<'a> FmtVisitor<'a> {
246248
let indent_str = self.block_indent.to_string(self.config);
247249
self.push_str(&indent_str);
248250
self.block_indent
251+
} else if self.config.version() == Version::Two && !snippet.starts_with('\n') {
252+
// The comment appears on the same line as the previous formatted code.
253+
// Assuming that comment is logically associated with that code, we want to keep it on
254+
// the same level and avoid mixing it with possible other comment.
255+
on_same_line = true;
256+
self.push_str(" ");
257+
Indent::from_width(self.config, last_line_indent(&self.buffer))
249258
} else {
250259
self.push_str(" ");
251260
Indent::from_width(self.config, last_line_width(&self.buffer))
@@ -256,9 +265,34 @@ impl<'a> FmtVisitor<'a> {
256265
self.config.max_width() - self.block_indent.width(),
257266
);
258267
let comment_shape = Shape::legacy(comment_width, comment_indent);
259-
let comment_str = rewrite_comment(subslice, false, comment_shape, self.config)
260-
.unwrap_or_else(|| String::from(subslice));
261-
self.push_str(&comment_str);
268+
269+
if on_same_line {
270+
match subslice.find("\n") {
271+
None => {
272+
self.push_str(subslice);
273+
}
274+
Some(offset) if offset + 1 == subslice.len() => {
275+
self.push_str(&subslice[..offset]);
276+
}
277+
Some(offset) => {
278+
// keep first line as is: if it were too long and wrapped, it may get mixed
279+
// with the other lines.
280+
let first_line = &subslice[..offset];
281+
self.push_str(first_line);
282+
self.push_str(&comment_indent.to_string_with_newline(self.config));
283+
284+
let other_lines = &subslice[offset + 1..];
285+
let comment_str =
286+
rewrite_comment(other_lines, false, comment_shape, self.config)
287+
.unwrap_or_else(|| String::from(other_lines));
288+
self.push_str(&comment_str);
289+
}
290+
}
291+
} else {
292+
let comment_str = rewrite_comment(subslice, false, comment_shape, self.config)
293+
.unwrap_or_else(|| String::from(subslice));
294+
self.push_str(&comment_str);
295+
}
262296

263297
status.last_wspace = None;
264298
status.line_start = offset + subslice.len();

‎src/utils.rs‎

Lines changed: 10 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -193,19 +193,26 @@ pub(crate) fn is_attributes_extendable(attrs_str: &str) -> bool {
193193
!attrs_str.contains('\n') && !last_line_contains_single_line_comment(attrs_str)
194194
}
195195

196-
// The width of the first line in s.
196+
/// The width of the first line in s.
197197
#[inline]
198198
pub(crate) fn first_line_width(s: &str) -> usize {
199199
unicode_str_width(s.splitn(2, '\n').next().unwrap_or(""))
200200
}
201201

202-
// The width of the last line in s.
202+
/// The width of the last line in s.
203203
#[inline]
204204
pub(crate) fn last_line_width(s: &str) -> usize {
205205
unicode_str_width(s.rsplitn(2, '\n').next().unwrap_or(""))
206206
}
207207

208-
// The total used width of the last line.
208+
/// The indent width of the last line in s.
209+
#[inline]
210+
pub(crate) fn last_line_indent(s: &str) -> usize {
211+
let last_line = s.rsplitn(2, '\n').next().unwrap_or("");
212+
last_line.chars().take_while(|c| c.is_whitespace()).count()
213+
}
214+
215+
/// The total used width of the last line.
209216
#[inline]
210217
pub(crate) fn last_line_used_width(s: &str, offset: usize) -> usize {
211218
if s.contains('\n') {

‎src/visitor.rs‎

Lines changed: 51 additions & 22 deletions
Original file line numberDiff line numberDiff line change
@@ -7,6 +7,7 @@ use syntax::{ast, visit};
77

88
use crate::attr::*;
99
use crate::comment::{rewrite_comment, CodeCharKind, CommentCodeSlices};
10+
use crate::config::Version;
1011
use crate::config::{BraceStyle, Config};
1112
use crate::coverage::transform_missing_snippet;
1213
use crate::items::{
@@ -252,32 +253,60 @@ impl<'b, 'a: 'b> FmtVisitor<'a> {
252253

253254
let mut comment_shape =
254255
Shape::indented(self.block_indent, config).comment(config);
255-
if comment_on_same_line {
256-
// 1 = a space before `//`
257-
let offset_len = 1 + last_line_width(&self.buffer)
258-
.saturating_sub(self.block_indent.width());
259-
match comment_shape
260-
.visual_indent(offset_len)
261-
.sub_width(offset_len)
262-
{
263-
Some(shp) => comment_shape = shp,
264-
None => comment_on_same_line = false,
265-
}
266-
};
267-
268-
if comment_on_same_line {
256+
if self.config.version() == Version::Two && comment_on_same_line {
269257
self.push_str(" ");
258+
// put the first line of the comment on the same line as the
259+
// block's last line
260+
match sub_slice.find("\n") {
261+
None => {
262+
self.push_str(&sub_slice);
263+
}
264+
Some(offset) if offset + 1 == sub_slice.len() => {
265+
self.push_str(&sub_slice[..offset]);
266+
}
267+
Some(offset) => {
268+
let first_line = &sub_slice[..offset];
269+
self.push_str(first_line);
270+
self.push_str(&self.block_indent.to_string_with_newline(config));
271+
272+
// put the other lines below it, shaping it as needed
273+
let other_lines = &sub_slice[offset + 1..];
274+
let comment_str =
275+
rewrite_comment(other_lines, false, comment_shape, config);
276+
match comment_str {
277+
Some(ref s) => self.push_str(s),
278+
None => self.push_str(other_lines),
279+
}
280+
}
281+
}
270282
} else {
271-
if count_newlines(snippet_in_between) >= 2 || extra_newline {
272-
self.push_str("\n");
283+
if comment_on_same_line {
284+
// 1 = a space before `//`
285+
let offset_len = 1 + last_line_width(&self.buffer)
286+
.saturating_sub(self.block_indent.width());
287+
match comment_shape
288+
.visual_indent(offset_len)
289+
.sub_width(offset_len)
290+
{
291+
Some(shp) => comment_shape = shp,
292+
None => comment_on_same_line = false,
293+
}
294+
};
295+
296+
if comment_on_same_line {
297+
self.push_str(" ");
298+
} else {
299+
if count_newlines(snippet_in_between) >= 2 || extra_newline {
300+
self.push_str("\n");
301+
}
302+
self.push_str(&self.block_indent.to_string_with_newline(config));
273303
}
274-
self.push_str(&self.block_indent.to_string_with_newline(config));
275-
}
276304

277-
let comment_str = rewrite_comment(&sub_slice, false, comment_shape, config);
278-
match comment_str {
279-
Some(ref s) => self.push_str(s),
280-
None => self.push_str(&sub_slice),
305+
let comment_str = rewrite_comment(&sub_slice, false, comment_shape, config);
306+
match comment_str {
307+
Some(ref s) => self.push_str(s),
308+
None => self.push_str(&sub_slice),
309+
}
281310
}
282311
}
283312
CodeCharKind::Normal if skip_normal(&sub_slice) => {

‎tests/source/trailing_comments.rs‎

Lines changed: 21 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,21 @@
1+
// rustfmt-version: Two
2+
// rustfmt-wrap_comments: true
3+
4+
pub const IFF_MULTICAST: ::c_int = 0x0000000800; // Supports multicast
5+
// Multicast using broadcst. add.
6+
7+
pub const SQ_CRETAB: u16 = 0x000e; // CREATE TABLE
8+
pub const SQ_DRPTAB: u16 = 0x000f; // DROP TABLE
9+
pub const SQ_CREIDX: u16 = 0x0010; // CREATE INDEX
10+
//const SQ_DRPIDX: u16 = 0x0011; // DROP INDEX
11+
//const SQ_GRANT: u16 = 0x0012; // GRANT
12+
//const SQ_REVOKE: u16 = 0x0013; // REVOKE
13+
14+
fn foo() {
15+
let f = bar(); // Donec consequat mi. Quisque vitae dolor. Integer lobortis. Maecenas id nulla. Lorem.
16+
// 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.
17+
let b = baz();
18+
19+
let normalized = self.ctfont.all_traits().normalized_weight(); // [-1.0, 1.0]
20+
// TODO(emilio): It may make sense to make this range [.01, 10.0], to align with css-fonts-4's range of [1, 1000].
21+
}

‎tests/target/trailing_comments.rs‎

Lines changed: 30 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,30 @@
1+
// rustfmt-version: Two
2+
// rustfmt-wrap_comments: true
3+
4+
pub const IFF_MULTICAST: ::c_int = 0x0000000800; // Supports multicast
5+
// Multicast using broadcst. add.
6+
7+
pub const SQ_CRETAB: u16 = 0x000e; // CREATE TABLE
8+
pub const SQ_DRPTAB: u16 = 0x000f; // DROP TABLE
9+
pub const SQ_CREIDX: u16 = 0x0010; // CREATE INDEX
10+
//const SQ_DRPIDX: u16 = 0x0011; // DROP INDEX
11+
//const SQ_GRANT: u16 = 0x0012; // GRANT
12+
//const SQ_REVOKE: u16 = 0x0013; // REVOKE
13+
14+
fn foo() {
15+
let f = bar(); // Donec consequat mi. Quisque vitae dolor. Integer lobortis. Maecenas id nulla. Lorem.
16+
// Id turpis. Nam posuere lectus vitae nibh. Etiam tortor orci, sagittis
17+
// malesuada, rhoncus quis, hendrerit eget, libero. Quisque commodo nulla at
18+
// nunc. Mauris consequat, enim vitae venenatis sollicitudin, dolor orci
19+
// bibendum enim, a sagittis nulla nunc quis elit. Phasellus augue. Nunc
20+
// suscipit, magna tincidunt lacinia faucibus, lacus tellus ornare purus, a
21+
// pulvinar lacus orci eget nibh. Maecenas sed nibh non lacus tempor faucibus.
22+
// In hac habitasse platea dictumst. Vivamus a orci at nulla tristique
23+
// condimentum. Donec arcu quam, dictum accumsan, convallis accumsan, cursus sit
24+
// amet, ipsum. In pharetra sagittis nunc.
25+
let b = baz();
26+
27+
let normalized = self.ctfont.all_traits().normalized_weight(); // [-1.0, 1.0]
28+
// TODO(emilio): It may make sense to make this range [.01, 10.0], to align
29+
// with css-fonts-4's range of [1, 1000].
30+
}

0 commit comments

Comments
 (0)