Merge pull request #2525 from sjalloq/fix/alignment_group_boundary
Fix --alignment_group_boundary string parsing
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.
