Chromium Code Reviews
chromiumcodereview-hr@appspot.gserviceaccount.com (chromiumcodereview-hr) | Please choose your nickname with Settings | Help | Chromium Project | Gerrit Changes | Sign out
(1762)

Unified Diff: tools/gn/command_format.cc

Issue 636663002: gn format: fix some cases of too many blank lines between items (Closed) Base URL: https://chromium.googlesource.com/chromium/src.git@master
Patch Set: gn format: fix some cases of too many blank lines between items Created 6 years, 2 months ago
Use n/p to move between diff chunks; N/P to move between comments. Draft comments are only viewable by you.
Jump to:
View side-by-side diff with in-line comments
Download patch
« no previous file with comments | « no previous file | tools/gn/command_format_unittest.cc » ('j') | no next file with comments »
Expand Comments ('e') | Collapse Comments ('c') | Show Comments Hide Comments ('s')
Index: tools/gn/command_format.cc
diff --git a/tools/gn/command_format.cc b/tools/gn/command_format.cc
index 05fca4cc68458e7ea7357644d6d915b1ab05e0cf..681c0e0c84459bf899045c4ccf84e5bfd9d10506 100644
--- a/tools/gn/command_format.cc
+++ b/tools/gn/command_format.cc
@@ -113,6 +113,13 @@ class Printer {
// 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 @@ bool Printer::HaveBlankLine() {
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 @@ void Printer::Block(const ParseNode* root) {
Newline();
}
}
- if (i < block->statements().size() - 1)
+ if (i < block->statements().size() - 1 &&
+ (ShouldAddBlankLineInBetween(block->statements()[i],
+ block->statements()[i + 1]))) {
Newline();
+ }
++i;
}
« no previous file with comments | « no previous file | tools/gn/command_format_unittest.cc » ('j') | no next file with comments »

Powered by Google App Engine
This is Rietveld 408576698