Skip to content

A map of my five open PRs: what each one is, and what depends on what #3965

Description

@Nathaniel-260

@caio-pizzol
First, thank you. Five PRs from a first-time contributor is a lot to land on
anyone's week, and you have given them real attention — the notes on #3953 and
#3961 caught things I had genuinely gotten wrong, not style nits. That kind of
review is expensive to give, and I don't take it for granted.

I also owe you an explanation, because the shape of what I opened is my doing
and it is harder to read than it needed to be.

I found one bug — the first column of a Hebrew section landing on the left —
and while fixing it I kept finding more in the same code. Rather than stopping,
I kept branching off my own work. CONTRIBUTING.md asks contributions to target
main, so I targeted main, which means each stacked PR carries its parent's
commits in its own diff. The result is three PRs whose diffs overlap heavily and
two that don't overlap at all, with no way to tell which is which from the PR
list. That's on me, not on the guidelines. This issue is the map I should have
given you on day one.

Two are independent — start here if you want the quick ones

PR What it fixes Size
#3958 Hebrew/Arabic Word writes bold as w:bCs with no w:b. SuperDoc read only w:b, so headings that are bold in Word rendered at normal weight. 2 files, +71
#3963 Safari leaves the RTL caret one position behind after a space. WebKit returns no rect for a collapsed range at the end of a text node; this restores the measurement. 4 files, +1800

Neither touches the column code. Neither depends on anything. They can merge in
any order, today.

Three are a stack, and must merge in order

main
 └─ #3953  place the first column on the right in RTL sections
      ├─ #3961  give the footnote band its own column layout
      └─ #3962  seven defects in column layout and balancing

#3953 is the foundation. ColumnLayout had no notion of direction at all.
This adds it and mirrors the column strip in RTL. Nothing else in the stack
compiles without it.

#3961 sits on #3953. A footnote band in a two-column RTL section printed one
body-column wide with the rule on the wrong side; Word prints one strip across
the content area. Only 2 commits are mine — everything else in its diff is
#3953 and disappears when #3953 merges.

#3962 also sits on #3953 — not on #3961. Nine commits of mine covering seven
defects, most of them found by your review and cubic's. The first two are older
than the RTL work and are reachable in plain LTR documents.

#3961 and #3962 are siblings, not a chain: they don't depend on each other
and can merge in either order once #3953 is in.

Since you squash-merge, both will need a rebase on main after #3953 lands. I'll
do that the moment it merges — say so if you'd rather have it earlier.

Suggested order

  1. fix(style-engine): resolve bold and italic from the complex-script toggles #3958 and fix(v2): keep the RTL caret on the boundary WebKit will not measure #3963 — independent, no coordination needed
  2. fix(layout): place the first column on the right in RTL sections #3953 — unblocks the rest
  3. fix(layout): give the footnote band its own column layout and separator placement #3961 and fix(layout): compare rendered gutters, and order a balanced page by column #3962 — either order, after I rebase

One thing worth knowing before #3953 merges

There is a real boundary defect in getColumnAtX: at an exact RTL column
boundary an inclusive comparison hands a fragment to the wrong column, and with
w:space="0" the two column resolvers disagree outright. cubic caught it on
#3961 and I confirmed it.

The fix lives in #3962, not in #3953. So #3953 and #3961 would merge with it
still present. If you'd rather main never be in that state, say the word and
I'll move that commit down into #3953.

Status

CI is green on all five and the CLA is signed. I've replied to every review
comment except one open note on #3963 about the analysis cache, which I'm
working on now.

Thanks again for the patience. Happy to close, split, or re-target any of these
if a different shape would be easier for you to review.

Activity

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

Type

No type

Projects

No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions