xlsx: don't let x15:workbookPr reset the date1904 flag

[youdie006]

Summary

Fixes #706. A genuine 1904-system xlsx workbook is read on the 1900 epoch when its <extLst> contains an <x15:workbookPr> element (which modern Excel writes), shifting every date 1462 days (4 years + 1 day) earlier.

Root cause

read_workbook matches the workbookPr local name:

Ok(Event::Start(e)) if e.local_name().as_ref() == b"workbookPr" => {
    self.is_1904 = match e.raw_attr(b"date1904")? {
        Some(v) => v == b"1" || v == b"true",
        None => false,
    };
}

local_name() strips the namespace prefix, so the extension element <x15:workbookPr> (inside <extLst>) also matches. It never carries date1904 and appears after the real <workbookPr date1904="1"/>, so the None => false arm resets is_1904 back to false. has_1904_epoch() then returns false and the +1462-day offset is dropped.

Fix

Only assign is_1904 when the date1904 attribute is actually present; the field already defaults to false, so a bare <workbookPr/> still means 1900-system.

Test

Added date_xlsx_1904_extlst with a fixture (tests/date_1904_extlst.xlsx) = a 1904 workbook whose extLst also contains <x15:workbookPr>. Before the fix has_1904_epoch() is false (test fails); after, it is true, and the existing date tests still pass.

---

Disclosure: prepared with AI assistance (Claude); I reviewed it and verified the red-green test and the date test suite.

[jmcnamara]

This comment is referring to the what the patch is fixing and not to what the code is doing. Change it to something like

// Set the 1904 date flag, if present.

[jmcnamara]

This doesn't need to be mut. Also the variable should be called xlsx and not xls.

[jmcnamara]

Thanks for the fix.

I have made some comments inline. Please address them and rebase and force push the changes to your branch. Then mark the comments as "resolved".

Also, reduce the commit message down to 1-2 lines max.

[youdie006]

Pushed a fix for the red CI. The failure was mine: the test I added had a let mut xls that does not need mut (has_1904_epoch() takes &self), which cargo clippy --all-targets --all-features -- -Dwarnings rejects. I had copied the shape from the neighbouring date_xlsx_1904 test, which does need mut because it calls worksheet_range_at.

One-character change, no behaviour difference. Verified locally with the same commands CI runs: cargo clippy --all-targets --all-features -- -Dwarnings clean, cargo test --all-features all green (44 + 162 + 67 passed, 0 failed).

Disclosure: I use AI assistance in my work, and I review and verify everything before it goes out.

[youdie006]

Both review comments addressed — apologies, I acted on the CI failure yesterday and only picked up the mut half of your second note.

src/xlsx/mod.rs: the comment now describes what the code does rather than what the patch fixed:

// Set the 1904 date flag, if present. The attribute is
// optional here because `<x15:workbookPr>` inside `<extLst>`
// shares this local name and never carries `date1904`.

I kept one clause on why the attribute is optional, since without it the if let looks like it could just be an unconditional assignment. Happy to trim it to the single line you suggested if you would rather.

tests/test.rs: renamed xls to xlsx, and the mut is gone.

Verified with the same commands CI runs: cargo clippy --all-targets --all-features -- -Dwarnings clean, cargo test --all-features green (44 + 162 + 67 passed, 0 failed).

Disclosure: I use AI assistance in my work, and I review and verify everything before it goes out.

[jmcnamara]

Merged. Thanks.