Skip to content

Conversation

@mawelborn
Copy link
Contributor

This PR fixes etloutput's parsing of cells into rows and columns when some of those cells span rows and/or columns. Previously, rowspans and colspans were not accounted for. This caused the indices to be wrong in Table.rows or Table.columns for cells that followed spanning cells.

For example, given the following table with Delta having a colspan=2:

| Alfa | Bravo | Charlie |
|------|-------|---------|
|    Delta     | Echo    |

etloutput would parse it into rows:

[
    [Alfa, Bravo, Charlie],
    [Delta, Echo],
]

Incorrectly putting Echo at indices row 1, column 1 instead of row 1, column 2. Both row 1, column 0 and row 1, column 1 should be Delta.

This also affects EtlOutput.table_cell_for() as it previously used binary search to skip over rows when finding the cell that a token appears in. But this doesn't work correctly when cells can span rows and/or columns. This has been replaced with a simpler, less performant linear search of all cells in the table. At present, I'm not aware of a logarithmic search algorithm that would speed this up whilst still being correct.

To it's credit, linear search is easier to read and comprehend, is correct with no edge cases, and rhymes with the table search right above it. So it's arguably an improvement, time complexity notwithstanding.

@mawelborn mawelborn self-assigned this Sep 10, 2025
@mawelborn mawelborn merged commit 1dbad06 into main Sep 10, 2025
10 checks passed
@mawelborn mawelborn deleted the mawelborn/fix-rowspan-colspan branch September 10, 2025 19:05
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants