diff --git a/common/formatting/BUILD b/common/formatting/BUILD index 5ad35ecd6..65e86301b 100644 --- a/common/formatting/BUILD +++ b/common/formatting/BUILD @@ -40,6 +40,7 @@ cc_test( ":token_partition_tree", ":unwrapped_line_test_utils", "//common/text:tree_builder_test_util", + "//common/util:range", "//common/util:spacer", "@com_google_absl//absl/strings", "@com_google_googletest//:gtest_main", diff --git a/common/formatting/align.cc b/common/formatting/align.cc index 1b7d8088b..d98a83b6d 100644 --- a/common/formatting/align.cc +++ b/common/formatting/align.cc @@ -660,4 +660,42 @@ void TabularAlignTokens(TokenPartitionTree* partition_ptr, VLOG(1) << "end of " << __FUNCTION__; } +std::vector GetPartitionAlignmentSubranges( + const TokenPartitionRange& partitions, + const std::function& + partition_selector, + int min_match_count) { + std::vector result; + + // Grab ranges of consecutive data declarations with >= 2 elements. + int match_count = 0; + auto last_range_start = partitions.begin(); + for (auto iter = last_range_start; iter != partitions.end(); ++iter) { + switch (partition_selector(*iter)) { + case AlignmentGroupAction::kIgnore: + continue; + case AlignmentGroupAction::kMatch: { + if (match_count == 0) { + // This is the start of a new range of interest. + last_range_start = iter; + } + ++match_count; + break; + } + case AlignmentGroupAction::kNoMatch: { + if (match_count >= min_match_count) { + result.emplace_back(last_range_start, iter); + } + match_count = 0; // reset + break; + } + } // switch + } // for + // Flush out the last range. + if (match_count >= min_match_count) { + result.emplace_back(last_range_start, partitions.end()); + } + return result; +} + } // namespace verible diff --git a/common/formatting/align.h b/common/formatting/align.h index e22916e9f..985e66f87 100644 --- a/common/formatting/align.h +++ b/common/formatting/align.h @@ -88,6 +88,44 @@ class ColumnSchemaScanner : public TreeContextPathVisitor { std::vector sparse_columns_; }; +// This enum drives partition sub-range selection in the +// GetPartitionAlignmentSubranges() function. +enum class AlignmentGroupAction { + kIgnore, // This does not influence the current matching range. + kMatch, // Include this partition in the current matching range. + kNoMatch, // Close the current matching range (if any). +}; + +// From a range of token 'partitions', this selects sub-ranges to align. +// 'partition_selector' decides which partitions qualify for alignment. +// 'min_match_count' sets the minimum sub-range size to return. +// +// Visualization from 'partition_selector's perspective: +// +// case 1: +// nomatch +// match // not enough matches to yield a group for min_match_count=2 +// nomatch +// +// case 2: +// nomatch +// match // an alignment group starts here +// match // ends here, inclusively +// nomatch +// +// case 3: +// nomatch +// match // an alignment group starts here +// ignore // ... continues ... +// match // ends here, inclusively +// nomatch +// +std::vector GetPartitionAlignmentSubranges( + const TokenPartitionRange& partitions, + const std::function& + partition_selector, + int min_match_count = 2); + // This is the interface used to extract alignment cells from ranges of tokens. // Note that it is not required to use a ColumnSchemaScanner. using AlignmentCellScannerFunction = diff --git a/common/formatting/align_test.cc b/common/formatting/align_test.cc index 18b1fe2ea..3488200d8 100644 --- a/common/formatting/align_test.cc +++ b/common/formatting/align_test.cc @@ -21,11 +21,14 @@ #include "common/formatting/token_partition_tree.h" #include "common/formatting/unwrapped_line_test_utils.h" #include "common/text/tree_builder_test_util.h" +#include "common/util/range.h" #include "common/util/spacer.h" namespace verible { namespace { +using ::testing::ElementsAre; + // Helper class that initializes an array of tokens to be partitioned // into TokenPartitionTree. class AlignmentTestFixture : public ::testing::Test, @@ -560,5 +563,94 @@ TEST_F(MultiAlignmentGroupTest, BlankLineSeparatedGroups) { // TODO(fangism): test case for demonstrating flush-right +class GetPartitionAlignmentSubrangesTestFixture : public AlignmentTestFixture { + public: + GetPartitionAlignmentSubrangesTestFixture() + : AlignmentTestFixture( + "ignore match nomatch match match match nomatch nomatch match " + "ignore match"), + syntax_tree_(TNode( + 1, // one token per partition for simplicity + TNode(2, Leaf(1, tokens_[0])), // + TNode(2, Leaf(1, tokens_[1])), // singleton range too short here + TNode(2, Leaf(1, tokens_[2])), // + TNode(2, Leaf(1, tokens_[3])), // expect match from here + TNode(2, Leaf(1, tokens_[4])), // ... + TNode(2, Leaf(1, tokens_[5])), // ... to here (inclusive) + TNode(2, Leaf(1, tokens_[6])), // + TNode(2, Leaf(1, tokens_[7])), // + TNode(2, Leaf(1, tokens_[8])), // and from here to the end(). + TNode(2, Leaf(1, tokens_[9])), // ... + TNode(2, Leaf(1, tokens_[10])))), // ... + partition_(/* temporary */ UnwrappedLine()) { + // Establish format token ranges per partition. + const auto begin = pre_format_tokens_.begin(); + UnwrappedLine all(0, begin); + all.SpanUpToToken(pre_format_tokens_.end()); + all.SetOrigin(&*syntax_tree_); + + std::vector uwlines; + for (int i = 0; i < pre_format_tokens_.size(); ++i) { + uwlines.emplace_back(0, begin + i); + uwlines.back().SpanUpToToken(begin + i + 1); + uwlines.back().SetOrigin( + DescendPath(*syntax_tree_, {static_cast(i)})); + } + + // Construct 2-level token partition. + using tree_type = TokenPartitionTree; + partition_ = tree_type{ + all, + tree_type{uwlines[0]}, + tree_type{uwlines[1]}, // one match not enough + tree_type{uwlines[2]}, + tree_type{uwlines[3]}, // start of match + tree_type{uwlines[4]}, // ... + tree_type{uwlines[5]}, // ... + tree_type{uwlines[6]}, // end of match + tree_type{uwlines[7]}, + tree_type{uwlines[8]}, // start of match + tree_type{uwlines[9]}, // ... + tree_type{uwlines[10]}, // ... + }; + } + + protected: + static AlignmentGroupAction PartitionSelector( + const TokenPartitionTree& partition) { + const absl::string_view text = + partition.Value().TokensRange().front().Text(); + if (text == "match") { + return AlignmentGroupAction::kMatch; + } else if (text == "nomatch") { + return AlignmentGroupAction::kNoMatch; + } else { + return AlignmentGroupAction::kIgnore; + } + } + + protected: + // Syntax tree from which token partition originates. + SymbolPtr syntax_tree_; + + // Format token partitioning (what would be the result of TreeUnwrapper). + TokenPartitionTree partition_; +}; + +TEST_F(GetPartitionAlignmentSubrangesTestFixture, VariousRanges) { + const TokenPartitionRange children(partition_.Children().begin(), + partition_.Children().end()); + + const std::vector ranges( + GetPartitionAlignmentSubranges(children, &PartitionSelector)); + + using P = std::pair; + std::vector

range_indices; + for (const auto& range : ranges) { + range_indices.push_back(SubRangeIndices(range, children)); + } + EXPECT_THAT(range_indices, ElementsAre(P(3, 6), P(8, 11))); +} + } // namespace } // namespace verible diff --git a/verilog/formatting/align.cc b/verilog/formatting/align.cc index 4fa1e530a..a32efc8b9 100644 --- a/verilog/formatting/align.cc +++ b/verilog/formatting/align.cc @@ -42,6 +42,7 @@ namespace formatter { using verible::AlignmentCellScannerGenerator; using verible::AlignmentColumnProperties; +using verible::AlignmentGroupAction; using verible::ByteOffsetSet; using verible::ColumnSchemaScanner; using verible::down_cast; @@ -342,40 +343,20 @@ static bool IsAlignableDeclaration(const SyntaxTreeNode& node) { } static std::vector GetConsecutiveDataDeclarationGroups( - const TokenPartitionRange& outer_partition_bounds) { + const TokenPartitionRange& partitions) { VLOG(2) << __FUNCTION__; - std::vector result; - - // Grab ranges of consecutive data declarations with >= 2 elements. - int consecutive_count = 0; - auto last_range_start = outer_partition_bounds.begin(); - for (auto iter = last_range_start; iter != outer_partition_bounds.end(); - ++iter) { - const Symbol* origin = iter->Value().Origin(); - if (origin == nullptr) continue; - const verible::SymbolTag symbol_tag = origin->Tag(); - if (symbol_tag.kind != verible::SymbolKind::kNode) continue; - const SyntaxTreeNode& node = verible::SymbolCastToNode(*origin); - if (IsAlignableDeclaration(node)) { - if (consecutive_count == 0) { - // This is the start of a new range of interest. - last_range_start = iter; - } - ++consecutive_count; - } else { - if (consecutive_count >= 2) { - result.emplace_back(last_range_start, iter); - } - consecutive_count = 0; // reset - } - } - // Flush out the last range. - if (consecutive_count >= 2) { - result.emplace_back(last_range_start, outer_partition_bounds.end()); - } - - VLOG(2) << "end of " << __FUNCTION__; - return result; + return GetPartitionAlignmentSubranges( + partitions, // + [](const TokenPartitionTree& partition) -> AlignmentGroupAction { + const Symbol* origin = partition.Value().Origin(); + if (origin == nullptr) return AlignmentGroupAction::kIgnore; + const verible::SymbolTag symbol_tag = origin->Tag(); + if (symbol_tag.kind != verible::SymbolKind::kNode) + return AlignmentGroupAction::kIgnore; + const SyntaxTreeNode& node = verible::SymbolCastToNode(*origin); + return IsAlignableDeclaration(node) ? AlignmentGroupAction::kMatch + : AlignmentGroupAction::kNoMatch; + }); } // Much of the implementation of this scanner was based on