Commit 7f86d891 authored by Scott Graham's avatar Scott Graham Committed by Commit Bot

gn: Check if formatting a single line list to multiple reduces penalty

Previously, when formatting binary operators, gn format would only try
to break around the operators, but the determination of whether a list
was single or multiline was only determined by the number of elements
in the list (and in some cases, the LHS of the assignment).

Instead now, check if spliting a single line list into multiline would
avoid going past 80 columns.

A test run of running this new behaviour on all of chromium/src is at
https://chromium-review.googlesource.com/c/chromium/src/+/801660/, and
it seems like a universal improvement, fixing a number of misformattings.

Bug: 790021
Change-Id: I06ab6f06388dc9429a75407864769484d3fcea41
Reviewed-on: https://chromium-review.googlesource.com/801658Reviewed-by: default avatarDirk Pranke <dpranke@chromium.org>
Commit-Queue: Scott Graham <scottmg@chromium.org>
Cr-Commit-Position: refs/heads/master@{#520726}
parent c1cceea1
...@@ -576,18 +576,45 @@ int Printer::Expr(const ParseNode* root, ...@@ -576,18 +576,45 @@ int Printer::Expr(const ParseNode* root,
sub2.Print(suffix); sub2.Print(suffix);
penalty_next_line += AssessPenalty(sub2.String()); penalty_next_line += AssessPenalty(sub2.String());
// If in both cases it was forced past 80col, then we don't break to avoid // Force a list on the RHS that would normally be a single line into
// multiline.
bool tried_rhs_multiline = false;
Printer sub3;
InitializeSub(&sub3);
int penalty_multiline_rhs_list = std::numeric_limits<int>::max();
const ListNode* rhs_list = binop->right()->AsList();
if (is_assignment && rhs_list &&
!ListWillBeMultiline(rhs_list->contents(), rhs_list->End())) {
sub3.Print(" ");
sub3.stack_.push_back(IndentState(start_column, false, false));
sub3.Sequence(kSequenceStyleList, rhs_list->contents(), rhs_list->End(),
true);
sub3.stack_.pop_back();
penalty_multiline_rhs_list = AssessPenalty(sub3.String());
tried_rhs_multiline = true;
}
// If in all cases it was forced past 80col, then we don't break to avoid
// breaking after '=' in the case of: // breaking after '=' in the case of:
// variable = "... very long string ..." // variable = "... very long string ..."
// as breaking and indenting doesn't make things much more readable, even // as breaking and indenting doesn't make things much more readable, even
// though there's less characters past the maximum width. // though there's fewer characters past the maximum width.
bool exceeds_maximum_either_way = ExceedsMaximumWidth(sub1.String()) && bool exceeds_maximum_all_ways =
ExceedsMaximumWidth(sub2.String()); ExceedsMaximumWidth(sub1.String()) &&
ExceedsMaximumWidth(sub2.String()) &&
(!tried_rhs_multiline || ExceedsMaximumWidth(sub3.String()));
if (penalty_current_line < penalty_next_line || if (penalty_current_line < penalty_next_line ||
exceeds_maximum_either_way) { exceeds_maximum_all_ways) {
Print(" "); Print(" ");
Expr(binop->right(), prec_right, std::string()); Expr(binop->right(), prec_right, std::string());
} else if (tried_rhs_multiline &&
penalty_multiline_rhs_list < penalty_next_line) {
// Force a multiline list on the right.
Print(" ");
stack_.push_back(IndentState(start_column, false, false));
Sequence(kSequenceStyleList, rhs_list->contents(), rhs_list->End(), true);
stack_.pop_back();
} else { } else {
// Otherwise, put first argument and op, and indent next. // Otherwise, put first argument and op, and indent next.
Newline(); Newline();
......
...@@ -100,3 +100,4 @@ FORMAT_TEST(065) ...@@ -100,3 +100,4 @@ FORMAT_TEST(065)
FORMAT_TEST(066) FORMAT_TEST(066)
FORMAT_TEST(067) FORMAT_TEST(067)
FORMAT_TEST(068) FORMAT_TEST(068)
FORMAT_TEST(069)
if (true) {
configs -= [ "//third_party/mini_chromium/mini_chromium/build:Wexit_time_destructors", ]
}
if (true) {
configs -= [
"//third_party/mini_chromium/mini_chromium/build:Wexit_time_destructors",
]
}
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment