Skip to content

Follow inserted, deleted and moved columns in diff and merge - #4

Open
thisiseesmaeel wants to merge 3 commits into
MichaelFowler1:mainfrom
thisiseesmaeel:columns
Open

thisiseesmaeel wants to merge 3 commits into
MichaelFowler1:mainfrom
thisiseesmaeel:columns

Conversation

@thisiseesmaeel

Copy link
Copy Markdown

What changed

Rows have been followed through diff and merge since 0.1; the README said "Inserted columns show as changed cells for now." This makes columns work the same way, through the same code.

  • Diff: column inserted, column deleted, column moved and columns inserted/deleted (empty ones) lines, with the column's cells summarised by full coordinate (D1: 'Gas', D2: 60, …). A sheet's diff is whichever of plain cells, rows or columns reads shortest. Markdown (PR comment) and the HTML view show them too: an inserted or moved column gets a marked header and cells, a deleted one a note line.
  • Merge: if one branch inserted, deleted or moved columns and the other edited cells, the edits land in the columns where their cells now are, with formula references renumbered the way Excel does. Relative and absolute refs (B2, $B$2, B$2, $B2), ranges (B2:D9, shrinking or #REF!), whole-column refs (B:D, SUM(#REF!) when deleted), cross-sheet refs ('Other Sheet'!C3), refs on other sheets pointing at the shifted sheet, and defined names all follow. An edit to a cell in a column the other branch deleted is a conflict; clearing it agrees with the deletion.
  • Same rules as rows: if both branches changed a sheet's rows or columns, or one branch changed both rows and columns on a sheet, that sheet merges cell by cell with a note. The branch that moved lines is the base of the result (so Excel's own renumbering of its tables, charts, names and widths is kept), swapping to theirs when only they moved lines.
  • Column-anchored objects, on both axes (these two also change the row merge, where the same gap existed):
    • A table's range on the branch that didn't move lines is moved through the map before the three-way, so "they insert a column inside the table, we add a row" gives a table with both (the row case "they insert rows above the table, we extend it" used to come out one row short).
    • A chart or pivot edited on that branch, pointing at a sheet where lines moved, still comes over but is listed as a conflict ("check its ranges"), instead of coming over silently with stale ranges.
  • The text diff's kind column grows from 14 to 15 characters to fit "column inserted" (two exact-string asserts and the README example updated).

Design

Everything the row path did was index arithmetic that didn't care which axis it ran on, so it now takes an Axis (ROW / COL: the event words, the Excel limit, the anchor line for formula shapes, how to split and build a coordinate). _by_row → _by_line, _shape anchors on the axis, _align/_aligned_diff are unchanged in logic, RowMap → AxisMap (with move(coord) and move_ref(range)), row_map → axis_map (same gates: the aligned reading must be shorter than plain and must line up strictly more base cells than leaving every line in place). rewrite_refs keeps its shape; _move_ref dispatches on the map's axis. Bare column letters only count as a whole-column ref inside a two-part range (B:D), because openpyxl's tokenizer labels names like Tax as range operands too.

plan_rows → plan_moves computes both maps per sheet and branch and applies the rules above. merge_sheet maps coordinates with AxisMap.move. merge_table_xml takes the sheet's map. flag_stale_ranges reports taken charts (by the sheets their <c:f> series point at) and pivot bundles (by their worksheet source and the sheet they sit on).

Nothing is duplicated: the row tests pass unchanged through the generalised code, and row_map / row_op_eligible / verify_rows remain as one-line wrappers.

Limitations remaining

  • Both branches changing lines on one sheet, or one branch changing both rows and columns on one sheet, still merge cell by cell (noted, never guessed).
  • The branch that moved lines is trusted for its own tables, charts, pivots and names (Excel wrote them). The other branch's chart and pivot edits are flagged, not rewritten.
  • Column widths, merged cells and conditional formatting on the non-moving branch aren't merged (as before).
  • Recognition is by content, like rows: an inserted empty column shows up only as an offset jump, and deleting one of several identical columns can't be told apart, so that sheet merges cell by cell.
  • A cell formula whose references change shape with a column shift (e.g. =B2+C2 in column D becoming =B2+D2 in E after an insert at C) shows as a changed cell in the diff, exactly as =SUM(D2:D6) → =SUM(D2:D7) does for rows.

