From b2208c3da7f2322c58756de825978e7c33b5d414 Mon Sep 17 00:00:00 2001 From: Nitish Agarwal <1592163+nitishagar@users.noreply.github.com> Date: Thu, 6 Aug 2026 17:08:23 +0530 Subject: [PATCH] fix(sheet): skip hidden and very-hidden worksheets 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. 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/anydoc#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 (#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. --- src/formats/sheet/mod.rs | 27 ++++++- tests/fixtures/xlsx/handmade-hidden.xlsx | Bin 0 -> 2309 bytes tests/gen_fixtures.py | 66 +++++++++++++++++ tests/snapshots.rs | 69 ++++++++++++++++++ ...snapshots__xlsx__handmade-hidden.xlsx.snap | 7 ++ 5 files changed, 166 insertions(+), 3 deletions(-) create mode 100644 tests/fixtures/xlsx/handmade-hidden.xlsx create mode 100644 tests/snapshots/snapshots__xlsx__handmade-hidden.xlsx.snap diff --git a/src/formats/sheet/mod.rs b/src/formats/sheet/mod.rs index f61ebcc..a20c73e 100644 --- a/src/formats/sheet/mod.rs +++ b/src/formats/sheet/mod.rs @@ -4,7 +4,7 @@ use crate::error::ConvertError; use crate::model::{Block, Cell, Document, GridBuilder, Inline, TableKind}; use crate::shared::header::resolve_header_rows; use crate::shared::text::clean_text; -use calamine::{Data, Dimensions, Reader, Sheets, open_workbook_auto_from_rs}; +use calamine::{Data, Dimensions, Reader, SheetVisible, Sheets, open_workbook_auto_from_rs}; use std::collections::{HashMap, HashSet}; use std::io::Cursor; @@ -25,12 +25,33 @@ pub fn parse(bytes: &[u8]) -> Result { contained("workbook open", || open_workbook_auto_from_rs(Cursor::new(bytes)))? .map_err(map_open_error)?; let sheet_names = contained("sheet listing", || workbook.sheet_names().to_owned())?; - let multi_sheet = sheet_names.len() > 1; + // `sheets_metadata()` shares one backing `Vec` with `sheet_names()`, + // so the Nth metadata entry describes the Nth name - a name lookup would + // reintroduce the bug under re-ordering, only positional access is sound. + let metadata = contained("sheet metadata", || workbook.sheets_metadata().to_owned())?; + // The multi-sheet heading ("## ") is only useful when more than one + // sheet is actually shown: a single visible sheet inside a workbook full + // of hidden ones must not gain (or lose) its heading. Count visible sheets + // rather than all sheets so hidden ones stay invisible to this decision. + let multi_sheet = sheet_names + .iter() + .zip(metadata.iter()) + .filter(|(_, m)| m.visible == SheetVisible::Visible) + .count() + > 1; let merged = merged_regions(&mut workbook, &sheet_names)?; let mut doc = Document::default(); let mut failed = 0usize; - for name in &sheet_names { + for (name, meta) in sheet_names.iter().zip(metadata.iter()) { + // A hidden or veryHidden sheet is not visible to an end user opening + // the workbook, so it must contribute no block at all - skip it before + // any heading or table is emitted. This runs before the read attempt, + // so a hidden sheet never counts as "unreadable" (hidden != unreadable) + // and an all-hidden workbook degrades to an empty Document, not an error. + if meta.visible != SheetVisible::Visible { + continue; + } let range = match contained("worksheet read", || workbook.worksheet_range(name))? { Ok(r) => r, Err(e) => { diff --git a/tests/fixtures/xlsx/handmade-hidden.xlsx b/tests/fixtures/xlsx/handmade-hidden.xlsx new file mode 100644 index 0000000000000000000000000000000000000000..7d8b75a9f065b231f6760513e6183b9c645a6ad3 GIT binary patch literal 2309 zcmZ{l2{@E_9LL8nqm0}WLu<4KA!kg7$(3=8W36k2u#QS@5}D*0j1kQ^qL8D@Mp-#~ zP&7u4TsbqFX)(ES>{uk%zIdLs&DQ%q@B4q=_kDif=ly*Dzr)g$lS>2y0__8?!%mTs ziEm&WAP|-l1QG_`IG*wMyX@(A*~R8+fM@VIoM{xVq8bm^lDe?1h*M1O*dwDa4un~h-dNOwifB1wnz zq-S;HwpxNR-0RJUlb@gxLwB0|60Su`C0-b0=_)#;qfl5LOXKB z5iP9~vPuMdMGjVEZ-88_(sG*Dn^4HrmyomTqU1##Zc49;eLdLnEAOuRH^h|zo+$wS z!GP5z$kQiSMS1(4+@;0$N|hJ-A=lbq#LEFnuskD5=!0i)68a*FOb*F#4AavD6`+Rg zn>ut)O^J@XlT>3nT?(_ITj7!s9af{6Im=;Bd|PFehUI`ki!8EPB!}ko{nSaYDe(X} zUhF+(F(0a-?lIo|(86e>B@4$0mg`61$7?Z+;4QBP9?uZPhCyGBlCy_4W3301dAjaP zLVefE+%+j4Q}`R;^BpeZkF!yQW^lQM^bsI>0FdzmYnYEpsDF^R zo4>#Jc19$V8~p$}AS1BjYsT_#HEflfx;5q`R=R6Yg@FoM?qjo}lS2wu-gTI0N3PC> zZy>@$^baC`i>B3q;`$6{rn(#GGmBK=BPQ`BQ_{b3UJ$V_J-=i$KBnD#uWbW$pe8*Y z=k4_aRC@7*6f;vfNL3!Wn(yhVVRy)lr(QW}^k?p|{par$j&2#d4Xz$MUKYV`>{X$< zEX({nx-La7Ket78ENCGdx;8b|gTHXj6DN(HPi0)s%}yO!+k_kWGq+537* zI}ccm7rSyD+d9&U(KRUXGYC)|<4(8u7=6swYkFp0N!b8P%P1y01qDehGOte3D45Uz z>22Jz#}EhsH9 z#KXe*|->n4T8uTofNHkGGo!(Hci!)t$Q9VH?#7G zP~1M-7X?y(YP;2C98I)o;770K`LasUv^-x@?E&qL$Uw-~*&rTheq$S54#)BX+fIv+n|cL7Dm4&E2hQz3k|JF)b=gQ3sde$HjKJj z+-o(y8k( z!_%nF#3>Uku1wU41EwU((3|w~a`BvVWw|fw6SnGDvX^QeHxBGuLk^~Zr8m*ylN0Y2 z*miwHnibj!z+=7-e{2`N5f`f_K$zF2JzGIX;sq!utl&00xx`%c9l@h=dA7p^fJbXl zc^T8x3#|)!5%=4mp>#;}&>;Eq#RcYSU~sHzQ((Yzbp)ojA-4YQqhW?2N+^pH!)r*n zQrx#dAxY4Oi5FN7_(1ip>o)g7g8sBpI6Irkbbx1yO$^hn1&o%=EpbJpmQ=a)zvt8A zmEDFv<&YcS?0RtP?4`e_gW~%4C;rz6*j3o;Xiwz_$TwAXs)}8OJt6m0#G&6* Y*-1}JQwZ=C1mXtH9>8%)?(L_40M==ZKL7v# literal 0 HcmV?d00001 diff --git a/tests/gen_fixtures.py b/tests/gen_fixtures.py index d70df84..d4ed952 100644 --- a/tests/gen_fixtures.py +++ b/tests/gen_fixtures.py @@ -914,6 +914,71 @@ def merged_xlsx(): ]) +# --------------------------------------------------------------------------- +# R-hidden-sheet: hidden / veryHidden worksheets must not render as visible +# content (sheet-level visibility, the half of #9 calamine exposes). A visible +# sheet keeps its table; a hidden and a veryHidden sheet drop out entirely. + +def hidden_xlsx(): + ct = """ + + + + + + + +""" + root_rels = """ + + +""" + workbook = """ + + + + + +""" + wb_rels = """ + + + + +""" + sheet1 = """ + + +visible cell +shown + +""" + sheet2 = """ + + +hidden cell +must not appear + +""" + sheet3 = """ + + +very hidden cell +must not appear either + +""" + write_zip(OUT / "xlsx" / "handmade-hidden.xlsx", [ + ("[Content_Types].xml", ct), + ("_rels/.rels", root_rels), + ("xl/workbook.xml", workbook), + ("xl/_rels/workbook.xml.rels", wb_rels), + ("xl/worksheets/sheet1.xml", sheet1), + ("xl/worksheets/sheet2.xml", sheet2), + ("xl/worksheets/sheet3.xml", sheet3), + ]) + + # --------------------------------------------------------------------------- # R16: ODF style:default-style beneath named chains; full ISO durations @@ -1748,6 +1813,7 @@ def main(): manyrefs_docx() defaults_odf() merged_xlsx() + hidden_xlsx() features_epub() bin_rtf() csvs() diff --git a/tests/snapshots.rs b/tests/snapshots.rs index 477a1ad..eddd4a6 100644 --- a/tests/snapshots.rs +++ b/tests/snapshots.rs @@ -11,6 +11,7 @@ mod common; use common::{fixture_root, walk}; use std::fmt::Write as _; +use std::io::Write as _; use std::path::Path; /// Convert one file, capturing panics so a bad parser records a baseline @@ -166,6 +167,74 @@ fn doc_inline_picture_is_retained() { ); } +/// Hidden and veryHidden worksheets contribute no heading and no table; only +/// the visible sheet renders. This is the sheet-level half of #9 (calamine +/// exposes `state="hidden"`/`state="veryHidden"`; row/column visibility is a +/// separate, out-of-scope half). The committed fixture holds one visible sheet +/// plus a hidden and a veryHidden sheet, each with identifying cell content. +#[test] +fn hidden_worksheets_do_not_render() { + let path = fixture_root().join("xlsx").join("handmade-hidden.xlsx"); + let bytes = std::fs::read(&path).unwrap(); + let doc = anydoc::to_document(&bytes, anydoc::Format::Excel).unwrap(); + // Exactly one table - the visible sheet's; no heading is emitted because a + // single visible sheet needs no "## " disambiguator. + assert_eq!(doc.blocks.len(), 1, "only the visible sheet should render, got {:?}", doc.blocks); + assert!(matches!(doc.blocks[0], anydoc::model::Block::Table(_))); + + // The hidden sheets' cell content and names must be absent from the + // rendered Markdown (no heading, no rendered table). + let md = anydoc::to_markdown(&path).unwrap(); + assert!(md.contains("visible cell"), "visible content should render:\n{md}"); + for needle in ["Hidden", "VeryHidden", "must not appear", "very hidden cell"] { + assert!(!md.contains(needle), "hidden content leaked into output ({needle}):\n{md}"); + } +} + +/// A workbook whose every sheet is hidden degrades to an empty Document, not a +/// panic and not a misleading "no sheet could be read" error (hidden is a valid +/// state, not unreadable). Built inline so no committed fixture is all-hidden. +#[test] +fn all_hidden_workbook_yields_empty_document() { + let bytes = all_hidden_xlsx(); + let doc = anydoc::to_document(&bytes, anydoc::Format::Excel).unwrap(); + assert!(doc.blocks.is_empty(), "an all-hidden workbook must render no blocks"); + let md = anydoc::to_markdown_bytes(&bytes, anydoc::Format::Excel).unwrap(); + assert!(md.trim().is_empty(), "an all-hidden workbook must render no markdown"); +} + +/// Minimal xlsx whose single sheet is `state="hidden"`, so every sheet is +/// non-visible. Mirrors the inline xlsx builder the sheet unit tests use. +fn all_hidden_xlsx() -> Vec { + let parts: &[(&str, &str)] = &[ + ( + "[Content_Types].xml", + r#""#, + ), + ( + "_rels/.rels", + r#""#, + ), + ( + "xl/workbook.xml", + r#""#, + ), + ( + "xl/_rels/workbook.xml.rels", + r#""#, + ), + ]; + let sheet = r#"hidden only"#; + let mut w = zip::ZipWriter::new(std::io::Cursor::new(Vec::new())); + for (name, body) in parts { + w.start_file(*name, zip::write::SimpleFileOptions::default()).unwrap(); + w.write_all(body.as_bytes()).unwrap(); + } + w.start_file("xl/worksheets/sheet1.xml", zip::write::SimpleFileOptions::default()).unwrap(); + w.write_all(sheet.as_bytes()).unwrap(); + w.finish().unwrap().into_inner() +} + /// The RTF `\pict` payload is retained as an asset (the Markdown output /// shows only the alt text, which is empty for pictures without one). #[test] diff --git a/tests/snapshots/snapshots__xlsx__handmade-hidden.xlsx.snap b/tests/snapshots/snapshots__xlsx__handmade-hidden.xlsx.snap new file mode 100644 index 0000000..4b8b528 --- /dev/null +++ b/tests/snapshots/snapshots__xlsx__handmade-hidden.xlsx.snap @@ -0,0 +1,7 @@ +--- +source: tests/snapshots.rs +expression: output +--- +| | | +| --- | --- | +| visible cell | shown |