fix(sheet): skip hidden and very-hidden worksheets - #39
Open
nitishagar wants to merge 1 commit into
Open
Conversation
calamine 0.36.1 exposes each sheet's visibility via sheets_metadata()
(SheetVisible::{Visible,Hidden,VeryHidden}), index-aligned with
sheet_names() through one backing Vec<Sheet>. The sheet frontend iterated
sheet_names() alone and rendered every sheet as visible content, so a
hidden or veryHidden worksheet was indistinguishable from a visible one
in both to_markdown() and to_document() (firecrawl#9, sheet-level
half).
Filter non-Visible sheets out before any heading or table is emitted:
- the multi-sheet heading decision now counts visible sheets, so a single
visible sheet among hidden ones keeps (or avoids) its heading correctly;
- the skip runs before worksheet_range(), so a hidden sheet never counts
as "unreadable" and an all-hidden workbook degrades to an empty Document
rather than a misleading "no sheet could be read" error.
Row/column visibility (firecrawl#9's other half) needs the in-house parser and is
out of scope; ODS hidden tables route through odf::parse, a separate
frontend, and are tracked as a follow-up.
Adds a handmade-hidden.xlsx fixture (visible + hidden + veryHidden sheet)
and its visible-only snapshot; existing snapshots are byte-for-byte
unchanged.
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.
Closes #9 (sheet-level half).
Summary
anydocrendered entire hidden /veryHiddenworksheets as visible content — indistinguishable from visible sheets in bothto_markdown()andto_document(). This adds a pure filter at the sheet frontend: sheets whosestateishiddenorveryHiddencontribute no heading and no table.calamine 0.36.1 exposes each sheet's visibility via
sheets_metadata()(SheetVisible::{Visible,Hidden,VeryHidden}), which shares one backingVec<Sheet>withsheet_names()— so the Nth metadata entry describes the Nth name. Positionalzip(never name lookup) is used so re-ordering cannot reintroduce the bug.The change is one file,
src/formats/sheet/mod.rs:worksheet_range(), so a hidden sheet never counts as "unreadable";Document(Ok), not a misleading "no sheet could be read" error (hidden is a valid state, not unreadable).Scope
This addresses the sheet-level half of #9. The row/column visibility half genuinely needs reading
xl/worksheets/sheetN.xml<row hidden>/<col hidden>attributes that calamine's xlsx reader does not expose — it depends on the in-house parser and is explicitly out of scope.ODS hidden-sheet filtering is also out of scope here:
.odsroutes throughodf::parse(a different frontend readingcontent.xmldirectly), notsheet::parse, so calamine'sSheetVisibleis irrelevant to that path. Tracked as a follow-up insrc/formats/odf/table.rs.Verification
cargo fmt --all --check✅cargo clippy --workspace --all-targets --all-features -- -D warnings✅cargo test --locked✅ (185 lib + 10 snapshot; existing snapshots byte-for-byte unchanged)hidden_worksheets_do_not_render(visible+hidden+veryHidden fixture → only visible renders),all_hidden_workbook_yields_empty_document(all-hidden → emptyDocument, not Err/panic),veryHiddencovered by the same fixture.main.Manual note (from the plan)
On the issue's openpyxl repro (visible row + hidden row + hidden column + hidden sheet), the hidden sheet disappears while the hidden row/column still render — confirming this does not accidentally touch the out-of-scope row/column half.
Need help on this PR? Tag
@codesmith-botwith what you need. Autofix is disabled.