From a4dad44a5097acb1aaa8446edcd8602c6774ac2e Mon Sep 17 00:00:00 2001 From: Jeremy Karst Date: Fri, 18 Sep 2026 18:09:05 -0400 Subject: [PATCH] Improved PPTX extraction and added comma separators to UI large numbers. --- Cargo.lock | 4 +- Cargo.toml | 2 +- config_example.toml | 2 +- crates/quicksearch-core/src/config/diff.rs | 27 +- crates/quicksearch-core/src/config/mod.rs | 2 +- crates/quicksearch-core/src/config/tests.rs | 44 +++ crates/quicksearch-core/src/extract/mod.rs | 2 +- crates/quicksearch-core/src/extract/office.rs | 295 ++++++++++++++++-- crates/quicksearch-core/src/extract/ole.rs | 15 +- .../quicksearch-core/src/extract/ole_tests.rs | 63 ++++ crates/quicksearch-core/src/mime.rs | 94 +++++- .../tests/fixtures/legacy/README.md | 2 +- crates/quicksearch-core/tests/full_index.rs | 140 +++++++++ crates/quicksearch-gui/src/format.rs | 87 ++++++ crates/quicksearch-gui/src/settings_tab.rs | 42 ++- .../quicksearch-gui/src/settings_tab/tests.rs | 40 +++ crates/quicksearch-gui/src/tips.rs | 6 +- 17 files changed, 820 insertions(+), 47 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index 1e69c8d..98f20f0 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -3108,7 +3108,7 @@ dependencies = [ [[package]] name = "quicksearch-core" -version = "1.1.8" +version = "1.1.9" dependencies = [ "argon2", "cfb", @@ -3150,7 +3150,7 @@ dependencies = [ [[package]] name = "quicksearch-gui" -version = "1.1.8" +version = "1.1.9" dependencies = [ "chrono", "eframe", diff --git a/Cargo.toml b/Cargo.toml index 23cf8c5..d8cb5ac 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -12,7 +12,7 @@ pdf-extract = { path = "vendor/pdf-extract" } # Patched unbounded reads which co rtf-parser = { path = "vendor/rtf-parser" } # Patched a parsing error which occurs on UTF-16 characters [workspace.package] -version = "1.1.8" +version = "1.1.9" edition = "2021" license = "GPL-3.0-or-later" authors = ["Jeremy "] diff --git a/config_example.toml b/config_example.toml index a7e8cd6..17bc6e1 100644 --- a/config_example.toml +++ b/config_example.toml @@ -86,7 +86,7 @@ hash_length = 8192 maximum_text_size = 262144 # Files larger than this skip text extraction entirely (bytes). Clamped to # 1..4294967296 on load. -maximum_text_file_size = 2097152 +maximum_text_file_size = 52428800 # Files per batch during walks / inserts / extraction. batch_size = 500 # Writer time one indexing root's turn may take before the round-robin diff --git a/crates/quicksearch-core/src/config/diff.rs b/crates/quicksearch-core/src/config/diff.rs index e1307e1..3b0f498 100644 --- a/crates/quicksearch-core/src/config/diff.rs +++ b/crates/quicksearch-core/src/config/diff.rs @@ -24,9 +24,12 @@ pub struct IndexWork { /// no walk and no root range would ever reach it again, so every row /// outside the roots goes. pub drop_aliases: bool, - /// `content_extensions` changed. Re-tested both ways: newly-included - /// files go back to pending, newly-excluded ones give up text and FTS - /// but keep the name/path row filename search needs. + /// `content_extensions` or `maximum_text_file_size` changed. Re-tested + /// both ways: newly-included files go back to pending, newly-excluded + /// ones give up text and FTS but keep the name/path row filename search + /// needs. Both settings decide the same thing — whether a file's text is + /// read — and [`crate::scope::advance`] re-runs the whole predicate, + /// size included, so one flag covers them. pub reconcile_content: bool, /// `store_text_for_snippets` turned on; rows extracted under the old /// setting kept no text, so they must run again. @@ -91,7 +94,9 @@ impl IndexWork { parts.push("symlinks no longer followed".into()); } if self.reconcile_content { - parts.push("changed content extensions".into()); + // Either setting that decides it; the flag does not say which, + // and the effect on the index is the same line of work. + parts.push("changed which files have their text read".into()); } if self.restore_text { parts.push("snippet text turned on".into()); @@ -213,7 +218,15 @@ pub fn diff_actions(old: &Config, new: &Config) -> ConfigActions { let old_content = content_filter_set(&old.indexing.content_extensions); let new_content = content_filter_set(&new.indexing.content_extensions); let content_widened = filter_widened(&old_content, &new_content); - work.reconcile_content = old_content != new_content; + + // The size cap decides the same thing the extension filter does — + // whether a file's text is read — so changing it has to be applied + // the same way. Without this a raised cap did nothing to an existing + // index: every file already written off as too large stays `NA`, + // because nothing re-asks until that file itself changes. + let old_cap = old.processing.maximum_text_file_size; + let new_cap = new.processing.maximum_text_file_size; + work.reconcile_content = old_content != new_content || old_cap != new_cap; let old_store = old.processing.store_text_for_snippets; let new_store = new.processing.store_text_for_snippets; @@ -228,6 +241,10 @@ pub fn diff_actions(old: &Config, new: &Config) -> ConfigActions { || (!old.indexing.include_hidden && new.indexing.include_hidden) || (!old.indexing.follow_symlinks && new.indexing.follow_symlinks) || content_widened + // A raised cap widens the same way an added extension does: the + // reconcile puts those rows back to pending, and the content + // pass that reads them runs as part of an indexing run. + || new_cap > old_cap || work.restore_text; } diff --git a/crates/quicksearch-core/src/config/mod.rs b/crates/quicksearch-core/src/config/mod.rs index 56835ed..5e3f57d 100644 --- a/crates/quicksearch-core/src/config/mod.rs +++ b/crates/quicksearch-core/src/config/mod.rs @@ -236,7 +236,7 @@ impl Default for ProcessingConfig { ProcessingConfig { hash_length: 1024 * 8, maximum_text_size: 1024 * 256, - maximum_text_file_size: 1024 * 1024 * 2, + maximum_text_file_size: 1024 * 1024 * 50, batch_size: 500, writer_turn_slice_ms: 100, maximum_wal_size: 1024 * 1024 * 1024 * 2, diff --git a/crates/quicksearch-core/src/config/tests.rs b/crates/quicksearch-core/src/config/tests.rs index 00e0a56..48ad021 100644 --- a/crates/quicksearch-core/src/config/tests.rs +++ b/crates/quicksearch-core/src/config/tests.rs @@ -220,6 +220,50 @@ fn comment_only_edit_is_no_work_at_all() { assert!(a.work.reconcile_content && !a.work.reindex); } +/// The size cap decides what gets read exactly as the extension filter +/// does, so it has to be applied the same way. It was not: a raised cap left +/// every file already written off as too large sitting at `NA`, and nothing +/// re-asks about a file until the file itself changes — so the setting +/// appeared to do nothing at all on an existing index. +#[test] +fn changing_the_text_file_size_cap_re_decides_the_stored_rows() { + let old = Config::default(); + + let mut raised = old.clone(); + raised.processing.maximum_text_file_size = old.processing.maximum_text_file_size * 2; + let a = diff_actions(&old, &raised); + assert!( + !a.requires_rebuild, + "no rebuild: the rows are all still right" + ); + assert!( + a.work.reconcile_content, + "rows written off as too large must be re-decided" + ); + assert!( + a.work.reindex, + "widening needs a run: the content pass is what reads the newly eligible files" + ); + + let mut lowered = old.clone(); + lowered.processing.maximum_text_file_size = old.processing.maximum_text_file_size / 2; + let a = diff_actions(&old, &lowered); + assert!( + a.work.reconcile_content, + "rows now over the cap must give up their text" + ); + assert!( + !a.work.reindex, + "narrowing finds nothing new on disk, so there is nothing to walk for" + ); + + assert_eq!( + diff_actions(&old, &old.clone()), + ConfigActions::default(), + "an unchanged cap is not work" + ); +} + /// An empty list means "everything allowed" — a superset of every other /// list, the case plain set arithmetic reads backwards. #[test] diff --git a/crates/quicksearch-core/src/extract/mod.rs b/crates/quicksearch-core/src/extract/mod.rs index 7142a51..0385c13 100644 --- a/crates/quicksearch-core/src/extract/mod.rs +++ b/crates/quicksearch-core/src/extract/mod.rs @@ -23,7 +23,7 @@ pub type ExtractError = String; /// /// Every extractor used to carry its own 64 MiB constants, chosen /// independently of the settings that actually bound the work: the content -/// pass never offers a file above `maximum_text_file_size` (2 MiB by +/// pass never offers a file above `maximum_text_file_size` (50 MiB by /// default), and everything an extractor produces past `maximum_text_size` /// (256 KiB) is discarded by the caller moments later. A ceiling 256× above /// the largest result that can be kept is not a safety margin, it is the diff --git a/crates/quicksearch-core/src/extract/office.rs b/crates/quicksearch-core/src/extract/office.rs index 8e1c323..62d981c 100644 --- a/crates/quicksearch-core/src/extract/office.rs +++ b/crates/quicksearch-core/src/extract/office.rs @@ -2,8 +2,19 @@ //! containers holding XML. Five share one event loop ([`collect_xml_text`], //! driven by a per-format [`TextSpec`]); XLSX keeps its own two loops because //! its text is not in the sheet at all but in a shared-string table the cells -//! index into. Dispatch is by file extension, not MIME: `.docm` carries the -//! same MIME as `.docx`, and the extension is what distinguishes them. +//! index into. +//! +//! Each of those formats has template and macro-enabled siblings holding the +//! same parts under the same names — `.pptx`, `.ppsx`, `.pptm`, `.potx` are +//! one parser — so every extension an office suite writes is routed here +//! rather than only the six default ones. +//! +//! Dispatch is by file extension, not MIME, because the pre-2007 formats +//! give a document and its template one MIME between them: `.doc` and `.dot` +//! are both `application/msword`, and only the name says which parser to +//! run. [`is_office_mime`] decides *whether* to run, the extension decides +//! *what*, and the two tables have to stay in step — the test at the bottom +//! of this file is what holds them there. use std::error::Error; use std::fs::File; @@ -18,19 +29,43 @@ use super::{ExtractError, Extractor, Scratch}; pub struct OfficeExtractor; -fn mime_to_ext(mime: &str) -> Option<&'static str> { - match mime { - "application/msword" => Some("doc"), - "application/vnd.openxmlformats-officedocument.wordprocessingml.document" => Some("docx"), - "application/vnd.ms-excel" => Some("xls"), - "application/vnd.openxmlformats-officedocument.spreadsheetml.sheet" => Some("xlsx"), - "application/vnd.ms-powerpoint" => Some("ppt"), - "application/vnd.openxmlformats-officedocument.presentationml.presentation" => Some("pptx"), - "application/vnd.oasis.opendocument.text" => Some("odt"), - "application/vnd.oasis.opendocument.spreadsheet" => Some("ods"), - "application/vnd.oasis.opendocument.presentation" => Some("odp"), - _ => None, - } +/// Every MIME this extractor claims, spelled lowercase because +/// [`super::Registry::find`] folds case before dispatch — the registered +/// spelling of the macro-enabled types is `macroEnabled`. +/// +/// Membership only: it answers "is this an office document", never which +/// parser runs. See the module docs for why that is the extension's job. +fn is_office_mime(mime: &str) -> bool { + matches!( + mime, + // Word: .doc/.dot, .docx/.dotx, .docm/.dotm + "application/msword" + | "application/vnd.openxmlformats-officedocument.wordprocessingml.document" + | "application/vnd.openxmlformats-officedocument.wordprocessingml.template" + | "application/vnd.ms-word.document.macroenabled.12" + | "application/vnd.ms-word.template.macroenabled.12" + // Excel: .xls/.xlt, .xlsx/.xltx, .xlsm/.xltm + | "application/vnd.ms-excel" + | "application/vnd.openxmlformats-officedocument.spreadsheetml.sheet" + | "application/vnd.openxmlformats-officedocument.spreadsheetml.template" + | "application/vnd.ms-excel.sheet.macroenabled.12" + | "application/vnd.ms-excel.template.macroenabled.12" + // PowerPoint: .ppt/.pps/.pot, .pptx/.ppsx/.potx, .pptm/.ppsm/.potm + | "application/vnd.ms-powerpoint" + | "application/vnd.openxmlformats-officedocument.presentationml.presentation" + | "application/vnd.openxmlformats-officedocument.presentationml.slideshow" + | "application/vnd.openxmlformats-officedocument.presentationml.template" + | "application/vnd.ms-powerpoint.presentation.macroenabled.12" + | "application/vnd.ms-powerpoint.slideshow.macroenabled.12" + | "application/vnd.ms-powerpoint.template.macroenabled.12" + // OpenDocument: .odt/.ott, .ods/.ots, .odp/.otp + | "application/vnd.oasis.opendocument.text" + | "application/vnd.oasis.opendocument.text-template" + | "application/vnd.oasis.opendocument.spreadsheet" + | "application/vnd.oasis.opendocument.spreadsheet-template" + | "application/vnd.oasis.opendocument.presentation" + | "application/vnd.oasis.opendocument.presentation-template" + ) } // The shared XML text walk @@ -286,6 +321,19 @@ fn collect_members( Ok(()) } +/// A deck's slides, then its speaker notes. +/// +/// Notes live in parts of their own (`ppt/notesSlides/`) that the slide text +/// never mentions, so a deck whose argument is in the notes was previously +/// indexed with none of it. +/// +/// They are appended in a block rather than placed under the slide each +/// belongs to. A note is bound to its slide by that slide's `.rels` and +/// **not** by its number — PowerPoint numbers notes parts in creation order, +/// so a deck whose only note is on slide 5 stores it as `notesSlide1.xml`. +/// Reading a relationship per slide to get the pairing right buys a search +/// nothing, and guessing it from the number would file notes under the wrong +/// slide in a snippet, which is worse than not claiming to know. fn extract_pptx(path: &Path, out: &mut String, scratch: &mut Scratch) -> Result<(), Box> { let limits = scratch.limits(); let mut archive = open_container(path)?; @@ -301,6 +349,19 @@ fn extract_pptx(path: &Path, out: &mut String, scratch: &mut Scratch) -> Result< out.push_str("\n--- New Slide ---\n"); Ok(()) }, + )?; + // The marker leads here, where the slides' trails: it introduces the note + // that follows it, rather than separating one from the next. + collect_members( + &mut archive, + "ppt/notesSlides/notesSlide", + out, + limits.text, + |archive, name, out| { + member_bytes(archive, name, limits.inflate, bytes)?; + out.push_str("\n--- Speaker Notes ---\n"); + collect_xml_text(xml_over(bytes), &PPTX, out, limits.text, events) + }, ) } @@ -475,6 +536,13 @@ fn extract_xlsx(path: &Path, out: &mut String, scratch: &mut Scratch) -> Result< /// Extract text from an office document, chosen by lowercase extension. An /// unhandled extension yields empty text: the MIME was claimed, so the file /// was simply named unlike its type. +/// +/// A template (`.dotx`, `.xltx`, `.potx`) and a macro-enabled document +/// (`.docm`, `.xlsm`, `.pptm`) hold their text in the same parts under the +/// same names as the plain form, so each is an alias rather than a format. +/// Leaving them off this list did not make them fail loudly — a claimed MIME +/// falling through to `_` is a file recorded as *successfully* holding no +/// text, which nothing later retries. fn extract_document_text( path: &Path, extension: &str, @@ -482,20 +550,26 @@ fn extract_document_text( scratch: &mut Scratch, ) -> Result<(), Box> { match extension { - "docx" => single_member(path, "word/document.xml", &DOCX, out, scratch), - "xlsx" => extract_xlsx(path, out, scratch), - "pptx" => extract_pptx(path, out, scratch), - "odt" | "odp" => single_member(path, "content.xml", &ODF_TEXT, out, scratch), - "ods" => single_member(path, "content.xml", &ODF_SHEET, out, scratch), + "docx" | "docm" | "dotx" | "dotm" => { + single_member(path, "word/document.xml", &DOCX, out, scratch) + } + "xlsx" | "xlsm" | "xltx" | "xltm" => extract_xlsx(path, out, scratch), + "pptx" | "pptm" | "ppsx" | "ppsm" | "potx" | "potm" => extract_pptx(path, out, scratch), + "odt" | "ott" | "odp" | "otp" => { + single_member(path, "content.xml", &ODF_TEXT, out, scratch) + } + "ods" | "ots" => single_member(path, "content.xml", &ODF_SHEET, out, scratch), // Pre-2007 binary formats: a different container entirely. - "doc" | "xls" | "ppt" => super::ole::extract_ole_text(path, extension, out, scratch), + "doc" | "dot" | "xls" | "xlt" | "ppt" | "pps" | "pot" => { + super::ole::extract_ole_text(path, extension, out, scratch) + } _ => Ok(()), } } impl Extractor for OfficeExtractor { fn supports(&self, mime: &str) -> bool { - mime_to_ext(mime).is_some() + is_office_mime(mime) } fn extract( @@ -732,6 +806,14 @@ mod tests { Body\ "; + /// A notes part: the note itself, then the slide-number field every + /// notes slide carries. The field's text is indexed like any other — + /// suppressing it would mean suppressing dates and footers too. + const PPTX_NOTES: &str = "\ + Mention the budget\ + 3\ + "; + const ODT_BODY: &str = "\ Heading\ Paraspan\ @@ -785,6 +867,127 @@ mod tests { ); } + /// Speaker notes are in parts of their own that the slide text never + /// mentions, so a deck arguing its case in the notes used to be indexed + /// with none of it. + #[test] + fn pptx_extracts_speaker_notes() { + let p = container( + "pptx-notes", + "pptx", + &[ + ("ppt/slides/slide1.xml", PPTX_SLIDE), + ("ppt/notesSlides/notesSlide1.xml", PPTX_NOTES), + ], + ); + assert_eq!( + extract_document_text(&p, "pptx").unwrap(), + "Title\nBody\n\n--- New Slide ---\n\n--- Speaker Notes ---\nMention the budget\n3\n" + ); + } + + /// The notes prefix must select notes *slides* and nothing that sits + /// beside them: the master is boilerplate on every deck, and a `.rels` + /// is a manifest whose text is filenames. + #[test] + fn the_notes_master_and_rels_are_not_notes() { + let p = container( + "pptx-notes-neighbours", + "pptx", + &[ + ("ppt/slides/slide1.xml", PPTX_SLIDE), + ( + "ppt/notesMasters/notesMaster1.xml", + "MASTERTEXT", + ), + ( + "ppt/notesSlides/_rels/notesSlide1.xml.rels", + "", + ), + ("ppt/notesSlides/notesSlide1.xml", PPTX_NOTES), + ], + ); + let out = extract_document_text(&p, "pptx").unwrap(); + assert!(out.contains("Mention the budget"), "{:?}", out); + assert!( + !out.contains("MASTERTEXT"), + "the notes master leaked: {:?}", + out + ); + assert!( + !out.contains("RELSTEXT"), + "a rels manifest leaked: {:?}", + out + ); + } + + /// A deck with no notes at all must read exactly as it did before notes + /// were collected — no stray marker, no trailing blank run. + #[test] + fn a_deck_without_notes_is_unchanged() { + let p = container( + "pptx-no-notes", + "pptx", + &[("ppt/slides/slide1.xml", PPTX_SLIDE)], + ); + assert_eq!( + extract_document_text(&p, "pptx").unwrap(), + "Title\nBody\n\n--- New Slide ---\n" + ); + } + + /// Templates and macro-enabled documents hold the same parts under the + /// same names, so each name is an alias for one parser. Leaving one off + /// the dispatch did not fail loudly: the MIME was still claimed, so the + /// file extracted to nothing and was recorded as *successfully* holding + /// no text, which nothing later retries. + #[test] + fn template_and_macro_enabled_names_reach_the_same_parser() { + /// One parser's aliases: the names, the members of a container to + /// try them against, and the text every one of them must produce. + type AliasCase = ( + &'static [&'static str], + &'static [(&'static str, &'static str)], + &'static str, + ); + let cases: &[AliasCase] = &[ + ( + &["docx", "docm", "dotx", "dotm"], + &[("word/document.xml", DOCX_BODY)], + "Helloworld\nSecond\n", + ), + ( + &["pptx", "pptm", "ppsx", "ppsm", "potx", "potm"], + &[("ppt/slides/slide1.xml", PPTX_SLIDE)], + "Title\nBody\n\n--- New Slide ---\n", + ), + ( + &["xlsx", "xlsm", "xltx", "xltm"], + &[ + ("xl/sharedStrings.xml", XLSX_SHARED), + ("xl/worksheets/sheet1.xml", XLSX_SHEET), + ], + "Shared 42 \nSecond \n", + ), + ( + &["odt", "ott", "odp", "otp"], + &[("content.xml", ODT_BODY)], + "Heading\nParaspan\n", + ), + (&["ods", "ots"], &[("content.xml", ODS_BODY)], "A1 \nB1 \n"), + ]; + for (extensions, members, expected) in cases { + for ext in *extensions { + let p = container(&format!("alias-{ext}"), ext, members); + assert_eq!( + &extract_document_text(&p, ext).unwrap(), + expected, + ".{ext} did not reach the same parser as its siblings" + ); + } + } + } + #[test] fn odt_and_odp_are_the_same_extraction() { let odt = container("odt", "odt", &[("content.xml", ODT_BODY)]); @@ -980,6 +1183,54 @@ mod tests { ); } + /// Every name this extractor handles, walked end to end: the sniffer + /// names the file, [`is_office_mime`] decides whether to run, and the + /// dispatch in [`extract_document_text`] decides what runs. All three + /// have to agree for one document to be indexed, and disagreeing costs + /// nothing visible at either end — an unclaimed MIME is never offered to + /// an extractor at all, and a claimed MIME whose extension the dispatch + /// does not know yields empty text on a file recorded as done. + const CLAIMED: &[&str] = &[ + "docx", "docm", "dotx", "dotm", "xlsx", "xlsm", "xltx", "xltm", "pptx", "pptm", "ppsx", + "ppsm", "potx", "potm", "odt", "ott", "ods", "ots", "odp", "otp", "doc", "dot", "xls", + "xlt", "ppt", "pps", + ]; + + #[test] + fn every_claimed_name_is_sniffed_claimed_and_parsed() { + // Nothing opens it: a dispatched extension fails on a missing file, + // while the fall-through arm reports success with no text. That is + // what separates "no parser" from "parser found nothing". + let missing = Path::new("/nonexistent/quicksearch-office-probe"); + for ext in CLAIMED { + let named = std::path::PathBuf::from(format!("probe.{ext}")); + let guessed = crate::mime::guess_mime_from_head(&named, b"") + .unwrap_or_else(|| panic!(".{ext} is sniffed as nothing at all")); + // Lowercased the way the registry lowercases before dispatch: + // the registered spelling is `macroEnabled`. + assert!( + OfficeExtractor.supports(&guessed.to_ascii_lowercase()), + ".{ext} is sniffed as {guessed}, which no extractor claims" + ); + assert!( + extract_document_text(missing, ext).is_err(), + ".{ext} is claimed but falls through to the no-op arm, so a real \ + one would be stored as holding no text" + ); + } + } + + /// `.pot` is the one name shared with an unrelated text format (gettext + /// templates), so it is sniffed from content rather than the name — the + /// PowerPoint one has to survive that. + #[test] + fn a_gettext_template_is_not_a_powerpoint_template() { + let text = b"msgid \"\"\nmsgstr \"\"\n\"Project-Id-Version: x\\n\"\n"; + let mime = crate::mime::guess_mime_from_head(Path::new("messages.pot"), text); + assert_eq!(mime, Some("text/plain")); + assert!(!OfficeExtractor.supports("text/plain")); + } + /// A self-closed `` has no `End` to hang the break on. #[test] fn a_self_closed_paragraph_still_breaks_the_line() { diff --git a/crates/quicksearch-core/src/extract/ole.rs b/crates/quicksearch-core/src/extract/ole.rs index 47ba451..9f6dca8 100644 --- a/crates/quicksearch-core/src/extract/ole.rs +++ b/crates/quicksearch-core/src/extract/ole.rs @@ -1,5 +1,6 @@ -//! Legacy binary Office formats: `.doc`, `.xls`, `.ppt` — OLE2 compound -//! files; three parsers sharing a reader, wanting only the *text*. Every +//! Legacy binary Office formats: `.doc`, `.xls`, `.ppt` and the template and +//! slideshow names for the same three containers — OLE2 compound files; three +//! parsers sharing a reader, wanting only the *text*. Every //! offset and count in these formats is attacker-controlled, so every read is //! bounds-checked, declared lengths are clamped, nothing is preallocated from //! a declared count, and a malformed file yields `Err` — never a partial @@ -26,10 +27,14 @@ pub fn extract_ole_text( let budget = scratch.limits().text; let mut cfb = cfb::CompoundFile::open(File::open(path)?) .map_err(|e| format!("not a readable OLE2 compound file: {}", e))?; + // A template holds the same streams as the document written from it, and + // a `.pps` is a `.ppt` that opens in the slideshow: one parser each, three + // names apiece. They share a MIME with the plain form, so leaving them out + // meant a claimed file extracting to nothing at all. let text = match extension { - "doc" => doc::extract(&mut cfb, budget), - "xls" => xls::extract(&mut cfb, budget), - "ppt" => ppt::extract(&mut cfb, budget), + "doc" | "dot" => doc::extract(&mut cfb, budget), + "xls" | "xlt" => xls::extract(&mut cfb, budget), + "ppt" | "pps" | "pot" => ppt::extract(&mut cfb, budget), other => Err(format!("no OLE2 parser for .{}", other).into()), }?; if out.is_empty() { diff --git a/crates/quicksearch-core/src/extract/ole_tests.rs b/crates/quicksearch-core/src/extract/ole_tests.rs index ff07439..f42ab0f 100644 --- a/crates/quicksearch-core/src/extract/ole_tests.rs +++ b/crates/quicksearch-core/src/extract/ole_tests.rs @@ -361,6 +361,69 @@ fn ppt_reads_both_atom_widths() { assert!(text.contains("Ωmega body"), "{text}"); } +/// A template and a slideshow are the same container under another name, and +/// they share the plain form's MIME — so a name the dispatch did not know was +/// claimed, opened, and extracted to nothing rather than failing. +#[test] +fn legacy_alias_names_reach_the_same_parser() { + let bytes = encoding_rs::WINDOWS_1252 + .encode("Slide title") + .0 + .into_owned(); + let deck = container( + "ole-alias-ppt", + "ppt", + &[( + "PowerPoint Document", + ppt_record(0x0000, ppt::TEXT_BYTES_ATOM, &bytes), + )], + ); + for ext in ["ppt", "pps", "pot"] { + assert!( + extract_ole_text(&deck, ext) + .unwrap() + .contains("Slide title"), + ".{ext} did not reach the PowerPoint parser" + ); + } + + // The trailing `\r` is Word's paragraph mark, which extracts as `\n`. + let prose = container( + "ole-alias-doc", + "doc", + &word_doc(&[("Hello from Word\r", true)]), + ); + for ext in ["doc", "dot"] { + assert_eq!( + extract_ole_text(&prose, ext).unwrap(), + "Hello from Word\n", + ".{ext} did not reach the Word parser" + ); + } + + let mut sst = Vec::new(); + sst.extend_from_slice(&le32(1)); + sst.extend_from_slice(&le32(1)); + sst.extend_from_slice(&sst_string("Revenue", false)); + let mut book = biff(xls::REC_SST, &sst); + book.extend_from_slice(&biff(xls::REC_LABELSST, &labelsst(0))); + let sheet = container("ole-alias-xls", "xls", &[("Workbook", book)]); + for ext in ["xls", "xlt"] { + assert!( + extract_ole_text(&sheet, ext).unwrap().contains("Revenue"), + ".{ext} did not reach the Excel parser" + ); + } +} + +/// An extension no parser handles must say so, not come back empty. +#[test] +fn an_unknown_extension_is_an_error() { + let deck = container("ole-unknown", "ppt", &[("PowerPoint Document", vec![])]); + let err = extract_ole_text(&deck, "zzz").unwrap_err().to_string(); + assert!(err.contains("no OLE2 parser"), "{err}"); +} + #[test] fn ppt_descends_into_containers() { let bytes = encoding_rs::WINDOWS_1252 diff --git a/crates/quicksearch-core/src/mime.rs b/crates/quicksearch-core/src/mime.rs index fccbe4a..f5c5530 100644 --- a/crates/quicksearch-core/src/mime.rs +++ b/crates/quicksearch-core/src/mime.rs @@ -218,9 +218,16 @@ pub fn mime_to_type(mime: &str) -> FileType { // Subtype-based classification for the `application/*` grab bag. match sub { // Office formats + // Each of the three families covers its template and macro-enabled + // names too: they are the same format, and a `.pptm` the Presentation + // filter hid would be a deck the user cannot find by type. "msword" | "vnd.openxmlformats-officedocument.wordprocessingml.document" + | "vnd.openxmlformats-officedocument.wordprocessingml.template" + | "vnd.ms-word.document.macroenabled.12" + | "vnd.ms-word.template.macroenabled.12" | "vnd.oasis.opendocument.text" + | "vnd.oasis.opendocument.text-template" | "rtf" | "pdf" | "epub+zip" @@ -229,12 +236,22 @@ pub fn mime_to_type(mime: &str) -> FileType { } "vnd.ms-excel" | "vnd.openxmlformats-officedocument.spreadsheetml.sheet" - | "vnd.oasis.opendocument.spreadsheet" => { + | "vnd.openxmlformats-officedocument.spreadsheetml.template" + | "vnd.ms-excel.sheet.macroenabled.12" + | "vnd.ms-excel.template.macroenabled.12" + | "vnd.oasis.opendocument.spreadsheet" + | "vnd.oasis.opendocument.spreadsheet-template" => { t |= FileType::DOCUMENT | FileType::SPREADSHEET; } "vnd.ms-powerpoint" | "vnd.openxmlformats-officedocument.presentationml.presentation" - | "vnd.oasis.opendocument.presentation" => { + | "vnd.openxmlformats-officedocument.presentationml.slideshow" + | "vnd.openxmlformats-officedocument.presentationml.template" + | "vnd.ms-powerpoint.presentation.macroenabled.12" + | "vnd.ms-powerpoint.slideshow.macroenabled.12" + | "vnd.ms-powerpoint.template.macroenabled.12" + | "vnd.oasis.opendocument.presentation" + | "vnd.oasis.opendocument.presentation-template" => { t |= FileType::DOCUMENT | FileType::PRESENTATION; } // Outlook saved messages and compiled HTML help are documents. @@ -312,6 +329,79 @@ mod tests { assert!(t.contains(FileType::PRESENTATION)); } + /// Every name an office suite writes classifies as the format it is, or + /// the type filter hides a deck the extractor happily read. Case is the + /// registered spelling — `macroEnabled` — because that is what reaches + /// the classifier from [`guess_mime_from_head`]. + #[test] + fn template_and_macro_enabled_names_classify_as_their_format() { + let cases: &[(&str, FileType)] = &[ + ( + "application/vnd.ms-powerpoint.presentation.macroEnabled.12", + FileType::PRESENTATION, + ), + ( + "application/vnd.openxmlformats-officedocument.presentationml.slideshow", + FileType::PRESENTATION, + ), + ( + "application/vnd.openxmlformats-officedocument.presentationml.template", + FileType::PRESENTATION, + ), + ( + "application/vnd.ms-powerpoint.slideshow.macroEnabled.12", + FileType::PRESENTATION, + ), + ( + "application/vnd.ms-powerpoint.template.macroEnabled.12", + FileType::PRESENTATION, + ), + ( + "application/vnd.oasis.opendocument.presentation-template", + FileType::PRESENTATION, + ), + ( + "application/vnd.ms-excel.sheet.macroEnabled.12", + FileType::SPREADSHEET, + ), + ( + "application/vnd.openxmlformats-officedocument.spreadsheetml.template", + FileType::SPREADSHEET, + ), + ( + "application/vnd.ms-excel.template.macroEnabled.12", + FileType::SPREADSHEET, + ), + ( + "application/vnd.oasis.opendocument.spreadsheet-template", + FileType::SPREADSHEET, + ), + ( + "application/vnd.ms-word.document.macroEnabled.12", + FileType::DOCUMENT, + ), + ( + "application/vnd.openxmlformats-officedocument.wordprocessingml.template", + FileType::DOCUMENT, + ), + ( + "application/vnd.ms-word.template.macroEnabled.12", + FileType::DOCUMENT, + ), + ( + "application/vnd.oasis.opendocument.text-template", + FileType::DOCUMENT, + ), + ]; + for (mime, expected) in cases { + let t = mime_to_type(mime); + assert!(t.contains(*expected), "{} is not classified", mime); + // Presentations and spreadsheets are documents as well, the way + // Baloo counts them. + assert!(t.contains(FileType::DOCUMENT), "{} is not a document", mime); + } + } + #[test] fn html_is_text_and_document() { let t = mime_to_type("text/html"); diff --git a/crates/quicksearch-core/tests/fixtures/legacy/README.md b/crates/quicksearch-core/tests/fixtures/legacy/README.md index 080c0dc..1e04bfd 100644 --- a/crates/quicksearch-core/tests/fixtures/legacy/README.md +++ b/crates/quicksearch-core/tests/fixtures/legacy/README.md @@ -55,7 +55,7 @@ files are still right. `sample.ppt` is ~460 KB because the PowerPoint 97 export filter embeds the master slide. There is no filter option that trims it, and it is well inside -the 2 MiB `maximum_text_file_size` the end-to-end run indexes with. +the 50 MiB `maximum_text_file_size` the end-to-end run indexes with. Unlike the generated half of the corpus, the text in these three is fixed: `QUICKSEARCH_CORPUS_SEED` does not affect them. diff --git a/crates/quicksearch-core/tests/full_index.rs b/crates/quicksearch-core/tests/full_index.rs index 656964c..0b95fbf 100644 --- a/crates/quicksearch-core/tests/full_index.rs +++ b/crates/quicksearch-core/tests/full_index.rs @@ -1914,3 +1914,143 @@ fn repeated_warm_reindexes_do_not_grow_the_fts_index() { sizes ); } + +// --------------------------------------------------------------------------- +// Container size against extraction cost +// --------------------------------------------------------------------------- + +/// A presentation carrying media: one slide of text inside megabytes of +/// picture, which is the shape every real deck has. +fn media_heavy_deck(path: &Path, media_bytes: usize) { + let file = std::fs::File::create(path).unwrap(); + let mut zip = zip::ZipWriter::new(file); + zip.start_file("ppt/slides/slide1.xml", zip::write::FileOptions::default()) + .unwrap(); + std::io::Write::write_all( + &mut zip, + b"\ + Quarterly chalcedony forecast\ + ", + ) + .unwrap(); + // Stored, not deflated: the file on disk has to really be this large, or + // the size gate under test never sees it. + zip.start_file( + "ppt/media/image1.png", + zip::write::FileOptions::default().compression_method(zip::CompressionMethod::Stored), + ) + .unwrap(); + std::io::Write::write_all(&mut zip, &vec![0u8; media_bytes]).unwrap(); + zip.finish().unwrap(); +} + +/// The content pass gates on a file's *size*, which for a container is its +/// media and not its text: a 5 MiB deck holds about 10 KiB of words, and +/// reading them costs a seek through the central directory plus the one +/// member named. A cap tuned to prose skipped every deck in the tree — +/// indexed by name, with nothing inside findable and no failure to show for +/// it. This pins the default cap against the shape that exposed it. +#[test] +fn a_media_heavy_presentation_is_indexed_under_the_default_cap() { + let root = Scratch::dir("deck-media"); + let db_dir = Scratch::dir("deck-media-db"); + let db = db_dir.join("index.sqlite"); + let deck = root.join("quarterly.pptx"); + media_heavy_deck(&deck, 5 * 1024 * 1024); + + let config = Config::default(); + let size = std::fs::metadata(&deck).unwrap().len(); + assert!( + size > 2 * 1024 * 1024, + "the fixture is only a test of the cap if it is large: {} bytes", + size + ); + assert!( + size <= config.processing.maximum_text_file_size, + "a {} byte deck is over the {} byte default cap, so its text is skipped", + size, + config.processing.maximum_text_file_size + ); + + index_once(&root, &db, &config); + + let text = stored_text(&db, "quarterly.pptx").expect("the deck stored no text at all"); + assert!( + text.contains("Quarterly chalcedony forecast"), + "the slide's text is not what was stored: {:?}", + text + ); + + let conn = rusqlite::Connection::open(&db).unwrap(); + let hits: i64 = conn + .query_row( + "SELECT COUNT(*) FROM searchabletext WHERE searchabletext MATCH ?1", + ["\"chalcedony\""], + |r| r.get(0), + ) + .unwrap(); + assert_eq!(hits, 1, "a word inside the deck is not searchable"); +} + +/// Raising the cap has to reach an index that already wrote a file off. The +/// row is `NA` and nothing re-asks about a file until the file itself +/// changes, so without a reconcile the setting looks like it does nothing: +/// the deck stays findable by name only, with no failure to explain it. +#[test] +fn raising_the_size_cap_reaches_files_already_written_off() { + use std::sync::atomic::AtomicBool; + + let root = Scratch::dir("cap-raise"); + let db_dir = Scratch::dir("cap-raise-db"); + let db = db_dir.join("index.sqlite"); + media_heavy_deck(&root.join("quarterly.pptx"), 5 * 1024 * 1024); + + // A cap under the deck's size: its text is never read. The root goes in + // the config too — `WorkCursor` scans the roots the config names, not + // the ones an indexing run was handed. + let mut tight = Config::default(); + tight.paths.indexing_paths = vec![root.to_string_lossy().into_owned()]; + tight.processing.maximum_text_file_size = 1024 * 1024; + index_once(&root, &db, &tight); + assert!( + stored_text(&db, "quarterly.pptx").is_none(), + "the fixture has to start out skipped, or this proves nothing" + ); + + // Raised, the way Settings raises it: one field, nothing else. + let mut raised = tight.clone(); + raised.processing.maximum_text_file_size = Config::default().processing.maximum_text_file_size; + let work = quicksearch_core::config::diff_actions(&tight, &raised).work; + assert!( + work.reconcile_content && work.reindex, + "a raised cap must ask for both halves: {:?}", + work + ); + + let mut conn = rusqlite::Connection::open(&db).unwrap(); + let registry = quicksearch_core::extract::Registry::default_set(); + let mut cursor = quicksearch_core::scope::WorkCursor::new(work, &raised).unwrap(); + let cancel = AtomicBool::new(false); + while !cursor.done() { + quicksearch_core::scope::advance( + &mut conn, + &raised, + ®istry, + &mut cursor, + Instant::now(), + &cancel, + ) + .unwrap(); + } + drop(conn); + + index_once(&root, &db, &raised); + + let text = stored_text(&db, "quarterly.pptx") + .expect("the deck is still skipped after the cap was raised"); + assert!( + text.contains("Quarterly chalcedony forecast"), + "the deck's text is not what was stored: {:?}", + text + ); +} diff --git a/crates/quicksearch-gui/src/format.rs b/crates/quicksearch-gui/src/format.rs index 48ad09f..c115053 100644 --- a/crates/quicksearch-gui/src/format.rs +++ b/crates/quicksearch-gui/src/format.rs @@ -65,6 +65,9 @@ pub fn fmt_interval(minutes: u64) -> String { } /// `1,234,567`. +/// +/// Read as well as written: the byte fields in Settings are *edited* in this +/// form, so [`parse_grouped`] has to take back whatever this puts out. pub fn group_thousands(n: u64) -> String { let digits = n.to_string(); let mut out = String::with_capacity(digits.len() + digits.len() / 3); @@ -77,6 +80,30 @@ pub fn group_thousands(n: u64) -> String { out } +/// [`group_thousands`] read back, whether or not the separators were typed: +/// `52,428,800`, `52428800` and `52 428 800` are one number. +/// +/// `None` for anything that is not a whole number — including the empty +/// string a half-cleared field holds. A `DragValue` whose parser declines +/// keeps the value it had, so a number being retyped cannot land in the +/// config as a zero on its way through. +pub fn parse_grouped(text: &str) -> Option { + let mut digits = String::with_capacity(text.len()); + for c in text.chars() { + // Every separator someone might type or paste in, including the + // narrow no-break space a spreadsheet copies out. + if c == ',' || c == '_' || c == '\u{202f}' || c.is_whitespace() { + continue; + } + if !c.is_ascii_digit() { + return None; + } + digits.push(c); + } + // `parse` turns down the empty string and anything past `u64::MAX`. + digits.parse().ok() +} + /// Files/sec. A nonzero rate never renders as "0.0": slow rates go per-minute. pub fn fmt_rate(files_per_sec: f64) -> String { if files_per_sec <= 0.0 { @@ -205,6 +232,66 @@ mod tests { assert_eq!(group_thousands(1_234_567), "1,234,567"); } + /// A typed number goes in whether or not it is punctuated — the field + /// shows separators, and someone retyping the value will not. + #[test] + fn grouped_numbers_parse_with_or_without_their_separators() { + assert_eq!(parse_grouped("52,428,800"), Some(52_428_800)); + assert_eq!(parse_grouped("52428800"), Some(52_428_800)); + assert_eq!(parse_grouped("52 428 800"), Some(52_428_800)); + assert_eq!(parse_grouped("52\u{202f}428\u{202f}800"), Some(52_428_800)); + assert_eq!(parse_grouped("52_428_800"), Some(52_428_800)); + // Separators are stripped, not validated: the field is for entering + // a number, not for spelling one correctly. + assert_eq!(parse_grouped("5,2,4,2,8,8,0,0"), Some(52_428_800)); + assert_eq!(parse_grouped(" 1024 "), Some(1024)); + assert_eq!(parse_grouped("0"), Some(0)); + } + + /// Anything that is not a whole number leaves the field's value alone. + #[test] + fn a_partial_or_bogus_entry_declines() { + // Empty is what a cleared field holds mid-edit; taking it as zero + // would write a zero to the config between keystrokes. + assert_eq!(parse_grouped(""), None); + assert_eq!(parse_grouped(" "), None); + assert_eq!(parse_grouped(","), None); + assert_eq!(parse_grouped("1.5"), None); + assert_eq!(parse_grouped("-1"), None); + assert_eq!(parse_grouped("2 MB"), None); + assert_eq!(parse_grouped("1e6"), None); + assert_eq!(parse_grouped("٤٢"), None); + // Past `u64::MAX`, rather than wrapping to something plausible. + assert_eq!(parse_grouped("18,446,744,073,709,551,616"), None); + } + + /// What the byte fields do every frame: render the stored number, then + /// read back exactly it. Every default and clamp bound, plus the widest + /// value any of those fields accepts. + #[test] + fn every_byte_field_value_survives_the_round_trip() { + for n in [ + 0, + 1, + 512, + 1024, + 8192, + 262_144, + 2_097_152, + 52_428_800, + 1_073_741_824, + 8_589_934_592, + u64::MAX, + ] { + assert_eq!( + parse_grouped(&group_thousands(n)), + Some(n), + "{} did not survive being displayed", + n + ); + } + } + #[test] fn rates_never_show_zero_for_nonzero() { assert_eq!(fmt_rate(0.0), "0 files/s"); diff --git a/crates/quicksearch-gui/src/settings_tab.rs b/crates/quicksearch-gui/src/settings_tab.rs index f5e1d72..1d238dd 100644 --- a/crates/quicksearch-gui/src/settings_tab.rs +++ b/crates/quicksearch-gui/src/settings_tab.rs @@ -1,6 +1,7 @@ //! The Settings tab. Edits happen on a draft; Apply validates, saves, and //! hands the new config to the app. +use crate::format; use crate::keychain; use crate::tips::{self, tip_row, Tipped}; use crate::ui_util::hint; @@ -61,6 +62,39 @@ impl Form { ui.add(egui::DragValue::new(value).range(range)) }); } + + /// [`Form::drag`] for a byte count, grouped as `52,428,800`. + /// + /// Only the byte fields get this. They are the ones whose values run to + /// eight and ten digits, where a misplaced zero is invisible; the rest + /// of the form counts files, minutes and milliseconds and reads better + /// plain. + /// + /// The number still goes in either way — [`format::parse_grouped`] + /// takes it with the separators or without — so a pasted `52,428,800` + /// and a typed `52428800` are the same edit. + fn drag_bytes( + self, + level: Level, + ui: &mut egui::Ui, + label: impl Into, + tip: &'static tips::Tip, + value: &mut N, + range: std::ops::RangeInclusive, + ) { + self.row(level, ui, label, tip, |ui| { + ui.add( + egui::DragValue::new(value) + .range(range) + // `DragValue` renders and parses through `f64`. Byte + // counts here are bounded by the config's own clamps (8 + // GiB at the largest), far inside the 2^53 an `f64` + // holds exactly, so the round trip cannot lose a digit. + .custom_formatter(|n, _| format::group_thousands(n.max(0.0) as u64)) + .custom_parser(|s| format::parse_grouped(s).map(|n| n as f64)), + ) + }); + } } /// A label ruled underneath in the palette's orange. @@ -884,7 +918,7 @@ fn config_editor_ui( ui.end_row(); } - form.drag( + form.drag_bytes( Level::Advanced, ui, "Hash sample size (bytes)", @@ -893,7 +927,7 @@ fn config_editor_ui( 512..=1_048_576, ); - form.drag( + form.drag_bytes( Level::Advanced, ui, "Max stored text (bytes)", @@ -902,7 +936,7 @@ fn config_editor_ui( 1024..=16_777_216, ); - form.drag( + form.drag_bytes( Level::Advanced, ui, "Max text file size (bytes)", @@ -920,7 +954,7 @@ fn config_editor_ui( 10..=100_000, ); - form.drag( + form.drag_bytes( Level::Advanced, ui, "Max WAL size (bytes)", diff --git a/crates/quicksearch-gui/src/settings_tab/tests.rs b/crates/quicksearch-gui/src/settings_tab/tests.rs index 2c7a909..2fc4e72 100644 --- a/crates/quicksearch-gui/src/settings_tab/tests.rs +++ b/crates/quicksearch-gui/src/settings_tab/tests.rs @@ -706,6 +706,46 @@ fn advanced_rows_are_hidden_until_asked_for() { ); } +/// The byte fields are shown grouped: their values run to eight and ten +/// digits, where a misplaced zero is invisible. Everything else in the form +/// counts files or milliseconds and stays plain, which is the other half of +/// the assertion — the grouping is scoped to the fields that needed it. +#[test] +fn only_the_byte_fields_are_shown_with_thousands_separators() { + let ctx = crate::test_ui::ctx(); + let mut cfg = Config::default(); + // The top of its range, so the plain rendering is long enough that a + // separator would show up if one were being inserted. + cfg.processing.batch_size = 100_000; + let out = ctx.run( + crate::test_ui::raw_input(egui::vec2(600.0, 800.0), vec![]), + |ctx| { + egui::CentralPanel::default().show(ctx, |ui| { + config_editor_ui( + ui, + &mut cfg, + Section::Processing, + None, + Form { advanced: true }, + ) + }); + }, + ); + let text = painted_text(&out).join("\n"); + + // Hash sample size, max stored text, max text file size, max WAL size. + for grouped in ["8,192", "262,144", "52,428,800", "2,147,483,648"] { + assert!( + text.contains(grouped), + "{grouped} is not on screen; the byte fields read: {text}" + ); + } + assert!( + text.contains("100000") && !text.contains("100,000"), + "batch size counts files, not bytes, and must stay plain: {text}" + ); +} + /// Revealing a setting is not editing one: the toggle writes through /// `SettingsOutput` and must never make the tab read as dirty, or looking at /// an advanced setting would demand an Apply. diff --git a/crates/quicksearch-gui/src/tips.rs b/crates/quicksearch-gui/src/tips.rs index e17d7ab..9f55366 100644 --- a/crates/quicksearch-gui/src/tips.rs +++ b/crates/quicksearch-gui/src/tips.rs @@ -211,8 +211,10 @@ pub static MAX_TEXT_FILE_SIZE: Tip = Tip { Those files still appear in results, found by their name, size or \ date.", examples: &[ - "2097152, 2 MB, skips very few ordinary documents.", - "52428800, 50 MB, when you search inside large log files or scanned PDFs.", + "52428800, 50 MB, the default, reads inside large log files, scanned PDFs \ + and presentations whose size is mostly pictures.", + "2097152, 2 MB, to open ordinary documents only and leave anything larger \ + to be found by name.", ], caution: None, };