Skip to content

Read PPTX merge attributes from table cells to preserve merged content - #1077

Merged
WaylandYang merged 1 commit into
deeplethe:devfrom
Floating-Y:fix/pptx-merge-attributes
Oct 4, 2026
Merged

WaylandYang merged 1 commit into
deeplethe:devfrom
Floating-Y:fix/pptx-merge-attributes

Conversation

@Floating-Y

@Floating-Y Floating-Y commented Oct 4, 2026 •

Copy link
Copy Markdown
Contributor

Why

DrawingML stores gridSpan, hMerge, and vMerge on a:tc, but the PPTX parser read them from a:tcPr. Correctly generated slides therefore lost their merge information. The existing fixtures used the same incorrect attribute location, and the empty vertical continuation could pass without reading vMerge.

Follow-up to #1023 and #1042.

What changes

  • Read merge attributes when entering a:tc, preserving the existing defaults and boolean parsing. Remove both a:tcPr merge-reading branches so neither XML event form can overwrite cell state.
  • Keep horizontal continuations skipped, vertical continuations empty, and the existing table rendering and text handling unchanged.
  • Correct the fixture helpers and exercise both <a:tcPr/> and <a:tcPr></a:tcPr>. Nonempty diagnostic markers in horizontal and vertical continuations must disappear, while the expected table shape and values remain.

How it was checked

Local verification during implementation:

  • Corrected fixtures against the old parser: 9 passed, 2 failed, exposing both diagnostic markers.
  • cargo test -p utopia-ingest --test pptx_tables: 11 passed after the fix.
  • cargo test -p utopia-ingest: 159 passed, including related table, single-column, and surrounding-text coverage.
  • cargo clippy -p utopia-ingest --all-targets -- -D warnings: passed.
  • cargo fmt --all --check: passed.
  • Generated a complete two-slide PPTX with python-pptx (40 package parts), inspected the actual cell attributes, and verified both merged-table outputs through the existing render_doc example. This is separate from the minimal parser fixtures; no PowerPoint UI validation was performed.

Before submission, rebased onto dev at 663881d; the intervening upstream changes only touch utopia-reason/src/derive.rs, with no ingest or dependency changes. Reviewed the resulting two-file diff and reran git diff --check. Full workspace Clippy/tests and frontend checks were not run locally; All six CI jobs passed for commit 8fce6f4: CI run.

Before review

  • Every commit is signed off (git commit -s).
  • cargo fmt --all --check and the ingest-specific tests and Clippy checks pass locally.
  • Full workspace Clippy, tests, and build pass in CI.

Signed-off-by: Floating-Y <118035379+Floating-Y@users.noreply.github.com>

@WaylandYang WaylandYang left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks @Floating-Y. You are right, and I missed it when I reviewed #1042: the fixtures put the merge attributes where the parser looked, so the test and the parser agreed with each other and not with DrawingML.

Checked with a real file written by python-pptx, one horizontal and one vertical merge. It writes <a:tc gridSpan="2"> and <a:tc hMerge="1">, with an empty <a:tcPr/> after. On dev the header row reads | Summary | | Total |; on this branch | | Summary | Total |, the grid renderer's shape for a spanning header. Landing it.

@WaylandYang
WaylandYang merged commit 17ce574 into deeplethe:dev Oct 4, 2026
6 checks passed
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.

2 participants