feat(xlsx): honour header/footer font and size codes - #799
Merged
Conversation
&"Font,Style" and &<n> select the face and size for the runs after them. parse_hf_format_string parsed both and discarded them -- the branches were "skip to closing quote" and "skip digits" -- so every header and footer printed in the fallback serif. Each section becomes a Vec<HfSegment> rather than one String, so a code partway through a section applies from that point rather than to the whole slot. &"-,Bold" keeps the face; a style word replaces both flags, so Excel's Regular turns them off. bold/italic stay None when no style word appears, leaving the renderer's default. Measured on headerFooterTest.xlsx: embedded fonts go from LibertinusSerif-Regular + Carlito-Regular to Carlito alone, i.e. the header text stops taking the fallback serif. The &<n> path is implemented and unit-tested but that fixture sets no size code, so the 11-vs-12pt difference #633 also reports is untouched: measured, it is Excel's default header size rather than a discarded code. Related: #633 Signed-off-by: developer0hye <developer.0hye@gmail.com>
8 tasks
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.
Summary
headerFooter/oddHeaderandoddFooterstrings carry&"Font,Style"and&<n>codes selecting the face and size for the runs after them.
parse_hf_format_stringparsed both and threw them away — the branches wereliterally "skip to closing quote" and "skip digits" — so every header and footer
printed in the fallback serif.
Measured
tests/fixtures/xlsx/headerFooterTest.xlsx, whose six slots each set&"Calibri,Regular". Embedded fonts:LibertinusSerif-Regular+Carlito-RegularCarlito-RegularaloneThe serif is gone: header and footer text now takes Calibri, which resolves to
Carlito here. Cell text was already Carlito, so the serif was the header/footer
and nothing else.
Change
Each section becomes a
Vec<HfSegment>rather than oneString. A font or sizecode opens a new segment carrying the updated style, so a code partway through a
section applies from that point on rather than to the whole slot —
&Lplain&"Calibri,Regular"styledyields two runs, only the second with a face.&"-,Bold"keeps the current face and sets the style; a style word replaces bothflags, so Excel's
Regularturns them back off.bold/italicstayNonewhenno style word appears, leaving the renderer's own default rather than pinning
false.Scope
The
&<n>size code is parsed and applied, but this fixture sets no sizecode, so its 11pt-vs-12pt difference is not addressed here. The issue reports
that as part of the same defect; measured, it is separate — Excel's default
header size, not a discarded code — and changing a default on this evidence
would be guessing. The code path is covered by a unit test instead.
Blast radius
3 of 69 XLSX fixtures change:
headerFooterTest,date,date_1904— the oneswith font codes in their header strings.
A correction to earlier PRs in this series while I am here: my local
mainbuildhad been failing since #790 (a gitignored
Cargo.lockpinning adocx-rscommit without
word_wrap), andcargo build's error was going into a line Idiscarded, so
target/releasestayed stale. Blast-radius counts I reported for#793 through #797 were measured against that stale binary and are inflated —
this PR's own sweep initially read 5 of 69 and drops to 3 once main is built
correctly, the two extras being #795/#796's icon changes misattributed here.
Every targeted before/after in those PRs was checked against a committed GT, so
those numbers stand; the sweep counts do not.
Tests
Four. One caught a real bug:
open_segmentcloned the previous segment toinherit an unchanged face and brought its text along, duplicating the earlier
run's text into the new one.
Visual impact
No rendered PDF change
Rendered PDF change or visual evidence added
Reason: the change is a font family selection, and this environment has
neither Calibri nor the GT's Arial installed — both resolve to metric-
compatible substitutes (Carlito, Liberation Sans), so a rendered comparison
here would show the substitution rather than the fix. The embedded-font list
above is the evidence: the fallback serif is no longer used for header text.
tests/fixtures/xlsx/headerFooterTest.xlsxhas no committed GT.Related: #633