Support pairing each alignment group with a different column extractor. Refactoring internal interface only, no functional change. PiperOrigin-RevId: 331871713
diff --git a/common/formatting/align.cc b/common/formatting/align.cc index 70efa81..8ad8f2a 100644 --- a/common/formatting/align.cc +++ b/common/formatting/align.cc
@@ -488,10 +488,9 @@ // This width calculation accounts for the unaligned tokens in the tail position // of each aligned row (e.g. unaligned trailing comments). -static bool AlignedRowsFitUnderColumnLimit(const AlignablePartitionGroup& rows, - const AlignmentMatrix& matrix, - int total_column_width, - int column_limit) { +static bool AlignedRowsFitUnderColumnLimit( + const std::vector<TokenPartitionIterator>& rows, + const AlignmentMatrix& matrix, int total_column_width, int column_limit) { auto partition_iter = rows.begin(); for (const auto& row : matrix) { if (!row.empty()) { @@ -532,7 +531,7 @@ }; static GroupAlignmentData AlignFilteredRows( - const AlignablePartitionGroup& rows, + const std::vector<TokenPartitionIterator>& rows, const AlignmentCellScannerFunction& cell_scanner_gen, MutableFormatTokenRange::iterator ftoken_base, int column_limit) { VLOG(1) << __FUNCTION__; @@ -651,7 +650,7 @@ // Signal that these partitions spacing/wrapping decisions have already been // solved (append everything because they fit on one line). { - auto partition_iter = rows.begin(); + auto partition_iter = rows.alignable_rows.begin(); for (auto& row : align_data.matrix) { // Commits to appending all tokens in this row (mutates format tokens) CommitAlignmentDecisionToRow(**partition_iter, row, ftoken_base); @@ -661,7 +660,7 @@ VLOG(1) << "end of " << __FUNCTION__; } -static AlignablePartitionGroup FilterAlignablePartitions( +static std::vector<TokenPartitionIterator> FilterAlignablePartitions( const TokenPartitionRange& group, const IgnoreAlignmentRowPredicate& ignore_partition_predicate) { // This partition group may contain partitions that should not be @@ -683,17 +682,19 @@ ExtractAlignmentGroupsFunction ExtractAlignmentGroupsAdapter( const std::function<std::vector<TokenPartitionRange>( const TokenPartitionRange&)>& legacy_extractor, - const IgnoreAlignmentRowPredicate& legacy_ignore_predicate) { - return [legacy_extractor, - legacy_ignore_predicate](const TokenPartitionRange& full_range) { + const IgnoreAlignmentRowPredicate& legacy_ignore_predicate, + const AlignmentCellScannerFunction& alignment_cell_scanner) { + return [legacy_extractor, legacy_ignore_predicate, + alignment_cell_scanner](const TokenPartitionRange& full_range) { // must copy the closures, not just reference, to ensure valid lifetime const std::vector<TokenPartitionRange> ranges(legacy_extractor(full_range)); std::vector<AlignablePartitionGroup> groups; groups.reserve(ranges.size()); for (const auto& range : ranges) { - groups.push_back( - FilterAlignablePartitions(range, legacy_ignore_predicate)); - if (groups.back().empty()) groups.pop_back(); + groups.emplace_back(AlignablePartitionGroup{ + FilterAlignablePartitions(range, legacy_ignore_predicate), + alignment_cell_scanner}); + if (groups.back().alignable_rows.empty()) groups.pop_back(); } return groups; }; @@ -834,8 +835,9 @@ const std::vector<AlignablePartitionGroup> alignment_groups( alignment_handler.extract_alignment_groups(subpartitions_range)); for (const auto& alignment_group : alignment_groups) { - const TokenPartitionRange partition_range(alignment_group.front(), - alignment_group.back() + 1); + const TokenPartitionRange partition_range( + alignment_group.alignable_rows.front(), + alignment_group.alignable_rows.back() + 1); if (partition_range.empty()) continue; if (AnyPartitionSubRangeIsDisabled(partition_range, full_text, disabled_byte_ranges)) { @@ -860,8 +862,8 @@ switch (policy) { case AlignmentPolicy::kAlign: case AlignmentPolicy::kInferUserIntent: - align_data = AlignFilteredRows(alignment_group, - alignment_handler.alignment_cell_scanner, + align_data = AlignFilteredRows(alignment_group.alignable_rows, + alignment_group.alignment_cell_scanner, ftokens->begin(), column_limit); break; default:
diff --git a/common/formatting/align.h b/common/formatting/align.h index 13f95c2..4558cfa 100644 --- a/common/formatting/align.h +++ b/common/formatting/align.h
@@ -129,17 +129,22 @@ partition_selector, int min_match_count = 2); -// This represents one unit of alignable work, which is usually a filtered -// subset of partitions within a contiguous range of partitions. -// TODO(fangism): pair this with an AlignmentCellScannerFunction to be able to -// support heterogeneous subgroup alignment. -using AlignablePartitionGroup = std::vector<TokenPartitionIterator>; - // 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 = std::function<std::vector<ColumnPositionEntry>(const TokenPartitionTree&)>; +// This represents one unit of alignable work, which is usually a filtered +// subset of partitions within a contiguous range of partitions. +struct AlignablePartitionGroup { + // The set of partitions to treat as rows for tabular alignment. + std::vector<TokenPartitionIterator> alignable_rows; + + // This function scans each row to identify column positions and properties of + // alignable cells (containing token ranges). + AlignmentCellScannerFunction alignment_cell_scanner; +}; + // This is the interface used to sub-divide a range of token partitions into // a sequence of sub-ranges for the purposes of formatting aligned groups. using ExtractAlignmentGroupsFunction = @@ -157,7 +162,8 @@ ExtractAlignmentGroupsFunction ExtractAlignmentGroupsAdapter( const std::function<std::vector<TokenPartitionRange>( const TokenPartitionRange&)>& legacy_extractor, - const IgnoreAlignmentRowPredicate& legacy_ignore_predicate); + const IgnoreAlignmentRowPredicate& legacy_ignore_predicate, + const AlignmentCellScannerFunction& alignment_cell_scanner); // Instantiates a ScannerType (implements ColumnSchemaScanner) and extracts // column alignment information, suitable as an AlignmentCellScannerFunction. @@ -236,10 +242,6 @@ // children of a parent partition of interest) into groups of lines that will // align with each other. ExtractAlignmentGroupsFunction extract_alignment_groups; - - // This function scans lines (token ranges) - // for token positions that mark the start of a new column. - AlignmentCellScannerFunction alignment_cell_scanner; }; // This aligns sections of text by modifying the spacing between tokens.
diff --git a/common/formatting/align_test.cc b/common/formatting/align_test.cc index 372c6e4..dc86054 100644 --- a/common/formatting/align_test.cc +++ b/common/formatting/align_test.cc
@@ -110,8 +110,8 @@ static const AlignedFormattingHandler kDefaultAlignmentHandler{ .extract_alignment_groups = ExtractAlignmentGroupsAdapter( - &verible::GetSubpartitionsBetweenBlankLines, &IgnoreNone), - .alignment_cell_scanner = AlignmentCellScannerGenerator<TokenColumnizer>(), + &verible::GetSubpartitionsBetweenBlankLines, &IgnoreNone, + AlignmentCellScannerGenerator<TokenColumnizer>()), }; TEST_F(TabularAlignTokenTest, EmptyPartitionRange) { @@ -282,9 +282,8 @@ static const AlignedFormattingHandler kFlushRightAlignmentHandler{ .extract_alignment_groups = ExtractAlignmentGroupsAdapter( - &verible::GetSubpartitionsBetweenBlankLines, &IgnoreNone), - .alignment_cell_scanner = - AlignmentCellScannerGenerator<TokenColumnizerRightFlushed>(), + &verible::GetSubpartitionsBetweenBlankLines, &IgnoreNone, + AlignmentCellScannerGenerator<TokenColumnizerRightFlushed>()), }; TEST_F(Sparse3x3MatrixAlignmentTest, RightFlushed) { @@ -340,9 +339,8 @@ const AlignedFormattingHandler handler{ .extract_alignment_groups = ExtractAlignmentGroupsAdapter( - &verible::GetSubpartitionsBetweenBlankLines, ignore_threes), - .alignment_cell_scanner = - AlignmentCellScannerGenerator<TokenColumnizer>(), + &verible::GetSubpartitionsBetweenBlankLines, ignore_threes, + AlignmentCellScannerGenerator<TokenColumnizer>()), }; TabularAlignTokens(&partition_, handler, &pre_format_tokens_, sample_, ByteOffsetSet(), AlignmentPolicy::kAlign, 40);
diff --git a/verilog/formatting/align.cc b/verilog/formatting/align.cc index 9715514..84325ef 100644 --- a/verilog/formatting/align.cc +++ b/verilog/formatting/align.cc
@@ -781,49 +781,52 @@ static const verible::AlignedFormattingHandler kPortDeclarationAligner{ .extract_alignment_groups = verible::ExtractAlignmentGroupsAdapter( &verible::GetSubpartitionsBetweenBlankLines, - &IgnoreWithinPortDeclarationPartitionGroup), - .alignment_cell_scanner = - AlignmentCellScannerGenerator<PortDeclarationColumnSchemaScanner>(), + &IgnoreWithinPortDeclarationPartitionGroup, + AlignmentCellScannerGenerator<PortDeclarationColumnSchemaScanner>()), }; static const verible::AlignedFormattingHandler kActualNamedParameterAligner{ .extract_alignment_groups = verible::ExtractAlignmentGroupsAdapter( &verible::GetSubpartitionsBetweenBlankLines, - &IgnoreWithinActualNamedParameterPartitionGroup), - .alignment_cell_scanner = AlignmentCellScannerGenerator< - ActualNamedParameterColumnSchemaScanner>(), + &IgnoreWithinActualNamedParameterPartitionGroup, + AlignmentCellScannerGenerator< + ActualNamedParameterColumnSchemaScanner>()), }; static const verible::AlignedFormattingHandler kActualNamedPortAligner{ .extract_alignment_groups = verible::ExtractAlignmentGroupsAdapter( &verible::GetSubpartitionsBetweenBlankLines, - &IgnoreWithinActualNamedPortPartitionGroup), - .alignment_cell_scanner = - AlignmentCellScannerGenerator<ActualNamedPortColumnSchemaScanner>(), + &IgnoreWithinActualNamedPortPartitionGroup, + AlignmentCellScannerGenerator<ActualNamedPortColumnSchemaScanner>()), }; static const verible::AlignedFormattingHandler kDataDeclarationAligner{ .extract_alignment_groups = verible::ExtractAlignmentGroupsAdapter( &GetConsecutiveDataDeclarationGroups, - &IgnoreCommentsAndPreprocessingDirectives), - .alignment_cell_scanner = - AlignmentCellScannerGenerator<DataDeclarationColumnSchemaScanner>(), + &IgnoreCommentsAndPreprocessingDirectives, + AlignmentCellScannerGenerator<DataDeclarationColumnSchemaScanner>()), }; static const verible::AlignedFormattingHandler kClassPropertyAligner{ .extract_alignment_groups = verible::ExtractAlignmentGroupsAdapter( &GetConsecutiveDataDeclarationGroups, - &IgnoreCommentsAndPreprocessingDirectives), - .alignment_cell_scanner = - AlignmentCellScannerGenerator<ClassPropertyColumnSchemaScanner>(), + &IgnoreCommentsAndPreprocessingDirectives, + AlignmentCellScannerGenerator<ClassPropertyColumnSchemaScanner>()), }; static const verible::AlignedFormattingHandler kCaseItemAligner{ .extract_alignment_groups = verible::ExtractAlignmentGroupsAdapter( &verible::GetSubpartitionsBetweenBlankLines, - &IgnoreMultilineCaseStatements), - .alignment_cell_scanner = - AlignmentCellScannerGenerator<CaseItemColumnSchemaScanner>(), + &IgnoreMultilineCaseStatements, + AlignmentCellScannerGenerator<CaseItemColumnSchemaScanner>()), +}; + +static const verible::AlignedFormattingHandler kParameterDeclarationAligner{ + .extract_alignment_groups = verible::ExtractAlignmentGroupsAdapter( + &verible::GetSubpartitionsBetweenBlankLines, + &IgnoreWithinPortDeclarationPartitionGroup, + AlignmentCellScannerGenerator< + ParameterDeclarationColumnSchemaScanner>()), }; struct AlignedFormattingConfiguration { @@ -835,14 +838,6 @@ std::function<AlignmentPolicy(const FormatStyle&)> policy; }; -static const verible::AlignedFormattingHandler kParameterDeclarationAligner{ - .extract_alignment_groups = verible::ExtractAlignmentGroupsAdapter( - &verible::GetSubpartitionsBetweenBlankLines, - &IgnoreWithinPortDeclarationPartitionGroup), - .alignment_cell_scanner = AlignmentCellScannerGenerator< - ParameterDeclarationColumnSchemaScanner>(), -}; - void TabularAlignTokenPartitions(TokenPartitionTree* partition_ptr, std::vector<PreFormatToken>* ftokens, absl::string_view full_text,