Skip to content

fix: read xf/@xfId so a cellXfs entry resolves against its own style - #355

Merged
MathNya merged 3 commits into
MathNya:masterfrom
developer0hye:fix/cell-format-xf-id-upstream
Aug 10, 2026
Merged

fix: read xf/@xfId so a cellXfs entry resolves against its own style#355
MathNya merged 3 commits into
MathNya:masterfrom
developer0hye:fix/cell-format-xf-id-upstream

Conversation

@developer0hye

@developer0hye developer0hye commented Aug 9, 2026

Copy link
Copy Markdown
Contributor

What

CellFormat::set_attributes reads numFmtId, fontId, fillId, borderId and
all six apply* flags, but not xfId — even though the struct already carries a
format_id field for it and write_to writes it back out.

get_format_id() therefore always returns 0, so Stylesheet::make_style pairs
every cellXfs entry with cellStyleXfs[0] instead of the style it names:

let def_cell_format = self.cell_style_formats.get_cell_format()
    .get(*cell_format.get_format_id() as usize)   // always 0

Why it matters

get_style_by_cell_format lets that referenced style turn a whole formatting
category off:

let mut apply = true;
if def_cell_format.has_apply_number_format() { apply = *def_cell_format.get_apply_number_format(); }

When cellStyleXfs[0] carries applyNumberFormat="0" — which Excel writes for
the Normal style in many templates — apply is false for every cell in the
workbook and no number format is ever attached. Percent, currency and date cells
all read back as the raw stored number.

Measured on such a workbook (cell holding 0.25 under numFmtId="9",
xfId="6", where cellStyleXfs[6] also declares numFmtId="9"):

workbook get_formatted_value() resolved format
as shipped "0.25" None
+ explicit <numFmt numFmtId="9" formatCode="0%"/> "0.25" None
applyNumberFormat="0" removed from cellStyleXfs[0] "25%" Some("0%")

0 of its 294 cells resolved a number format. Adding the format definition changes
nothing — only the wrong default xf matters.

Reading the attribute also stops a round trip from rewriting every xfId as 0,
which currently loses the cell-style association on save.

Tests

Three unit tests in src/structs/cell_format.rs: a declared xfId is read from a
self-closing element and from one with children, and an absent one stays 0.

cargo test on this branch: 109 + 113 + 4 + 79 pass.

cargo clippy -- -D warnings and cargo test are green on 1.88.0, stable and
nightly; cargo +nightly fmt --all --check is clean.

Note

The second commit is unrelated to the xfId fix and is here only to get CI
green. Current nightly clippy denies nonstandard_macro_braces, so
cargo clippy -- -D warnings fails on master at src/writer/csv.rs:47:

error: use of irregular braces for `format!` macro
  --> src/writer/csv.rs:47:25

Because that job aborts the matrix, the other three toolchains are cancelled
before they finish and every open pull request reads as red. The commit
switches that one call to parentheses, which is what clippy suggests; rustfmt
then wraps the arguments, since the brace form had exempted it. Happy to split
it out if you would rather take it separately.

The third commit swaps the deprecated get_format_id / get_number_format_id
in the new tests for format_id / number_format_id, so the suite compiles
without deprecated warnings.

`CellFormat::set_attributes` read `numFmtId`, `fontId`, `fillId`,
`borderId` and every `apply*` flag, but not `xfId`, even though the
struct carries a `format_id` field for it and `write_to` writes it back
out. `get_format_id()` therefore always returned 0, so
`Stylesheet::make_style` paired every `cellXfs` entry with
`cellStyleXfs[0]`.

That default stand-in can veto the cell's own formatting:
`get_style_by_cell_format` lets the referenced style turn a category off,
so a workbook whose Normal style declares `applyNumberFormat="0"` — which
Excel writes for plenty of templates — lost the number format on **every**
cell. A percent cell read back its stored `0.25` instead of `25%`, and
dates and currency went the same way.

Reading it also stops a round trip from rewriting each `xfId` as 0.

Signed-off-by: developer0hye <developer.0hye@gmail.com>
Current nightly clippy denies `nonstandard_macro_braces` for `format!`, so
`cargo clippy -- -D warnings` fails on master:

    error: use of irregular braces for `format!` macro
      --> src/writer/csv.rs:47:25

That aborts the build matrix before the other toolchains finish, so every
open pull request reads as red regardless of its own contents.

Switching to parentheses is the fix clippy suggests; rustfmt then wraps the
arguments, which the brace form had exempted it from. No behaviour change.

Signed-off-by: developer0hye <developer.0hye@gmail.com>
`get_format_id` and `get_number_format_id` are deprecated since 3.0.0 in
favour of `format_id` and `number_format_id`, so the new tests compiled with
four `deprecated` warnings. Use the current names.

Signed-off-by: developer0hye <developer.0hye@gmail.com>
@MathNya

MathNya commented Aug 10, 2026

Copy link
Copy Markdown
Owner

@developer0hye
Thank you for the PR.
I'll go ahead and merge it.

@MathNya
MathNya merged commit 4c24569 into MathNya:master Aug 10, 2026
5 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