Skip to content

Don't be so aggressive about line-breaking strings #911

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

Merged
merged 1 commit into from
Apr 11, 2016
Merged

Don't be so aggressive about line-breaking strings #911

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
47 changes: 28 additions & 19 deletions src/comment.rs
View file
Open in desktop
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
Open in desktop
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
Open in desktop
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
Open in desktop
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
Open in desktop
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
Open in desktop
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
Open in desktop
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
Open in desktop
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
Open in desktop
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