Fix --alignment_group_boundary string parsing The initial implementation used to find separator comments was too strict: it required the entire comment body to be 4+ repeats of a single character. This failed to identify dividers that also contain text, e.g. "// ---- Registers ----". The fix scans for a run of 4 or more consecutive identical "divider" characters (any non-alphanumeric, non-whitespace character) anywhere in the comment, ignoring surrounding text.
diff --git a/verible/verilog/formatting/BUILD b/verible/verilog/formatting/BUILD index baa7b60..6150594 100644 --- a/verible/verilog/formatting/BUILD +++ b/verible/verilog/formatting/BUILD
@@ -40,6 +40,7 @@ "//verible/verilog/parser:verilog-token-classifications", "//verible/verilog/parser:verilog-token-enum", "@abseil-cpp//absl/log:die_if_null", + "@abseil-cpp//absl/strings", ], )
diff --git a/verible/verilog/formatting/align.cc b/verible/verilog/formatting/align.cc index d19fa2c..1f7f472 100644 --- a/verible/verilog/formatting/align.cc +++ b/verible/verilog/formatting/align.cc
@@ -22,6 +22,7 @@ #include <vector> #include "absl/log/die_if_null.h" +#include "absl/strings/ascii.h" #include "verible/common/formatting/align.h" #include "verible/common/formatting/format-token.h" #include "verible/common/formatting/token-partition-tree.h" @@ -95,15 +96,18 @@ } // Returns true if the partition contains a separator comment line. -// A separator comment is an EOL comment whose body (after stripping "//" -// and optional leading whitespace) consists of 4 or more consecutive -// identical characters. Examples: "// ----", "// ====", -// "/////////////////////". +// A separator comment is an EOL comment whose body contains a run of 4 or +// more consecutive identical "divider" characters, where a divider character +// is any non-alphanumeric, non-whitespace character (e.g. '-', '=', '*', +// '#', '/'). The run may be surrounded by other text, so both a bare rule +// and a captioned one are recognised. Examples: "// ----", "// ====", +// "/////////////////////", "// ------ section heading ------". static bool IsSeparatorComment(const TokenPartitionTree &partition) { const auto &uwline = partition.Value(); const auto token_range = uwline.TokensRange(); if (token_range.empty()) return false; + constexpr int kMinRunLength = 4; for (const auto &ftoken : token_range) { if (ftoken.TokenEnum() != static_cast<int>(verilog_tokentype::TK_EOL_COMMENT)) { @@ -113,19 +117,21 @@ std::string_view text = ftoken.Text(); // Strip leading "//" if (text.size() < 2 || text[0] != '/' || text[1] != '/') continue; - std::string_view body = text.substr(2); + const std::string_view body = text.substr(2); - // Strip optional leading whitespace - const auto start = body.find_first_not_of(" \t"); - if (start == std::string_view::npos) continue; - body = body.substr(start); - - // Check for 4+ consecutive identical characters - if (body.size() < 4) continue; - const char first = body[0]; - if (std::all_of(body.begin(), body.end(), - [first](char ch) { return ch == first; })) { - return true; + // Look for a run of kMinRunLength or more consecutive identical divider + // characters anywhere in the comment body. + int run = 0; + char prev = '\0'; + for (const char ch : body) { + const bool is_divider = + !absl::ascii_isalnum(ch) && !absl::ascii_isspace(ch); + if (is_divider && ch == prev) { + if (++run >= kMinRunLength) return true; + } else { + run = is_divider ? 1 : 0; + prev = ch; + } } } return false;
diff --git a/verible/verilog/formatting/formatter_test.cc b/verible/verilog/formatting/formatter_test.cc index 8710052..e885d74 100644 --- a/verible/verilog/formatting/formatter_test.cc +++ b/verible/verilog/formatting/formatter_test.cc
@@ -18938,6 +18938,36 @@ " assign baaaaz = 1'b1;\n" " assign c = 1'b0;\n" "endmodule\n"}, + {// Captioned divider (text between separator runs) also breaks + "module m;\n" + "assign foo = 1'b1;\n" + "assign baar = 1'b0;\n" + "// ------ section heading ------\n" + "assign baaaaz = 1'b1;\n" + "assign c = 1'b0;\n" + "endmodule\n", + "module m;\n" + " assign foo = 1'b1;\n" + " assign baar = 1'b0;\n" + " // ------ section heading ------\n" + " assign baaaaz = 1'b1;\n" + " assign c = 1'b0;\n" + "endmodule\n"}, + {// Leading-only divider run with trailing caption text + "module m;\n" + "assign foo = 1'b1;\n" + "assign baar = 1'b0;\n" + "// ==== Registers\n" + "assign baaaaz = 1'b1;\n" + "assign c = 1'b0;\n" + "endmodule\n", + "module m;\n" + " assign foo = 1'b1;\n" + " assign baar = 1'b0;\n" + " // ==== Registers\n" + " assign baaaaz = 1'b1;\n" + " assign c = 1'b0;\n" + "endmodule\n"}, {// No space after // also works "module m;\n" "assign foo = 1'b1;\n"
diff --git a/verible/verilog/tools/formatter/README.md b/verible/verilog/tools/formatter/README.md index 5a54654..0dbafdb 100644 --- a/verible/verilog/tools/formatter/README.md +++ b/verible/verilog/tools/formatter/README.md
@@ -369,10 +369,12 @@ * `none` (default): no additional splitting; the whole section is one group. * `blank-lines`: a blank line starts a new alignment group. * `separator-comments`: a separator comment starts a new alignment group. A - separator comment is a `//` comment on its own line whose text (after the - `//`) is four or more consecutive identical characters, e.g. `// ----`, - `// ====`, or `/////`. A trailing comment after code on the same line does - not count. + separator comment is a `//` comment on its own line whose text contains a + run of four or more consecutive identical "divider" characters (any + non-alphanumeric, non-whitespace character). The run may be surrounded by + caption text, so all of `// ----`, `// ====`, `/////`, and + `// ------ section heading ------` count. A trailing comment after code on + the same line does not count. * `blank-lines-and-separator-comments`: both blank lines and separator comments start a new alignment group.