Skip to content

Preserve explicit zero page margins and expose presence - #373

Merged
MathNya merged 1 commit into
MathNya:masterfrom
developer0hye:fix/page-margin-presence
Sep 23, 2026
Merged

MathNya merged 1 commit into
MathNya:masterfrom
developer0hye:fix/page-margin-presence

Conversation

@developer0hye

Copy link
Copy Markdown
Contributor

Explicitly setting all six print margins to zero currently writes Excel defaults instead. This change distinguishes unset margins from supplied values, so zero survives a write/read cycle and defaults apply only to unset fields.

  • Add PageMargins::has_left, has_right, has_top, has_bottom, has_header, and has_footer using the existing internal presence state. Existing numeric accessors retain their behavior.
  • Select writer defaults independently for each unset margin; partially configured objects now default their unset fields rather than writing numeric zero for them.
  • Cover explicit all-zero margins, completely unset margins, mixed zero/nonzero/unset margins, and a worksheet without a pageMargins element. Tests use synthetic workbooks generated in memory.
  • Document the presence API and writer behavior.

The base-compatible zero-margin regression fails on master 35bbffa4 (all six values reopen as defaults) and passes with this change. The default-only control passes on both.

Validation on macOS:

  • cargo test --locked: 344 passed, 23 existing ignored tests.
  • cargo clippy --locked -- -D warnings: passed.
  • cargo clippy --locked --test page_margin_presence -- -D warnings: passed.
  • cargo +nightly-2026-09-11 fmt --all --check: passed.
  • git diff --check: passed.

The local lockfile is untracked, consistent with this repository. No dependency changes or external fixtures.

Related: #372. Downstream: developer0hye/office2pdf#1812.

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

MathNya commented Sep 23, 2026

Copy link
Copy Markdown
Owner

@developer0hye
Thank you for the PR.
Since there were no issues with the changes, I will merge them.

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