Follow-ups I'd be happy to do, if wanted

  • Rewrite the other branch's chart series ranges and pivot locations through the map (the way cell formulas, names and table ranges already are) instead of flagging them. Small and self-contained once this is in; it would improve the row merge the same way.
  • Compose a row map and a column map on one sheet, so "ours inserted rows, theirs inserted columns" and "one branch did both" also merge instead of falling back to cell by cell. Bigger, and it overlaps with the roadmap's "both branches inserting rows" item, so I'd open an issue first.

How it was tested

  • 101 tests pass (69 before + 32 new), on Python 3.14. New tests, all building base/ours/theirs with XlsxWriter the way Excel would write them after the edit:
    • tests/test_cols.py: diff lines for inserted, deleted, moved and empty columns; plain edits stay cell by cell; text, markdown and HTML output.
    • tests/test_col_merge.py: theirs inserts a column and ours edits to its right (both ways round); theirs deletes a column and ours edits another column; ours edits a cell in the deleted column → conflict; clearing it agrees; a clash keeps your value in the right column; both branches change columns → cell by cell; rows on one branch and columns on the other → cell by cell; rows and columns on one branch → cell by cell; the row merge still works; every reference form above, for an insert and a delete, on the shifted sheet, from another sheet and in a defined name, with each branch as the mover.
    • tests/test_col_objects.py, through real git merge with the driver installed: a chart, a table and a defined name survive a column insert (both ways round); a table widened by one branch and extended by the other comes out with both; a chart edited on the other branch is flagged.
  • Fuzzer: fuzz/merge_fuzz.py --mode cols (new) and --mode rows, over the ~1,000 workbooks from the open-source test suites (fetch_corpus.sh --no-enron), two random edits per file, with --mode rows on main run on the same sample and seed as a baseline:
    • sample of 120 files × 2 rounds: cols 38 ok / 0 fail (column change recognised exactly in 14); rows on this branch 52 ok / 0 fail (24 recognised), identical to main.
    • full corpus, --mode cols, 996 files × 2 rounds (1,988 of 1,992 runs completed; the harness hung on the last four files): 304 ok / 0 fail, column change recognised exactly in 130, 1,684 skipped (sheet too complex for the harness's own column shifter, too small, or unreadable input).
    • --mode rows on a 400-file sample, this branch vs main (same files, same seed): 80 ok / 0 fail, row change recognised exactly in 36, and identical status, recognition and conflict count on every single run, so the shared code did not change row behaviour.
    • Most runs skip because these test-suite sheets carry merged cells, tables, drawings or pivots the harness's own line shifter doesn't move; the Enron corpus would give more passing runs.

The row code (alignment by content, the map from base rows to where they
went, formula renumbering) was index arithmetic that didn't care which
axis it ran on, so it now takes one: _by_line, _shape, _align,
_aligned_diff, AxisMap and axis_map work on rows or columns, and
rewrite_refs moves column letters under a column map the way it moves
row numbers under a row map (cell refs with any $ mix, ranges that shrink
or become #REF!, whole-column B:D, cross-sheet and from other sheets,
named ranges through merge_names).

Diffs show "column inserted / deleted / moved" lines, with full
coordinates in the summary, and pick whichever of plain, rows or columns
reads shortest. The text output's kind column grows from 14 to 15
characters to fit "column inserted".

The merge follows one branch's column changes exactly as it follows row
changes, and merges cell by cell, with a note, when both branches moved
lines on a sheet or one branch moved both rows and columns. Two things
the row path left alone are now handled on both axes: a table's range on
the branch that didn't move lines is moved before the three-way, so
"they insert a column inside the table, we add a row" merges; and a
chart or pivot that branch edited, pointing at a moved sheet, is flagged
as a conflict instead of coming over silently with stale ranges.
The row machinery (its own regex-based formula shifter, the op that
rewrites sheet XML the way Excel would, the expectation worked out from
the lines xlgit matched) takes an axis. Column shifts also move <cols>
widths along and match whole-column refs.

This branch has not been deployed

No deployments
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.

1 participant