Skip to content

Commit

Permalink
Don't be so aggressie about line-breaking strings (#911)
Browse files Browse the repository at this point in the history 
We will no longer break in the middle of words, only at whitespace or punctuation.

This means we sometimes over-run, but that seems better than some of the bad splits we see.

Closes #369
  • Loading branch information
@nrc @marcusklaas
nrc authored and marcusklaas committed Apr 11, 2016
1 parent 19849fe commit 7ac354f
Show file tree
Hide file tree
Showing 9 changed files with 80 additions and 99 deletions.
47 changes: 28 additions & 19 deletions src/comment.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -102,7 +102,7 @@ pub fn rewrite_comment(orig: &str,
}

if config.wrap_comments && line.len() > max_chars {
let rewrite = try_opt!(rewrite_string(line, &fmt));
let rewrite = rewrite_string(line, &fmt).unwrap_or(line.to_owned());
result.push_str(&rewrite);
} else {
if line.len() == 0 {
Expand Down Expand Up @@ -672,27 +672,36 @@ mod test {
fn format_comments() {
let mut config: ::config::Config = Default::default();
config.wrap_comments = true;
assert_eq!("/* test */", rewrite_comment(" //test", true, 100, Indent::new(0, 100),
&config).unwrap());
assert_eq!("// comment\n// on a", rewrite_comment("// comment on a", false, 10,
Indent::empty(), &config).unwrap());

assert_eq!("// A multi line comment\n // between args.",
rewrite_comment("// A multi line comment\n // between args.",
false,
60,
Indent::new(0, 12),
&config).unwrap());

let comment = rewrite_comment(" //test", true, 100, Indent::new(0, 100), &config).unwrap();
assert_eq!("/* test */", comment);

let comment = rewrite_comment("// comment on a",
false,
10,
Indent::empty(),
&config).unwrap();
assert_eq!("// comment\n// on a", comment);

let comment = rewrite_comment("// A multi line comment\n // between args.",
false,
60,
Indent::new(0, 12),
&config).unwrap();
assert_eq!("// A multi line comment\n // between args.", comment);

let input = "// comment";
let expected =
"/* com\n \
* men\n \
* t */";
assert_eq!(expected, rewrite_comment(input, true, 9, Indent::new(0, 69), &config).unwrap());

assert_eq!("/* trimmed */", rewrite_comment("/* trimmed */", true, 100,
Indent::new(0, 100), &config).unwrap());
"/* comment */";
let comment = rewrite_comment(input, true, 9, Indent::new(0, 69), &config).unwrap();
assert_eq!(expected, comment);

let comment = rewrite_comment("/* trimmed */",
true,
100,
Indent::new(0, 100),
&config).unwrap();
assert_eq!("/* trimmed */", comment);
}

// This is probably intended to be a non-test fn, but it is not used. I'm
Expand Down
48 changes: 32 additions & 16 deletions src/string.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -39,6 +39,7 @@ pub fn rewrite_string<'a>(orig: &str, fmt: &StringFormat<'a>) -> Option<String>
let indent = fmt.offset.to_string(fmt.config);
let punctuation = ":,;.";

// `cur_start` is the position in `orig` of the start of the current line.
let mut cur_start = 0;
let mut result = String::with_capacity(stripped_str.len()
.checked_next_power_of_two()
Expand All @@ -50,30 +51,43 @@ pub fn rewrite_string<'a>(orig: &str, fmt: &StringFormat<'a>) -> Option<String>
// succeed.
let max_chars = try_opt!(fmt.width.checked_sub(fmt.opener.len() + ender_length + 1)) + 1;

loop {
// Snip a line at a time from `orig` until it is used up. Push the snippet
// onto result.
'outer: loop {
// `cur_end` will be where we break the line, as an offset into `orig`.
// Initialised to the maximum it could be (which may be beyond `orig`).
let mut cur_end = cur_start + max_chars;

// We can fit the rest of the string on this line, so we're done.
if cur_end >= graphemes.len() {
let line = &graphemes[cur_start..].join("");
result.push_str(line);
break;
break 'outer;
}

// Push cur_end left until we reach whitespace.
// Push cur_end left until we reach whitespace (or the line is too small).
while !graphemes[cur_end - 1].trim().is_empty() {
cur_end -= 1;
if cur_end - cur_start < MIN_STRING {
if cur_end < cur_start + MIN_STRING {
// We couldn't find whitespace before the string got too small.
// So start again at the max length and look for punctuation.
cur_end = cur_start + max_chars;
// Look for punctuation to break on.
while (!punctuation.contains(graphemes[cur_end - 1])) && cur_end > 1 {
while !punctuation.contains(graphemes[cur_end - 1]) {
cur_end -= 1;
}
// We can't break at whitespace or punctuation, fall back to splitting
// anywhere that doesn't break an escape sequence.
if cur_end < cur_start + MIN_STRING {
cur_end = cur_start + max_chars;
while graphemes[cur_end - 1] == "\\" && cur_end > 1 {
cur_end -= 1;

// If we can't break at whitespace or punctuation, grow the string instead.
if cur_end < cur_start + MIN_STRING {
cur_end = cur_start + max_chars;
while !(punctuation.contains(graphemes[cur_end - 1]) ||
graphemes[cur_end - 1].trim().is_empty()) {
if cur_end >= graphemes.len() {
let line = &graphemes[cur_start..].join("");
result.push_str(line);
break 'outer;
}
cur_end += 1;
}
break;
}
}
break;
Expand All @@ -83,12 +97,13 @@ pub fn rewrite_string<'a>(orig: &str, fmt: &StringFormat<'a>) -> Option<String>
while cur_end < stripped_str.len() && graphemes[cur_end].trim().is_empty() {
cur_end += 1;
}

// Make the current line and add it on to result.
let raw_line = graphemes[cur_start..cur_end].join("");
let line = if fmt.trim_end {
raw_line.trim()
} else {
// FIXME: use as_str once it's stable.
&*raw_line
raw_line.as_str()
};

result.push_str(line);
Expand All @@ -97,10 +112,11 @@ pub fn rewrite_string<'a>(orig: &str, fmt: &StringFormat<'a>) -> Option<String>
result.push_str(&indent);
result.push_str(fmt.line_start);

// The next line starts where the current line ends.
cur_start = cur_end;
}
result.push_str(fmt.closer);

result.push_str(fmt.closer);
Some(result)
}

Expand Down
9 changes: 3 additions & 6 deletions tests/target/enum.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -51,8 +51,7 @@ enum X {
CreateWebGLPaintTask(Size2D<i32>,
GLContextAttributes,
IpcSender<Result<(IpcSender<CanvasMsg>, usize), String>>), /* This is
* a post c
* omment */
* a post comment */
}

pub enum EnumWithAttributes {
Expand All @@ -68,8 +67,7 @@ pub enum EnumWithAttributes {
ItemStruct {
x: usize,
y: usize,
}, /* Comment AAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAA
* AAAAAAAAAAAAAAAAAAA */
}, /* Comment AAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAA */
// And another
ForcedPreflight, /* AAAAAAAAAAAAAAA AAAAAAAAAAAAAAAAAAAAAAAAAAAAAAA
* AAAAAAAAAAAAAAAAAAAAAAAAAAAAAAA */
Expand All @@ -79,8 +77,7 @@ pub enum SingleTuple {
// Pre Comment AAAAAAAAAAAAAAAAAAAAAAA AAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAA
// AAAAAAAAAAAAAAAAAAAAAAAAAAAAAA
Match(usize, usize, String), /* Post-comment
* AAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAA
* A */
* AAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAA */
}

pub enum SingleStruct {
Expand Down
3 changes: 1 addition & 2 deletions tests/target/hard-tabs.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -26,8 +26,7 @@ fn main() {
a: i32) {
}

let str = "AAAAAAAAAAAAAAaAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAaAAAAAAAAAAAAAAAAAAAAA\
AAAAAAAAAAAAaAa";
let str = "AAAAAAAAAAAAAAaAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAaAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAaAa";

if let (some_very_large,
tuuuuuuuuuuuuuuuuuuuuuuuuuuuuuuuuuuuuuuuuuuuuuuuuuuuuuuuuuuuuuuuuuple) = 1 + 2 + 3 {
Expand Down
3 changes: 1 addition & 2 deletions tests/target/string-lit-2.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,5 @@ fn main() -> &'static str {
// Crappy formatting :-(
let change_me = "sssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssss
\
jjjjjjjjjjjjjjjjjjjjjjjjjjjjjjjjjjjjjjjjjjjjjjjjjjjjjjjjjjjjjjjjjjjjjjjjjjjjj\
j";
jjjjjjjjjjjjjjjjjjjjjjjjjjjjjjjjjjjjjjjjjjjjjjjjjjjjjjjjjjjjjjjjjjjjjjjjjjjjjj";
}
22 changes: 6 additions & 16 deletions tests/target/string-lit.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -4,22 +4,16 @@
fn main() -> &'static str {
let str = "AAAAAAAAAAAAAAaAAAAAAAAAAAAAAAAAAAAAAAA AAAAAAAAAAAAAAAAAAAAAAaAA \
AAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAaAa";
let str = "AAAAAAAAAAAAAAaAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAaAAAAAAAAAAAAAAAAAAAAA\
AAAAAAAAAAAAaAa";
let str = "AAAAAAAAAAAAAAaAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAaAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAaAa";
let str = "AAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAA";

let too_many_lines = "Hello";

// Make sure we don't break after an escape character.
let odd_length_name = "\n\n\n\n\n\n\n\n\n\n\n\n\n\n\n\n\n\n\n\n\n\n\n\n\n\n\n\n\n\n\n\n\n\n\n\
\n\n\n";
let even_length_name = "\n\n\n\n\n\n\n\n\n\n\n\n\n\n\n\n\n\n\n\n\n\n\n\n\n\n\n\n\n\n\n\n\n\n\n\
\n\n\n";
let odd_length_name = "\n\n\n\n\n\n\n\n\n\n\n\n\n\n\n\n\n\n\n\n\n\n\n\n\n\n\n\n\n\n\n\n\n\n\n\n\n\n";
let even_length_name = "\n\n\n\n\n\n\n\n\n\n\n\n\n\n\n\n\n\n\n\n\n\n\n\n\n\n\n\n\n\n\n\n\n\n\n\n\n\n";

let really_long_variable_name = "AAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAA\
AAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAA\
AAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAA\
AA";
let really_long_variable_name = "AAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAA";

let raw_string = r#"Do
not
Expand All @@ -28,16 +22,12 @@ formatting"#;

filename.replace(" ", "\\");

let xxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxx = funktion("yyyyyyyyyyyyyyyyyyyyy\
yyyyyyyyyyyyyyyyyyyyy\
yyyyyyyyyyyyyyyyyyyyy\
yyyyyyyyyy");
let xxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxx = funktion("yyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyy");

let unicode = "a̐éö̲\r\n";
let unicode2 = "Löwe 老虎 Léopard";
let unicode3 = "中华Việt Nam";
let unicode4 = "☃☃☃☃☃☃☃☃☃☃☃☃☃☃☃☃☃☃☃☃☃☃☃☃☃☃☃☃☃☃☃☃☃☃☃☃☃☃☃☃☃☃☃☃☃☃☃☃☃☃☃☃☃☃☃☃☃☃☃☃☃☃☃☃☃☃☃☃☃☃☃☃☃☃☃☃☃☃\
☃☃☃☃☃☃☃☃☃☃☃☃☃☃☃☃☃☃☃☃☃☃☃☃☃☃☃☃☃☃☃☃☃☃☃☃☃☃☃☃☃☃";
let unicode4 = "☃☃☃☃☃☃☃☃☃☃☃☃☃☃☃☃☃☃☃☃☃☃☃☃☃☃☃☃☃☃☃☃☃☃☃☃☃☃☃☃☃☃☃☃☃☃☃☃☃☃☃☃☃☃☃☃☃☃☃☃☃☃☃☃☃☃☃☃☃☃☃☃☃☃☃☃☃☃☃☃☃☃☃☃☃☃☃☃☃☃☃☃☃☃☃☃☃☃☃☃☃☃☃☃☃☃☃☃☃☃☃☃☃☃☃☃☃☃☃☃";

"stuffin'"
}
Expand Down
3 changes: 1 addition & 2 deletions tests/target/string_punctuation.rs
View file
Original file line number Diff line number Diff line change
@@ -1,8 +1,7 @@
fn main() {
println!("ThisIsAReallyLongStringWithNoSpaces.It_should_prefer_to_break_onpunctuation:\
Likethisssssssssssss");
format!("{}__{}__{}ItShouldOnlyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyNoticeSemicolonsPeriodsColo\
nsAndCommasAndResortToMid-CharBreaksAfterPunctuation{}{}",
format!("{}__{}__{}ItShouldOnlyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyNoticeSemicolonsPeriodsColonsAndCommasAndResortToMid-CharBreaksAfterPunctuation{}{}",
x,
y,
z,
Expand Down
22 changes: 4 additions & 18 deletions tests/target/struct_lits_visual.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -16,24 +16,10 @@ fn main() {

Fooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooo { a: f(), b: b() };

Foooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooo { // Commen
// t
a: foo(), /* C
* o
* m
* m
* e
* n
* t */
// Commen
// t
b: bar(), /* C
* o
* m
* m
* e
* n
* t */ };
Foooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooo { // Comment
a: foo(), /* Comment */
// Comment
b: bar(), /* Comment */ };

Foo { a: Bar, b: f() };

Expand Down
22 changes: 4 additions & 18 deletions tests/target/struct_lits_visual_multiline.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -18,24 +18,10 @@ fn main() {
Fooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooo { a: foo(),
b: bar(), };

Foooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooo { // Commen
// t
a: foo(), /* C
* o
* m
* m
* e
* n
* t */
// Commen
// t
b: bar(), /* C
* o
* m
* m
* e
* n
* t */ };
Foooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooo { // Comment
a: foo(), /* Comment */
// Comment
b: bar(), /* Comment */ };

Foo { a: Bar,
b: foo(), };
Expand Down

0 comments on commit 7ac354f

Please sign in to comment.