Remove irregular formatting for some lists after =
GN treated a few variables with magic names (deps, sources, ...)
as magic and always put lists after them on multiple lines,
even if the lists only contained single elements. This isn't
the case for lists with other names, and it doesn't happen
for the special names with += either.
Remove the special case for consistency (and simpler code).
See discussion on "[gn-dev] Inconsistent list formatting for '=' versus '+=' ?"
Bug: none
Change-Id: I015647b3f2117b38c8d44efa8df0a222197c8347
Reviewed-on: https://gn-review.googlesource.com/c/gn/+/6860
Commit-Queue: Brett Wilson <brettw@chromium.org>
Reviewed-by: Brett Wilson <brettw@chromium.org>
diff --git a/src/gn/command_format.cc b/src/gn/command_format.cc
index 03d9311..668fedd 100644
--- a/src/gn/command_format.cc
+++ b/src/gn/command_format.cc
@@ -144,9 +144,6 @@
// Whether there's a blank separator line at the current position.
bool HaveBlankLine();
- // Flag assignments to sources, deps, etc. to make their RHSs multiline.
- void AnnotatePreferredMultilineAssignment(const BinaryOpNode* binop);
-
// Sort a list on the RHS if the LHS is 'sources', 'deps' or 'public_deps'.
// The 'sources' are sorted alphabetically while the 'deps' and 'public_deps'
// are sorted putting first the relative targets and then the global ones
@@ -318,21 +315,6 @@
return n > 2 && output_[n - 1] == '\n' && output_[n - 2] == '\n';
}
-void Printer::AnnotatePreferredMultilineAssignment(const BinaryOpNode* binop) {
- const IdentifierNode* ident = binop->left()->AsIdentifier();
- const ListNode* list = binop->right()->AsList();
- // This is somewhat arbitrary, but we include the 'deps'- and 'sources'-like
- // things, but not flags things.
- if (binop->op().value() == "=" && ident && list) {
- const std::string_view lhs = ident->value().value();
- if (lhs == "data" || lhs == "datadeps" || lhs == "data_deps" ||
- lhs == "deps" || lhs == "inputs" || lhs == "outputs" ||
- lhs == "public" || lhs == "public_deps" || lhs == "sources") {
- const_cast<ListNode*>(list)->set_prefer_multiline(true);
- }
- }
-}
-
void Printer::SortIfSourcesOrDeps(const BinaryOpNode* binop) {
if (const Comments* comments = binop->comments()) {
const std::vector<Token>& before = comments->before();
@@ -587,7 +569,6 @@
}
} else if (const BinaryOpNode* binop = root->AsBinaryOp()) {
CHECK(precedence_.find(binop->op().value()) != precedence_.end());
- AnnotatePreferredMultilineAssignment(binop);
SortIfSourcesOrDeps(binop);
@@ -625,15 +606,12 @@
// common in .gn files, don't indent them + 4, even though they're just
// continuations when they're simple lists like "x = [ a, b, c, ... ]" or
// scopes like "x = { a = 1 b = 2 }". Put back to "normal" indenting.
- const ListNode* right_as_list = binop->right()->AsList();
- if (right_as_list) {
- if (right_as_list->prefer_multiline() ||
- ListWillBeMultiline(right_as_list->contents(),
+ if (const ListNode* right_as_list = binop->right()->AsList()) {
+ if (ListWillBeMultiline(right_as_list->contents(),
right_as_list->End()))
indent_column = start_column;
} else {
- const BlockNode* right_as_block = binop->right()->AsBlock();
- if (right_as_block)
+ if (binop->right()->AsBlock())
indent_column = start_column;
}
}
@@ -753,10 +731,8 @@
} else if (const IdentifierNode* identifier = root->AsIdentifier()) {
Print(identifier->value().value());
} else if (const ListNode* list = root->AsList()) {
- bool force_multiline =
- list->prefer_multiline() && !list->contents().empty();
Sequence(kSequenceStyleList, list->contents(), list->End(),
- force_multiline);
+ /*force_multiline=*/false);
} else if (const LiteralNode* literal = root->AsLiteral()) {
Print(literal->value().value());
} else if (const UnaryOpNode* unaryop = root->AsUnaryOp()) {
diff --git a/src/gn/format_test_data/003.golden b/src/gn/format_test_data/003.golden
index 2fdcb9e..a5bd5d3 100644
--- a/src/gn/format_test_data/003.golden
+++ b/src/gn/format_test_data/003.golden
@@ -4,7 +4,5 @@
"things.cc",
]
- deps = [
- "//base",
- ]
+ deps = [ "//base" ]
}
diff --git a/src/gn/format_test_data/004.golden b/src/gn/format_test_data/004.golden
index 68373d1..1d000c6 100644
--- a/src/gn/format_test_data/004.golden
+++ b/src/gn/format_test_data/004.golden
@@ -7,7 +7,5 @@
]
# Comment attached to statement.
- deps = [
- "//base",
- ]
+ deps = [ "//base" ]
}
diff --git a/src/gn/format_test_data/007.golden b/src/gn/format_test_data/007.golden
index 610fa3f..2d2b4c1 100644
--- a/src/gn/format_test_data/007.golden
+++ b/src/gn/format_test_data/007.golden
@@ -1,7 +1,5 @@
executable("test") {
- sources = [
- "a.cc",
- ]
+ sources = [ "a.cc" ]
# This is an unusual comment that's a header for the stuff that comes after
# it. We want to make sure that it's not connected to the next element in
diff --git a/src/gn/format_test_data/008.golden b/src/gn/format_test_data/008.golden
index aec281a..8e4c5f1 100644
--- a/src/gn/format_test_data/008.golden
+++ b/src/gn/format_test_data/008.golden
@@ -1,5 +1,3 @@
if (is_win) {
- sources = [
- "win.cc",
- ]
+ sources = [ "win.cc" ]
}
diff --git a/src/gn/format_test_data/009.golden b/src/gn/format_test_data/009.golden
index af27d79..20663f8 100644
--- a/src/gn/format_test_data/009.golden
+++ b/src/gn/format_test_data/009.golden
@@ -1,9 +1,5 @@
if (is_win) {
- sources = [
- "win.cc",
- ]
+ sources = [ "win.cc" ]
} else {
- sources = [
- "linux.cc",
- ]
+ sources = [ "linux.cc" ]
}
diff --git a/src/gn/format_test_data/010.golden b/src/gn/format_test_data/010.golden
index 64fabb9..9512a2e 100644
--- a/src/gn/format_test_data/010.golden
+++ b/src/gn/format_test_data/010.golden
@@ -1,9 +1,5 @@
if (is_win) {
- sources = [
- "win.cc",
- ]
+ sources = [ "win.cc" ]
} else if (is_linux) {
- sources = [
- "linux.cc",
- ]
+ sources = [ "linux.cc" ]
}
diff --git a/src/gn/format_test_data/011.golden b/src/gn/format_test_data/011.golden
index 4ce009e..43d70c5 100644
--- a/src/gn/format_test_data/011.golden
+++ b/src/gn/format_test_data/011.golden
@@ -1,13 +1,7 @@
if (is_win) {
- sources = [
- "win.cc",
- ]
+ sources = [ "win.cc" ]
} else if (is_linux) {
- sources = [
- "linux.cc",
- ]
+ sources = [ "linux.cc" ]
} else {
- sources = [
- "wha.cc",
- ]
+ sources = [ "wha.cc" ]
}
diff --git a/src/gn/format_test_data/012.golden b/src/gn/format_test_data/012.golden
index a0049b2..7e3fc98 100644
--- a/src/gn/format_test_data/012.golden
+++ b/src/gn/format_test_data/012.golden
@@ -5,18 +5,12 @@
if (is_win) {
# This is some special stuff for Windows
- sources = [
- "win.cc",
- ]
+ sources = [ "win.cc" ]
} else if (is_linux) {
# This is a block comment inside the linux block, but not attached.
- sources = [
- "linux.cc",
- ]
+ sources = [ "linux.cc" ]
} else {
# A comment with trailing spaces
- sources = [
- "wha.cc",
- ]
+ sources = [ "wha.cc" ]
}
diff --git a/src/gn/format_test_data/015.golden b/src/gn/format_test_data/015.golden
index 553f01c..8e12688 100644
--- a/src/gn/format_test_data/015.golden
+++ b/src/gn/format_test_data/015.golden
@@ -1,6 +1,4 @@
if (is_win) {
- sources = [
- "a.cc",
- ]
+ sources = [ "a.cc" ]
# Some comment at end.
}
diff --git a/src/gn/format_test_data/042.golden b/src/gn/format_test_data/042.golden
index 1968d0a..2baca56 100644
--- a/src/gn/format_test_data/042.golden
+++ b/src/gn/format_test_data/042.golden
@@ -17,30 +17,16 @@
cflags = [ "x" ]
cflags_c = [ "x" ]
cflags_cc = [ "x" ]
- data = [
- "x",
- ]
- datadeps = [
- "x",
- ]
+ data = [ "x" ]
+ datadeps = [ "x" ]
defines = [ "x" ]
- deps = [
- "x",
- ]
+ deps = [ "x" ]
include_dirs = [ "x" ]
- inputs = [
- "x",
- ]
+ inputs = [ "x" ]
ldflags = [ "x" ]
- outputs = [
- "x",
- ]
- public_deps = [
- "x",
- ]
- sources = [
- "x",
- ]
+ outputs = [ "x" ]
+ public_deps = [ "x" ]
+ sources = [ "x" ]
} else {
cflags = [
"x",
diff --git a/src/gn/format_test_data/062.golden b/src/gn/format_test_data/062.golden
index dc32658..3d653ce 100644
--- a/src/gn/format_test_data/062.golden
+++ b/src/gn/format_test_data/062.golden
@@ -2,9 +2,7 @@
sources = []
-sources = [
- "x.cc",
-]
+sources = [ "x.cc" ]
sources = [
"/a",
diff --git a/src/gn/format_test_data/064.golden b/src/gn/format_test_data/064.golden
index 3ff56f6..c186725 100644
--- a/src/gn/format_test_data/064.golden
+++ b/src/gn/format_test_data/064.golden
@@ -1,5 +1,3 @@
source_set("test") {
- deps = [
- rebase_path(sdk_dep, ".", mojo_root),
- ]
+ deps = [ rebase_path(sdk_dep, ".", mojo_root) ]
}
diff --git a/src/gn/format_test_data/066.golden b/src/gn/format_test_data/066.golden
index 9b7bb5c..bdcf055 100644
--- a/src/gn/format_test_data/066.golden
+++ b/src/gn/format_test_data/066.golden
@@ -4,9 +4,7 @@
sources = []
# NOSORT
-sources = [
- "a",
-]
+sources = [ "a" ]
# NOSORT
sources += [ "a" ]
diff --git a/src/gn/parse_tree.cc b/src/gn/parse_tree.cc
index e1fc5a5..8625e45 100644
--- a/src/gn/parse_tree.cc
+++ b/src/gn/parse_tree.cc
@@ -597,7 +597,7 @@
// ListNode -------------------------------------------------------------------
-ListNode::ListNode() : prefer_multiline_(false) {}
+ListNode::ListNode() {}
ListNode::~ListNode() = default;
diff --git a/src/gn/parse_tree.h b/src/gn/parse_tree.h
index 8d14437..19f0887 100644
--- a/src/gn/parse_tree.h
+++ b/src/gn/parse_tree.h
@@ -419,14 +419,6 @@
void SortAsStringsList();
void SortAsDepsList();
- // During formatting, do we want this list to always be multliline? This is
- // used to make assignments to deps, sources, etc. always be multiline lists,
- // rather than collapsed to a single line when they're one element.
- bool prefer_multiline() const { return prefer_multiline_; }
- void set_prefer_multiline(bool prefer_multiline) {
- prefer_multiline_ = prefer_multiline;
- }
-
struct SortRange {
size_t begin;
size_t end;
@@ -443,7 +435,6 @@
// custom parse node so that it can have comments hung off of it.
Token begin_token_;
std::unique_ptr<EndNode> end_;
- bool prefer_multiline_;
std::vector<std::unique_ptr<const ParseNode>> contents_;
