diff --git a/merge_split_test.go b/merge_split_test.go index 3b7498a..021cf81 100644 --- a/merge_split_test.go +++ b/merge_split_test.go @@ -115,6 +115,52 @@ func TestMergeSplitTokensLeavesRealColumnsAlone(t *testing.T) { } } +// TestMergeSplitTokensKeepsTableRectangular is the property that makes +// the feature safe to use on a real table. +// +// A column boundary belongs to the table, not to one row. If the merge +// decision were made per-row, a header band that happens to contain the +// split would collapse while the data rows below it did not — leaving +// rows with different column counts, so the header's second date would +// sit above the first column of figures. A sheared grid is worse than +// the split it set out to fix. +func TestMergeSplitTokensKeepsTableRectangular(t *testing.T) { + // Row 0 has the split ("Dec"+"31,"); row 1 does not — its two cells + // are a genuine column apart. + chars := []Char{ + // row 0, y 560-568: adjacent glyphs across the boundary + {Text: "c", X0: 110, X1: 118, Y0: 560, Y1: 568}, + {Text: "3", X0: 118.02, X1: 126, Y0: 560, Y1: 568}, + // row 1, y 540-548: a wide gutter across the same boundary + {Text: "A", X0: 92, X1: 100, Y0: 540, Y1: 548}, + {Text: "9", X0: 160, X1: 168, Y0: 540, Y1: 548}, + } + cells := [][]BBox{ + {{X0: 90, X1: 118, Y0: 559, Y1: 569}, {X0: 118, X1: 180, Y0: 559, Y1: 569}}, + {{X0: 90, X1: 118, Y0: 539, Y1: 549}, {X0: 118, X1: 180, Y0: 539, Y1: 549}}, + } + rows := [][]string{{"Dec", "31,"}, {"A", "9"}} + + gotRows, gotCells := mergeSplitTokens(rows, cells, chars, 3) + + if len(gotRows) != 2 { + t.Fatalf("got %d rows, want 2", len(gotRows)) + } + if len(gotRows[0]) != len(gotRows[1]) { + t.Fatalf("rows have different column counts (%d vs %d) — the grid sheared: %q", + len(gotRows[0]), len(gotRows[1]), gotRows) + } + if len(gotCells[0]) != len(gotCells[1]) { + t.Errorf("cell rows have different lengths (%d vs %d)", len(gotCells[0]), len(gotCells[1])) + } + // The boundary split a token in row 0, so it goes for the whole table — + // row 1 merges too, keeping the grid rectangular. + want := [][]string{{"Dec31,"}, {"A9"}} + if !reflect.DeepEqual(gotRows, want) { + t.Errorf("rows = %q, want %q", gotRows, want) + } +} + // TestMergeSplitTokensIsOptIn pins the default. pdfplumber produces the // same splits (verified against pdfplumber 0.11.9), so turning this on // by default would silently break the parity this package promises. diff --git a/page.go b/page.go index da528d7..5e6d1a3 100644 --- a/page.go +++ b/page.go @@ -724,27 +724,81 @@ func mergeSplitTokens(rows [][]string, cells [][]BBox, chars []Char, tol float64 if tol <= 0 { tol = 3 } + cols := 0 + for _, r := range rows { + if len(r) > cols { + cols = len(r) + } + } + if cols < 2 { + return rows, cells + } + + // A column boundary is a property of the TABLE, not of one row. Decide + // once, over every row, whether boundary ci|ci+1 cuts a token — then + // apply that decision uniformly. + // + // Doing this per-row instead would merge the header band (where the + // split occurs) while leaving the data rows alone, so rows would end + // up with different column counts and the grid would shear: the + // header's second date would sit above the first column of figures. + // A ragged table is a worse outcome than the split it set out to fix. + drop := make([]bool, cols) + for ci := 0; ci+1 < cols; ci++ { + for ri := range cells { + if ci+1 >= len(cells[ri]) { + continue + } + l, r := cells[ri][ci], cells[ri][ci+1] + if l.IsZero() || r.IsZero() { + continue + } + if rows[ri][ci] == "" || rows[ri][ci+1] == "" { + continue + } + if boundarySplitsToken(chars, l, r, tol) { + drop[ci] = true + break + } + } + } + anyDrop := false + for _, d := range drop { + if d { + anyDrop = true + break + } + } + if !anyDrop { + return rows, cells + } + outRows := make([][]string, len(rows)) outCells := make([][]BBox, len(cells)) - for ri := range rows { var rowText []string var rowCells []BBox - for ci := range rows[ri] { + for ci := 0; ci < cols; ci++ { + text := "" + if ci < len(rows[ri]) { + text = rows[ri][ci] + } cell := BBox{} if ri < len(cells) && ci < len(cells[ri]) { cell = cells[ri][ci] } - text := rows[ri][ci] - - // Try to append to the previous cell rather than start a new - // one. Only ever merges leftwards, so a run of fragments - // collapses into a single cell in one pass. - if n := len(rowText); n > 0 && text != "" && rowText[n-1] != "" && - !cell.IsZero() && !rowCells[n-1].IsZero() && - boundarySplitsToken(chars, rowCells[n-1], cell, tol) { - rowText[n-1] += text - rowCells[n-1] = rowCells[n-1].Union(cell) + // drop[ci-1] means the boundary to this cell's LEFT is gone, + // so it belongs to the cell before it. + if ci > 0 && drop[ci-1] && len(rowText) > 0 { + n := len(rowText) - 1 + rowText[n] += text + if !cell.IsZero() { + if rowCells[n].IsZero() { + rowCells[n] = cell + } else { + rowCells[n] = rowCells[n].Union(cell) + } + } continue } rowText = append(rowText, text)