Introduce --format-width option (and format_width .gn variable). The change in [1] considerably changed the output of `gn format` which makes a new version of GN unrollable to the Fuchsia tree (as this breaks many shac tests). Since fixing all build files in Fuchsia is going to take too long, this introduces a new .gn variable named 'format_width' and associated command-line flag --format-width to override the default to a new value. By setting `format_width` to 120, the changes to the Fuchsia build files are minimized, making this roll easier to do. [1]: https://gn-review.googlesource.com/c/gn/+/22000 Change-Id: I089b7c82bb2e5de96b46b69a54c37255e53bc480 Reviewed-on: https://gn-review.googlesource.com/c/gn/+/23020 Commit-Queue: David Turner <digit@google.com> Reviewed-by: Andrew Grieve <agrieve@google.com>
diff --git a/docs/reference.md b/docs/reference.md index 5a509df..75b75a0 100644 --- a/docs/reference.md +++ b/docs/reference.md
@@ -699,7 +699,7 @@ Shows defines set for the //base:base target, annotated by where each one was set from. ``` -### <a name="cmd_format"></a>**gn format [\--dump-tree] (\--stdin | <list of build_files...>)** [Back to Top](#gn-reference) +### <a name="cmd_format"></a>**gn format [\--dump-tree] [\--format-width=WIDTH] (\--stdin | <list of build_files...>)** [Back to Top](#gn-reference) ``` Formats .gn file to a standard format. @@ -725,6 +725,10 @@ - Exit code 1: general failure (parse error, etc.) - Exit code 2: successful format, but differs from on disk. + --format-width=WIDTH + Override the default format width. WIDTH must be a strictly positive + integer. + --dump-tree[=( text | json )] Dumps the parse tree to stdout and does not update the file or print formatted output. If no format is specified, text format will be used. @@ -8589,6 +8593,7 @@ * --enumerate-files-with-git: Use git to list files. * --error-limit: Limit the number of errors or warnings to print. * --fail-on-unused-args: Treat unused build args as fatal errors. + * --format-width: Set the formatting width (default is 80) * --markdown: Write help output in the Markdown format. * --ninja-executable: Set the Ninja executable. * --nocolor: Force non-colored output.
diff --git a/src/gn/command_format.cc b/src/gn/command_format.cc index f7649cd..b49c480 100644 --- a/src/gn/command_format.cc +++ b/src/gn/command_format.cc
@@ -12,6 +12,7 @@ #include "base/files/file_util.h" #include "base/json/json_reader.h" #include "base/json/json_writer.h" +#include "base/strings/string_number_conversions.h" #include "base/strings/string_split.h" #include "base/strings/string_util.h" #include "gn/commands.h" @@ -33,6 +34,8 @@ namespace commands { +const size_t kDefaultFormatWidth = 80; + const char kSwitchDryRun[] = "dry-run"; const char kSwitchDumpTree[] = "dump-tree"; const char kSwitchReadTree[] = "read-tree"; @@ -43,7 +46,7 @@ const char kFormat[] = "format"; const char kFormat_HelpShort[] = "format: Format .gn files."; const char kFormat_Help[] = - R"(gn format [--dump-tree] (--stdin | <list of build_files...>) + R"(gn format [--dump-tree] [--format-width=WIDTH] (--stdin | <list of build_files...>) Formats .gn file to a standard format. @@ -66,6 +69,10 @@ - Exit code 1: general failure (parse error, etc.) - Exit code 2: successful format, but differs from on disk. + --format-width=WIDTH + Override the default format width. WIDTH must be a strictly positive + integer. + --dump-tree[=( text | json )] Dumps the parse tree to stdout and does not update the file or print formatted output. If no format is specified, text format will be used. @@ -91,7 +98,6 @@ namespace { const int kIndentSize = 2; -const int kMaximumWidth = 80; const int kPenaltyLineBreak = 500; const int kPenaltyHorizontalSeparation = 100; @@ -130,7 +136,7 @@ class Printer { public: - Printer(); + Printer(size_t format_width); ~Printer(); void Block(const ParseNode* file); @@ -247,11 +253,12 @@ bool ListWillBeMultiline(const std::vector<std::unique_ptr<PARSENODE>>& list, const ParseNode* end); + size_t format_width_ = kDefaultFormatWidth; std::string output_; // Output buffer. std::vector<Token> comments_; // Pending end-of-line comments. int margin() const { return stack_.back().margin; } - int penalty_depth_; + int penalty_depth_ = 0; int GetPenaltyForLineBreak() const { return penalty_depth_ * kPenaltyLineBreak; } @@ -285,7 +292,7 @@ Printer& operator=(const Printer&) = delete; }; -Printer::Printer() : penalty_depth_(0) { +Printer::Printer(size_t format_width) : format_width_(format_width) { output_.reserve(100 << 10); precedence_["="] = kPrecedenceAssign; precedence_["+="] = kPrecedenceAssign; @@ -402,7 +409,7 @@ is_indented || has_url || is_list || is_pragma; paragraphs.push_back(p); } else { - if (margin() + trimmed.size() > kMaximumWidth) { + if (margin() + trimmed.size() > format_width_) { current_paragraph.should_reflow = true; } current_paragraph.tokens.push_back(c); @@ -455,7 +462,7 @@ } if (!have_empty_line && j < words.size() - 1 && - CurrentColumn() + 1 + words[j + 1].size() > kMaximumWidth) { + CurrentColumn() + 1 + words[j + 1].size() > format_width_) { start_next_line(); continuation = true; } @@ -862,8 +869,8 @@ output, "\n", base::KEEP_WHITESPACE, base::SPLIT_WANT_ALL); penalty += static_cast<int>(lines.size() - 1) * GetPenaltyForLineBreak(); for (const auto& line : lines) { - if (line.size() > kMaximumWidth) - penalty += static_cast<int>(line.size() - kMaximumWidth) * kPenaltyExcess; + if (line.size() > format_width_) + penalty += static_cast<int>(line.size() - format_width_) * kPenaltyExcess; } return penalty; } @@ -873,7 +880,7 @@ base::SPLIT_WANT_ALL)) { std::string_view trimmed = TrimString(line, " ", base::TrimPositions::TRIM_TRAILING); - if (trimmed.size() > kMaximumWidth) { + if (trimmed.size() > format_width_) { return true; } } @@ -980,7 +987,7 @@ stack_.push_back(IndentState(indent_column, stack_.back().continuation_requires_indent, binop->op().value() == "||")); - Printer sub_left; + Printer sub_left(format_width_); InitializeSub(&sub_left); sub_left.Expr(binop->left(), prec_left, std::string(" ") + std::string(binop->op().value())); @@ -992,7 +999,7 @@ std::back_inserter(comments_)); // Single line. - Printer sub1; + Printer sub1(format_width_); InitializeSub(&sub1); sub1.Print(" "); int penalty_current_line = sub1.Expr(binop->right(), prec_right, at_end); @@ -1007,7 +1014,7 @@ } // Break after operator. - Printer sub2; + Printer sub2(format_width_); InitializeSub(&sub2); sub2.Newline(); int penalty_next_line = sub2.Expr(binop->right(), prec_right, at_end); @@ -1018,7 +1025,7 @@ // Force a list on the RHS that would normally be a single line into // multiline. bool tried_rhs_multiline = false; - Printer sub3; + Printer sub3(format_width_); InitializeSub(&sub3); int penalty_multiline_rhs_list = std::numeric_limits<int>::max(); const ListNode* rhs_list = binop->right()->AsList(); @@ -1238,7 +1245,7 @@ list.size() != 1 || !list[0]->AsBinaryOp(); // 1: Same line. - Printer sub1; + Printer sub1(format_width_); InitializeSub(&sub1); sub1.stack_.push_back( IndentState(CurrentColumn(), continuation_requires_indent, false)); @@ -1256,7 +1263,7 @@ (CountLines(sub1.String()) - 1) * kPenaltyBrokenLineOnOneLiner; // 2: Starting on same line, broken at commas. - Printer sub2; + Printer sub2(format_width_); InitializeSub(&sub2); sub2.stack_.push_back( IndentState(CurrentColumn(), continuation_requires_indent, false)); @@ -1273,7 +1280,7 @@ penalty_multiline_start_same_line += AssessPenalty(sub2.String()); // 3: Starting on next line, broken at commas. - Printer sub3; + Printer sub3(format_width_); InitializeSub(&sub3); sub3.stack_.push_back(IndentState(margin() + kIndentSize * 2, continuation_requires_indent, false)); @@ -1402,6 +1409,7 @@ void DoFormat(const ParseNode* root, TreeDumpMode dump_tree, + size_t format_width, std::string* output, std::string* dump_output) { if (dump_tree == TreeDumpMode::kPlainText) { @@ -1415,23 +1423,26 @@ *dump_output = os; } - Printer pr; + Printer pr(format_width); pr.Block(root); *output = pr.String(); } } // namespace -bool FormatJsonToString(const std::string& json, std::string* output) { +bool FormatJsonToString(const std::string& json, + size_t format_width, + std::string* output) { base::JSONReader reader; std::unique_ptr<base::Value> json_root = reader.Read(json); std::unique_ptr<ParseNode> root = ParseNode::BuildFromJSON(*json_root); - DoFormat(root.get(), TreeDumpMode::kInactive, output, nullptr); + DoFormat(root.get(), TreeDumpMode::kInactive, format_width, output, nullptr); return true; } bool FormatStringToString(const std::string& input, TreeDumpMode dump_tree, + size_t maximum_width, std::string* output, std::string* dump_output) { SourceFile source_file; @@ -1453,7 +1464,7 @@ return false; } - DoFormat(parse_node.get(), dump_tree, output, dump_output); + DoFormat(parse_node.get(), dump_tree, maximum_width, output, dump_output); return true; } @@ -1464,13 +1475,12 @@ _setmode(_fileno(stderr), _O_BINARY); #endif - bool dry_run = - base::CommandLine::ForCurrentProcess()->HasSwitch(kSwitchDryRun); + const base::CommandLine* cmdline = base::CommandLine::ForCurrentProcess(); + + bool dry_run = cmdline->HasSwitch(kSwitchDryRun); TreeDumpMode dump_tree = TreeDumpMode::kInactive; - if (base::CommandLine::ForCurrentProcess()->HasSwitch(kSwitchDumpTree)) { - std::string tree_type = - base::CommandLine::ForCurrentProcess()->GetSwitchValueString( - kSwitchDumpTree); + if (cmdline->HasSwitch(kSwitchDumpTree)) { + std::string tree_type = cmdline->GetSwitchValueString(kSwitchDumpTree); if (tree_type == kSwitchTreeTypeJSON) { dump_tree = TreeDumpMode::kJSON; } else if (tree_type.empty() || tree_type == kSwitchTreeTypeText) { @@ -1485,16 +1495,29 @@ return 1; } } - bool from_stdin = - base::CommandLine::ForCurrentProcess()->HasSwitch(kSwitchStdin); + bool from_stdin = cmdline->HasSwitch(kSwitchStdin); if (dry_run) { // --dry-run only works with an actual file to compare to. from_stdin = false; } - bool quiet = - base::CommandLine::ForCurrentProcess()->HasSwitch(switches::kQuiet); + bool quiet = cmdline->HasSwitch(switches::kQuiet); + + size_t format_width = kDefaultFormatWidth; + if (cmdline->HasSwitch(switches::kFormatWidth)) { + const std::string format_width_str = + cmdline->GetSwitchValueString(switches::kFormatWidth); + int format_width_int = 0; + if (!base::StringToInt( + cmdline->GetSwitchValueString(switches::kFormatWidth), + &format_width_int) || + format_width_int <= 0) { + Err(Location(), "Invalid --format-width value: " + format_width_str); + return 1; + } + format_width = static_cast<size_t>(format_width_int); + } if (from_stdin) { if (args.size() != 0) { @@ -1502,10 +1525,13 @@ .PrintToStdout(); return 1; } + std::string input = ReadStdin(); std::string output; std::string dump_output; - if (!FormatStringToString(input, dump_tree, &output, &dump_output)) + + if (!FormatStringToString(input, dump_tree, format_width, &output, + &dump_output)) return 1; printf("%s", dump_output.c_str()); printf("%s", output.c_str()); @@ -1522,10 +1548,8 @@ SourceDir source_dir = SourceDirForCurrentDirectory(setup.build_settings().root_path()); - if (base::CommandLine::ForCurrentProcess()->HasSwitch(kSwitchReadTree)) { - std::string tree_type = - base::CommandLine::ForCurrentProcess()->GetSwitchValueString( - kSwitchReadTree); + if (cmdline->HasSwitch(kSwitchReadTree)) { + std::string tree_type = cmdline->GetSwitchValueString(kSwitchReadTree); if (tree_type != kSwitchTreeTypeJSON) { Err(Location(), "Only json supported for read-tree.\n").PrintToStdout(); return 1; @@ -1546,7 +1570,7 @@ } base::FilePath to_format = setup.build_settings().GetFullPath(file); std::string output; - FormatJsonToString(ReadStdin(), &output); + FormatJsonToString(ReadStdin(), format_width, &output); if (base::WriteFile(to_format, output.data(), static_cast<int>(output.size())) == -1) { Err(Location(), std::string("Failed to write output to \"") + @@ -1585,8 +1609,8 @@ std::string output_string; std::string dump_output_string; - if (!FormatStringToString(original_contents, dump_tree, &output_string, - &dump_output_string)) { + if (!FormatStringToString(original_contents, dump_tree, format_width, + &output_string, &dump_output_string)) { exit_code = 1; continue; }
diff --git a/src/gn/command_format.h b/src/gn/command_format.h index 0eb667f..7854d9e 100644 --- a/src/gn/command_format.h +++ b/src/gn/command_format.h
@@ -24,10 +24,16 @@ kJSON }; -bool FormatJsonToString(const std::string& input, std::string* output); +// The default format width. +extern const size_t kDefaultFormatWidth; + +bool FormatJsonToString(const std::string& input, + size_t format_width, + std::string* output); bool FormatStringToString(const std::string& input, TreeDumpMode dump_tree, + size_t format_width, std::string* output, std::string* dump_output);
diff --git a/src/gn/command_format_unittest.cc b/src/gn/command_format_unittest.cc index 0149cbc..4a8c597 100644 --- a/src/gn/command_format_unittest.cc +++ b/src/gn/command_format_unittest.cc
@@ -14,40 +14,46 @@ using FormatTest = TestWithScheduler; -#define FORMAT_TEST(n) \ - TEST_F(FormatTest, n) { \ - ::Setup setup; \ - std::string input; \ - std::string out; \ - std::string expected; \ - base::FilePath src_dir = \ - GetExePath().DirName().Append(FILE_PATH_LITERAL("..")); \ - base::SetCurrentDirectory(src_dir); \ - ASSERT_TRUE(base::ReadFileToString( \ - base::FilePath(FILE_PATH_LITERAL("src/gn/format_test_data/") \ - FILE_PATH_LITERAL(#n) FILE_PATH_LITERAL(".gn")), \ - &input)); \ - ASSERT_TRUE(base::ReadFileToString( \ - base::FilePath(FILE_PATH_LITERAL("src/gn/format_test_data/") \ - FILE_PATH_LITERAL(#n) \ - FILE_PATH_LITERAL(".golden")), \ - &expected)); \ - EXPECT_TRUE(commands::FormatStringToString( \ - input, commands::TreeDumpMode::kInactive, &out, nullptr)); \ - EXPECT_EQ(expected, out); \ - /* Make sure formatting the output doesn't cause further changes. */ \ - std::string out_again; \ - EXPECT_TRUE(commands::FormatStringToString( \ - out, commands::TreeDumpMode::kInactive, &out_again, nullptr)); \ - ASSERT_EQ(out, out_again); \ - /* Make sure we can roundtrip to json without any changes. */ \ - std::string as_json; \ - std::string unused; \ - EXPECT_TRUE(commands::FormatStringToString( \ - out_again, commands::TreeDumpMode::kJSON, &unused, &as_json)); \ - std::string rewritten; \ - EXPECT_TRUE(commands::FormatJsonToString(as_json, &rewritten)); \ - ASSERT_EQ(out, rewritten); \ +#define FORMAT_TEST(n) FORMAT_TEST_WITH_WIDTH(n, commands::kDefaultFormatWidth) + +#define FORMAT_TEST_WITH_WIDTH(n, format_width) \ + TEST_F(FormatTest, n) { \ + ::Setup setup; \ + std::string input; \ + std::string out; \ + std::string expected; \ + base::FilePath src_dir = \ + GetExePath().DirName().Append(FILE_PATH_LITERAL("..")); \ + base::SetCurrentDirectory(src_dir); \ + ASSERT_TRUE(base::ReadFileToString( \ + base::FilePath(FILE_PATH_LITERAL("src/gn/format_test_data/") \ + FILE_PATH_LITERAL(#n) FILE_PATH_LITERAL(".gn")), \ + &input)); \ + ASSERT_TRUE(base::ReadFileToString( \ + base::FilePath(FILE_PATH_LITERAL("src/gn/format_test_data/") \ + FILE_PATH_LITERAL(#n) \ + FILE_PATH_LITERAL(".golden")), \ + &expected)); \ + EXPECT_TRUE(commands::FormatStringToString( \ + input, commands::TreeDumpMode::kInactive, format_width, &out, \ + nullptr)); \ + EXPECT_EQ(expected, out); \ + /* Make sure formatting the output doesn't cause further changes. */ \ + std::string out_again; \ + EXPECT_TRUE( \ + commands::FormatStringToString(out, commands::TreeDumpMode::kInactive, \ + format_width, &out_again, nullptr)); \ + ASSERT_EQ(out, out_again); \ + /* Make sure we can roundtrip to json without any changes. */ \ + std::string as_json; \ + std::string unused; \ + EXPECT_TRUE(commands::FormatStringToString( \ + out_again, commands::TreeDumpMode::kJSON, format_width, &unused, \ + &as_json)); \ + std::string rewritten; \ + EXPECT_TRUE( \ + commands::FormatJsonToString(as_json, format_width, &rewritten)); \ + ASSERT_EQ(out, rewritten); \ } // These are expanded out this way rather than a runtime loop so that @@ -158,3 +164,4 @@ FORMAT_TEST(103) FORMAT_TEST(104) FORMAT_TEST(105) +FORMAT_TEST_WITH_WIDTH(106, 20)
diff --git a/src/gn/format_test_data/106.gn b/src/gn/format_test_data/106.gn new file mode 100644 index 0000000..c49d312 --- /dev/null +++ b/src/gn/format_test_data/106.gn
@@ -0,0 +1 @@ +# This is a line that exceeds the formatting width of 20
diff --git a/src/gn/format_test_data/106.golden b/src/gn/format_test_data/106.golden new file mode 100644 index 0000000..3497b38 --- /dev/null +++ b/src/gn/format_test_data/106.golden
@@ -0,0 +1,4 @@ +# This is a line +# that exceeds the +# formatting width +# of 20
diff --git a/src/gn/setup.cc b/src/gn/setup.cc index 555e34c..132d711 100644 --- a/src/gn/setup.cc +++ b/src/gn/setup.cc
@@ -15,6 +15,7 @@ #include "base/files/file_path.h" #include "base/files/file_util.h" #include "base/memory/ref_counted.h" +#include "base/strings/string_number_conversions.h" #include "base/strings/string_split.h" #include "base/strings/string_util.h" #include "base/strings/utf_string_conversions.h" @@ -690,7 +691,8 @@ std::string contents = args_input_file_->contents(); commands::FormatStringToString(contents, commands::TreeDumpMode::kInactive, - &contents, nullptr); + commands::kDefaultFormatWidth, &contents, + nullptr); #if defined(OS_WIN) // Use Windows lineendings for this file since it will often open in // Notepad which can't handle Unix ones.
diff --git a/src/gn/setup.h b/src/gn/setup.h index 59ec134..4004891 100644 --- a/src/gn/setup.h +++ b/src/gn/setup.h
@@ -63,6 +63,10 @@ const base::CommandLine& cmdline, Err* err); + // Setup just enough data for the 'format' command, which doesn't require + // a build directory. + bool DoSetupForFormat(); + // Runs the load, returning true on success. On failure, prints the error // and returns false. This includes both RunPreMessageLoop() and // RunPostMessageLoop().
diff --git a/src/gn/switches.cc b/src/gn/switches.cc index 5442776..61b3425 100644 --- a/src/gn/switches.cc +++ b/src/gn/switches.cc
@@ -112,6 +112,17 @@ flag to force GN to fail in that case. )"; +const char kFormatWidth[] = "format-width"; +const char kFormatWidth_HelpShort[] = + "--format-width: Set the formatting width (default is 80)"; + +const char kFormatWidth_Help[] = + R"("--format-width: Set the formatting width, + + Override the format width (default is 80) used by the 'format' + command. This takes a strictly positive integer decimal value. +)"; + const char kMarkdown[] = "markdown"; const char kMarkdown_HelpShort[] = "--markdown: Write help output in the Markdown format."; @@ -359,6 +370,7 @@ INSERT_VARIABLE(EnumerateFilesWithGit) INSERT_VARIABLE(ErrorLimit) INSERT_VARIABLE(FailOnUnusedArgs) + INSERT_VARIABLE(FormatWidth) INSERT_VARIABLE(Markdown) INSERT_VARIABLE(NinjaExecutable) INSERT_VARIABLE(NoColor)
diff --git a/src/gn/switches.h b/src/gn/switches.h index 2c7c42b..70f0f19 100644 --- a/src/gn/switches.h +++ b/src/gn/switches.h
@@ -50,6 +50,10 @@ extern const char kFailOnUnusedArgs_HelpShort[]; extern const char kFailOnUnusedArgs_Help[]; +extern const char kFormatWidth[]; +extern const char kFormatWidth_HelpShort[]; +extern const char kFormatWidth_Help[]; + extern const char kMarkdown[]; extern const char kMarkdown_HelpShort[]; extern const char kMarkdown_Help[];