Skip to content

fix: preserve color selectors and avoid loaded-sheet style cloning - #365

Merged
MathNya merged 7 commits into
MathNya:masterfrom
PSU3D0:fix/upstream-3.1-migration
Sep 11, 2026
Merged

MathNya merged 7 commits into
MathNya:masterfrom
PSU3D0:fix/upstream-3.1-migration

Conversation

@PSU3D0

@PSU3D0 PSU3D0 commented Sep 9, 2026

Copy link
Copy Markdown
Contributor

Summary

  • Preserve OOXML color selector identity, tint, automatic colors, literal RGB colors, and expanded color elements across styles, including gradient stops.
  • Avoid cloning the workbook stylesheet when a lazy-loaded worksheet is already deserialized; retain the existing lazy deserialization behavior and public NotFound/NotDeserialized semantics.
  • Add independent OOXML fidelity coverage, a public lazy-sheet correctness regression, and a reproducible public-API probe at examples/loaded_sheet_access_probe.rs (it reports timings without assertions).
  • Apply the one-line nightly rustfmt update required in src/reader/driver.rs.

Motivation and behavior

Border::set_attributes previously parsed into a discarded clone. Color hashes also lacked selector-kind separation (theme="1" versus indexed="1"), while border hashes omitted theme identity. The fixes preserve the serialized selector and tint rather than flattening colors to display RGB. Font, border, pattern-fill, and gradient-stop readers now accept both empty and expanded color elements.

Explicit set_argb/set_argb_str calls now keep RGB identity even for palette colors. This intentionally changes callers' indexed-color introspection after setting RGB; callers that want an indexed selector can continue using set_indexed. The additive automatic-color API preserves explicit true/false and clears a previous base selector when switching representations.

Mutable access to an already loaded sheet previously cloned the stylesheet before discovering that deserialization was unnecessary. The fast path checks the sheet state first. It does not change cell storage, add unsafe code, or alter lazy deserialization of unloaded sheets.

The fidelity tests resolve worksheet cell/row/column references through cellXfs, component IDs, and conditional-format dxfId, then assert the exact raw XML attributes. They run ordinary and expanded XML cases with theme/indexed tint, palette RGB, automatic colors, pattern fills, and gradients. The five base-compatible tests fail on the base commit and pass here; the automatic-setter test exercises a new API.

Validation

  • cargo test
  • cargo clippy -- -D warnings
  • cargo +nightly fmt --all -- --check
  • cargo run --release --example loaded_sheet_access_probe -- 1000 1000

The row-height-policy change and its associated test are intentionally not part of this PR.

@MathNya

MathNya commented Sep 11, 2026

Copy link
Copy Markdown
Owner

@PSU3D0
Thank you for the PR.
I don't see any issues with the changes, so I'll go ahead and merge them.

@MathNya
MathNya merged commit 4195480 into MathNya:master Sep 11, 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