gn format: don't break+indent if it's going to be forced past 80col anyway

A penalty is accrued for each character past 80col, but when both
breaking and not breaking are too long, don't break because it doesn't
help readability.

R=brettw@chromium.org,dpranke@chromium.org
BUG=348474

Review URL: https://codereview.chromium.org/775793003

Cr-Commit-Position: refs/heads/master@{#306515}

6 files changed, 31 insertions, 9 deletions

diff --git a/tools/gn/command_format.cc b/tools/gn/command_format.cc
index 75fdfb6..1ce072d 100644
--- a/tools/gn/command_format.cc
+++ b/tools/gn/command_format.cc
@@ -145,6 +145,9 @@ class Printer {
   // Generic penalties for exceeding maximum width, adding more lines, etc.
   int AssessPenalty(const std::string& output);
 
+  // Tests if any lines exceed the maximum width.
+  bool ExceedsMaximumWidth(const std::string& output);
+
   // Format a list of values using the given style.
   // |end| holds any trailing comments to be printed just before the closing
   // bracket.
@@ -374,6 +377,16 @@ int Printer::AssessPenalty(const std::string& output) {
   return penalty;
 }
 
+bool Printer::ExceedsMaximumWidth(const std::string& output) {
+  std::vector<std::string> lines;
+  base::SplitStringDontTrim(output, '\n', &lines);
+  for (const auto& line : lines) {
+    if (line.size() > kMaximumWidth)
+      return true;
+  }
+  return false;
+}
+
 void Printer::AddParen(int prec, int outer_prec, bool* parenthesized) {
   if (prec < outer_prec) {
     Print("(");
@@ -497,7 +510,16 @@ int Printer::Expr(const ParseNode* root,
     sub2.Print(suffix);
     penalty_next_line += AssessPenalty(sub2.String());
 
-    if (penalty_current_line < penalty_next_line) {
+    // If in both cases it was forced past 80col, then we don't break to avoid
+    // breaking after '=' in the case of:
+    //   variable = "... very long string ..."
+    // as breaking and indenting doesn't make things much more readable, even
+    // though there's less characters past the maximum width.
+    bool exceeds_maximum_either_way = ExceedsMaximumWidth(sub1.String()) &&
+                                      ExceedsMaximumWidth(sub2.String());
+
+    if (penalty_current_line < penalty_next_line ||
+        exceeds_maximum_either_way) {
       Print(" ");
       Expr(binop->right(), prec_right, std::string());
     } else {
diff --git a/tools/gn/command_format_unittest.cc b/tools/gn/command_format_unittest.cc
index df9e41d..73badce 100644
--- a/tools/gn/command_format_unittest.cc
+++ b/tools/gn/command_format_unittest.cc
@@ -95,3 +95,4 @@ FORMAT_TEST(056)
 FORMAT_TEST(057)
 FORMAT_TEST(058)
 FORMAT_TEST(059)
+FORMAT_TEST(060)
diff --git a/tools/gn/format_test_data/043.gn b/tools/gn/format_test_data/043.gn
index a2748d5..b95c6a5 100644
--- a/tools/gn/format_test_data/043.gn
+++ b/tools/gn/format_test_data/043.gn
@@ -1,6 +1,4 @@
-# We might prefer not to break the first one since it still exceeds the maximum
-# width. At the moment, each character past the end is a greater penalty, so
-# breaking to +4 is "better".
+# Don't break and indent when it's hopeless.
 # 80 ---------------------------------------------------------------------------
 android_java_prebuilt("android_support_v13_java") {
   jar_path = "$android_sdk_root/extras/android/support/v7/appcompat/libs/android-support-v7-appcompat.jar"
diff --git a/tools/gn/format_test_data/043.golden b/tools/gn/format_test_data/043.golden
index 5ba421b..336ec2f 100644
--- a/tools/gn/format_test_data/043.golden
+++ b/tools/gn/format_test_data/043.golden
@@ -1,10 +1,7 @@
-# We might prefer not to break the first one since it still exceeds the maximum
-# width. At the moment, each character past the end is a greater penalty, so
-# breaking to +4 is "better".
+# Don't break and indent when it's hopeless.
 # 80 ---------------------------------------------------------------------------
 android_java_prebuilt("android_support_v13_java") {
-  jar_path =
-      "$android_sdk_root/extras/android/support/v7/appcompat/libs/android-support-v7-appcompat.jar"
+  jar_path = "$android_sdk_root/extras/android/support/v7/appcompat/libs/android-support-v7-appcompat.jar"
   jar_path =
       "$android_sdk_root/extras/android/support/v13/android-support-v13.jar"
 }
diff --git a/tools/gn/format_test_data/060.gn b/tools/gn/format_test_data/060.gn
new file mode 100644
index 0000000..2b0da79
--- /dev/null
+++ b/tools/gn/format_test_data/060.gn
@@ -0,0 +1,2 @@
+some_variable = "this is a very long string that is going to exceed 80 col and will never under any circumstance fit"
+another_variable = [ "this is a very long string that is going to exceed 80 col and will never under any circumstance fit" ]
diff --git a/tools/gn/format_test_data/060.golden b/tools/gn/format_test_data/060.golden
new file mode 100644
index 0000000..2b0da79
--- /dev/null
+++ b/tools/gn/format_test_data/060.golden
@@ -0,0 +1,2 @@
+some_variable = "this is a very long string that is going to exceed 80 col and will never under any circumstance fit"
+another_variable = [ "this is a very long string that is going to exceed 80 col and will never under any circumstance fit" ]