format: Fix incorrect dewrap of `if`
When `if (...stuff...) {` was exactly 81 characters, it would be
incorrectly wrapped because the trailing { wasn't being taken into
account in penalizing single line formatting.
To avoid this, pass the full suffix into the child formatter.
In a Chromium tree, diffing
gn ls-files '*.gn' '*.gni' | xargs -l1 gn format
vs.
gn ls-files '*.gn' '*.gni' | xargs -l1 /work/gn/out/gn format
results in
https://gist.github.com/sgraham/2e33ae36db6255af0be0785ea3d0b058
which are all 81 character long if statements being fixed.
Bug: gn:36
Change-Id: I5af9a62a3de4953ea0d60a85bd558281410a8f09
Reviewed-on: https://gn-review.googlesource.com/c/3621
Reviewed-by: Nico Weber <thakis@chromium.org>
Reviewed-by: Petr Hosek <phosek@google.com>
Commit-Queue: Scott Graham <scottmg@google.com>
diff --git a/tools/gn/command_format.cc b/tools/gn/command_format.cc
index 483a24c..0f7b7b2 100644
--- a/tools/gn/command_format.cc
+++ b/tools/gn/command_format.cc
@@ -112,6 +112,7 @@
enum SequenceStyle {
kSequenceStyleList,
kSequenceStyleBracedBlock,
+ kSequenceStyleBracedBlockAlreadyOpen,
};
struct Metrics {
@@ -722,10 +723,10 @@
false);
} else if (const ConditionNode* condition = root->AsConditionNode()) {
Print("if (");
- // TODO(scottmg): The { needs to be included in the suffix here.
- Expr(condition->condition(), kPrecedenceLowest, ") ");
- Sequence(kSequenceStyleBracedBlock, condition->if_true()->statements(),
- condition->if_true()->End(), false);
+ Expr(condition->condition(), kPrecedenceLowest, ") {");
+ Sequence(kSequenceStyleBracedBlockAlreadyOpen,
+ condition->if_true()->statements(), condition->if_true()->End(),
+ false);
if (condition->if_false()) {
Print(" else ");
// If it's a block it's a bare 'else', otherwise it's an 'else if'. See
@@ -782,10 +783,13 @@
const std::vector<std::unique_ptr<PARSENODE>>& list,
const ParseNode* end,
bool force_multiline) {
- if (style == kSequenceStyleList)
+ if (style == kSequenceStyleList) {
Print("[");
- else if (style == kSequenceStyleBracedBlock)
+ } else if (style == kSequenceStyleBracedBlock) {
Print("{");
+ } else if (style == kSequenceStyleBracedBlockAlreadyOpen) {
+ style = kSequenceStyleBracedBlock;
+ }
if (style == kSequenceStyleBracedBlock) {
force_multiline = true;
diff --git a/tools/gn/command_format_unittest.cc b/tools/gn/command_format_unittest.cc
index db63a8c..f5e0c35 100644
--- a/tools/gn/command_format_unittest.cc
+++ b/tools/gn/command_format_unittest.cc
@@ -109,3 +109,4 @@
FORMAT_TEST(071)
FORMAT_TEST(072)
FORMAT_TEST(073)
+FORMAT_TEST(074)
diff --git a/tools/gn/format_test_data/074.gn b/tools/gn/format_test_data/074.gn
new file mode 100644
index 0000000..d5e5101
--- /dev/null
+++ b/tools/gn/format_test_data/074.gn
@@ -0,0 +1,7 @@
+config("something") {
+ # Makes builds independent of absolute file path.
+ if (symbol_level != 0 && is_clang &&
+ strip_absolute_paths_from_debug_symbols) {
+ print("hi")
+ }
+}
diff --git a/tools/gn/format_test_data/074.golden b/tools/gn/format_test_data/074.golden
new file mode 100644
index 0000000..d5e5101
--- /dev/null
+++ b/tools/gn/format_test_data/074.golden
@@ -0,0 +1,7 @@
+config("something") {
+ # Makes builds independent of absolute file path.
+ if (symbol_level != 0 && is_clang &&
+ strip_absolute_paths_from_debug_symbols) {
+ print("hi")
+ }
+}
