feat(docx): space paragraphs and size table rows - #705
Merged
Conversation
`w:spacing` becomes the paragraph's top and bottom margin and its line height, `w:trHeight` the table row's height, and `w:contextualSpacing` drops the spacing towards a neighbour of the same style so a list stays tight. A table now resolves its `w:tblStyle`, whose paragraph and text properties the element cascade carries into the table — without it the doc defaults would loosen every cell. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_012jwYugqjwV6vTLJYRbFgtP
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: b4d806978a
ℹ️ About Codex in GitHub
Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
A `w:sdt` renders as nothing but its children and a `w:bookmarkEnd` is not content at all, yet either sitting between two paragraphs of the same style kept `w:contextualSpacing` from seeing them as neighbours. The lookup now walks document order: it steps over marker elements, descends into the wrappers and climbs back out of them, and stops at a `w:tbl` or the end of the container so a paragraph never neighbours one outside it. Also records what `w:lineRule="atLeast"` does not do: css has no minimum line height, so it lowers to the same fixed `line-height` as `exact`. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_012jwYugqjwV6vTLJYRbFgtP
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
🤖 Generated with Claude Code
A
.docxwas rendered with none of the spacing Word gives it: paragraphs satline against line and every table row collapsed onto its text. The renderer was
not at fault —
html/document_style.cpphas emittedmargin-top/bottom,line-heightand a rowheightall along. The docx style reader simply neverproduced them:
resolve_paragraph_style_readw:jcandw:indand nothingelse, and
resolve_table_row_style_was an empty// TODO.What it reads now
w:spacing→w:before/w:afteras the paragraph's top and bottommargin,
w:lineas its line height, honouringw:lineRule:autois240ths of a line and becomes a percentage,
atLeast/exactare twips. Aw:beforeAutospacing/w:afterAutospacingflag suppresses the value itshadows rather than applying a number Word would have ignored.
w:trPr/w:trHeight→ the table row's height, skipped forw:hRule="auto". CSS gives a<tr>height minimum semantics, which is whatatLeastmeans (Word omits the rule for it); anexactcap is notexpressible on a row and is not attempted.
What had to come with it
Both alone would have been a net regression, so this PR also carries:
w:contextualSpacing— Word drops the spacing towards a neighbouringparagraph of the same style, which is what keeps a list tight. Without it
every
ListParagraphlist gained 8pt between items. It is decided perparagraph rather than per style, so
partial_paragraph_stylecompares thew:pStyleof the adjacentw:psiblings and zeroes the margin it appliesto;
Stylecarries the flag beside itsResolvedStyleso an inherited oneis seen.
w:tblStyleresolution — a table style carries the paragraph and textproperties of everything in the table (
TableGridsetsw:spacing w:after="0"). Without it the doc defaults loosened every cell. A table nowresolves its style reference the way a paragraph resolves
w:pStyleandcontributes the whole resolved style, which the existing element-tree cascade
carries down. Its conditional formats (
w:tblStylePr) are still ignored.ooxml_utilgainsread_on_off_attributeforST_OnOffin both its shapes.Verification
ooxml_text_style_test.cpp, inline XML, no fixtures..docxin the test data was rendered and compared against the previousreference output in a browser. Tables stay tight, lists stay tight, headings
gain their real space-before, and
sample1.docx's calendar table now rendersright-aligned grey day numbers in tall rows — what Word shows, and what the
old output missed entirely.
Reference output
Regenerated and pushed; both pins advanced. 16 files in
odr-public, 3 inodr-private, alldocx/*/document.html— no other format moved.