fix(pm-adapter): plumb body section direction context through resolver#3199
Open
caio-pizzol wants to merge 2 commits intomainfrom
Open
fix(pm-adapter): plumb body section direction context through resolver#3199caio-pizzol wants to merge 2 commits intomainfrom
caio-pizzol wants to merge 2 commits intomainfrom
Conversation
Per ECMA §17.3.1.41, paragraph w:textDirection inherits from the parent section when omitted. The previous call site built sectionContext from `undefined`, so directionContext.writingMode was always 'horizontal-tb' even when the body's w:sectPr declared a vertical writing-mode. This wires SectionDirectionContext through ConverterContext, populated once at top-level conversion from the body sectPr. Paragraphs that omit their own w:textDirection now correctly inherit writing-mode. Scope: - Body-level sectPr only. Per-paragraph-section variation (each section with its own sectPr) and table-cell direction context are not yet plumbed through. Both gaps are documented inline and tracked under SD-2777 (migrate remaining direction-aware consumers). - No consumer currently reads directionContext.writingMode in production, so this fixes the data contract before the first consumer arrives. Tests: - New: paragraph inherits body sectionDirectionContext.writingMode - New: paragraph w:textDirection still wins as explicit override
Codecov Report✅ All modified and coverable lines are covered by tests. 📢 Thoughts on this report? Let us know! |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 7ef0198d01
ℹ️ 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".
Codex finding on PR #3199: when callers reuse one ConverterContext across documents (toFlowBlocksMap does this), the previous `??` cache let the first document's body sectPr resolve once and stick. A vertical doc 1 followed by a horizontal doc 2 would have doc 2's paragraphs inherit doc 1's writing-mode. Fix: drop the `??` and always overwrite. The shared ConverterContext is mutated freshly each call before children read it, so per-document recomputation is enough. (The pre-existing `sectionDirection` field on the line above has the same pattern but is out of scope for this PR.) Test added: toFlowBlocksMap with two docs (vertical w:textDirection then horizontal w:textDirection) sharing one converterContext - asserts each doc's paragraphs get their own writingMode. Failed before the fix because the cached vertical-rl persisted into the horizontal doc; passes after.
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.
Follow-up to #3184. The resolver chain that PR landed builds `sectionContext` from `undefined` at the paragraph call site, so `directionContext.writingMode` is always `'horizontal-tb'` even when the body's `w:sectPr` declares a vertical writing-mode. ECMA-376 §17.3.1.41 specifies that paragraph `w:textDirection` inherits from the parent section when omitted; the data contract was incomplete.
What changes
What it does NOT do
Two known gaps remain, both documented inline and tracked under SD-2777:
Why now
No consumer currently reads `directionContext.writingMode` in production code (grep confirms). Fixing the data contract before the first consumer arrives prevents downstream code from being written against the wrong default.
Validation