gn format: fix some cases of too many blank lines between items
R=brettw@chromium.org
BUG=348474
Review URL: https://codereview.chromium.org/636663002
Cr-Original-Commit-Position: refs/heads/master@{#298341}
Cr-Mirrored-From: https://chromium.googlesource.com/chromium/src
Cr-Mirrored-Commit: 785ffa786a39e0b98ff3ebf1dfeec3c47340fd15
diff --git a/tools/gn/command_format.cc b/tools/gn/command_format.cc
index 05fca4c..681c0e0 100644
--- a/tools/gn/command_format.cc
+++ b/tools/gn/command_format.cc
@@ -113,6 +113,13 @@
// Whether there's a blank separator line at the current position.
bool HaveBlankLine();
+ bool IsAssignment(const ParseNode* node);
+
+ // Heuristics to decide if there should be a blank line added between two
+ // items. For various "small" items, it doesn't look nice if there's too much
+ // vertical whitespace added.
+ bool ShouldAddBlankLineInBetween(const ParseNode* a, const ParseNode* b);
+
// Get the 0-based x position on the current line.
int CurrentColumn();
@@ -227,6 +234,31 @@
return n > 2 && output_[n - 1] == '\n' && output_[n - 2] == '\n';
}
+bool Printer::IsAssignment(const ParseNode* node) {
+ return node->AsBinaryOp() && (node->AsBinaryOp()->op().value() == "=" ||
+ node->AsBinaryOp()->op().value() == "+=" ||
+ node->AsBinaryOp()->op().value() == "-=");
+}
+
+bool Printer::ShouldAddBlankLineInBetween(const ParseNode* a,
+ const ParseNode* b) {
+ // A bunch of imports looks silly at the top of a file separated by blank
+ // lines, even though they are statements.
+ if (a->AsFunctionCall() && b->AsFunctionCall() &&
+ a->AsFunctionCall()->function().value() == "import" &&
+ b->AsFunctionCall()->function().value() == "import") {
+ return false;
+ }
+
+ if (IsAssignment(a) && IsAssignment(b)) {
+ Metrics first = GetLengthOfExpr(a, kPrecedenceLowest);
+ Metrics second = GetLengthOfExpr(b, kPrecedenceLowest);
+ if (!first.multiline && !second.multiline)
+ return false;
+ }
+ return true;
+}
+
int Printer::CurrentColumn() {
int n = 0;
while (n < static_cast<int>(output_.size()) &&
@@ -261,8 +293,11 @@
Newline();
}
}
- if (i < block->statements().size() - 1)
+ if (i < block->statements().size() - 1 &&
+ (ShouldAddBlankLineInBetween(block->statements()[i],
+ block->statements()[i + 1]))) {
Newline();
+ }
++i;
}
diff --git a/tools/gn/command_format_unittest.cc b/tools/gn/command_format_unittest.cc
index 157461f..4583b6c 100644
--- a/tools/gn/command_format_unittest.cc
+++ b/tools/gn/command_format_unittest.cc
@@ -62,3 +62,5 @@
FORMAT_TEST(025)
FORMAT_TEST(026)
FORMAT_TEST(027)
+FORMAT_TEST(028)
+FORMAT_TEST(029)
diff --git a/tools/gn/format_test_data/028.gn b/tools/gn/format_test_data/028.gn
new file mode 100644
index 0000000..d84e1f8
--- /dev/null
+++ b/tools/gn/format_test_data/028.gn
@@ -0,0 +1,9 @@
+# Don't separate these.
+import("wee.gni")
+import("waa.gni")
+
+import("woo.gni")
+
+
+
+import("blah.gni")
diff --git a/tools/gn/format_test_data/028.golden b/tools/gn/format_test_data/028.golden
new file mode 100644
index 0000000..d22d4a2
--- /dev/null
+++ b/tools/gn/format_test_data/028.golden
@@ -0,0 +1,5 @@
+# Don't separate these.
+import("wee.gni")
+import("waa.gni")
+import("woo.gni")
+import("blah.gni")
diff --git a/tools/gn/format_test_data/029.gn b/tools/gn/format_test_data/029.gn
new file mode 100644
index 0000000..ac67830
--- /dev/null
+++ b/tools/gn/format_test_data/029.gn
@@ -0,0 +1,9 @@
+# Don't separate small simple statements.
+is_android = false
+is_chromeos = false
+is_ios = false
+is_linux -= false
+is_mac = true
+is_nacl = false
+is_posix += true
+is_win = false
diff --git a/tools/gn/format_test_data/029.golden b/tools/gn/format_test_data/029.golden
new file mode 100644
index 0000000..ac67830
--- /dev/null
+++ b/tools/gn/format_test_data/029.golden
@@ -0,0 +1,9 @@
+# Don't separate small simple statements.
+is_android = false
+is_chromeos = false
+is_ios = false
+is_linux -= false
+is_mac = true
+is_nacl = false
+is_posix += true
+is_win = false
