diff --git a/.claude/CLAUDE.md b/.claude/CLAUDE.md index 627fd0a..5b9b45d 100644 --- a/.claude/CLAUDE.md +++ b/.claude/CLAUDE.md @@ -57,6 +57,10 @@ The scaffold is functional with image display, async OCR pipeline, drag-select o - Full Viewer window (headerbar, arrow key navigation) - File info in the headerbar (filename, dimensions, file size) - Async OCR (Tesseract TSV → word bounding boxes) +- OCR settings via `~/.config/quickview/config.toml` (`quickview-core` + `config.rs`): lang (precedence `--lang` > `QUICKVIEW_LANG` > config > + `eng`) and `tessdata_dir` (`--tessdata-dir` > config); both live in + `OcrOptions` and join the cache key - On-disk OCR cache (`~/.cache/quickview/ocr/`, keyed by path+lang+mtime+size; no eviction in v1 — see ADR-0009 implementation notes) - Drag-select overlay with word highlighting diff --git a/Cargo.lock b/Cargo.lock index 7891942..a5775bc 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -1621,7 +1621,7 @@ version = "3.4.0" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "219cb19e96be00ab2e37d6e299658a0cfa83e52429179969b0f0121b4ac46983" dependencies = [ - "toml_edit", + "toml_edit 0.23.10+spec-1.0.0", ] [[package]] @@ -1639,6 +1639,7 @@ version = "0.1.0" dependencies = [ "anyhow", "clap", + "quickview-core", "quickview-ui", "tempfile", "tracing", @@ -1657,6 +1658,7 @@ dependencies = [ "serde_json", "tempfile", "thiserror 2.0.18", + "toml 0.8.23", ] [[package]] @@ -1850,6 +1852,15 @@ dependencies = [ "syn", ] +[[package]] +name = "serde_spanned" +version = "0.6.9" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "bf41e0cfaf7226dca15e8197172c295a782857fcb97fad1808a166870dee75a3" +dependencies = [ + "serde", +] + [[package]] name = "serde_spanned" version = "1.0.4" @@ -1928,7 +1939,7 @@ dependencies = [ "cfg-expr", "heck", "pkg-config", - "toml", + "toml 0.9.12+spec-1.1.0", "version-compare", ] @@ -2000,6 +2011,18 @@ dependencies = [ "cfg-if", ] +[[package]] +name = "toml" +version = "0.8.23" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "dc1beb996b9d83529a9e75c17a1686767d148d70663143c7854d8b4a09ced362" +dependencies = [ + "serde", + "serde_spanned 0.6.9", + "toml_datetime 0.6.11", + "toml_edit 0.22.27", +] + [[package]] name = "toml" version = "0.9.12+spec-1.1.0" @@ -2008,13 +2031,22 @@ checksum = "cf92845e79fc2e2def6a5d828f0801e29a2f8acc037becc5ab08595c7d5e9863" dependencies = [ "indexmap", "serde_core", - "serde_spanned", - "toml_datetime", + "serde_spanned 1.0.4", + "toml_datetime 0.7.5+spec-1.1.0", "toml_parser", "toml_writer", "winnow", ] +[[package]] +name = "toml_datetime" +version = "0.6.11" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "22cddaf88f4fbc13c51aebbf5f8eceb5c7c5a9da2ac40a13519eb5b0a0e8f11c" +dependencies = [ + "serde", +] + [[package]] name = "toml_datetime" version = "0.7.5+spec-1.1.0" @@ -2024,6 +2056,20 @@ dependencies = [ "serde_core", ] +[[package]] +name = "toml_edit" +version = "0.22.27" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "41fe8c660ae4257887cf66394862d21dbca4a6ddd26f04a3560410406a2f819a" +dependencies = [ + "indexmap", + "serde", + "serde_spanned 0.6.9", + "toml_datetime 0.6.11", + "toml_write", + "winnow", +] + [[package]] name = "toml_edit" version = "0.23.10+spec-1.0.0" @@ -2031,7 +2077,7 @@ source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "84c8b9f757e028cee9fa244aea147aab2a9ec09d5325a9b01e0a49730c2b5269" dependencies = [ "indexmap", - "toml_datetime", + "toml_datetime 0.7.5+spec-1.1.0", "toml_parser", "winnow", ] @@ -2045,6 +2091,12 @@ dependencies = [ "winnow", ] +[[package]] +name = "toml_write" +version = "0.1.2" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "5d99f8c9a7727884afe522e9bd5edbfc91a3312b36a77b5fb8926e4c31a41801" + [[package]] name = "toml_writer" version = "1.0.6+spec-1.1.0" diff --git a/adrs/ADR-0009-Caching.md b/adrs/ADR-0009-Caching.md index bc5de50..91f8143 100644 --- a/adrs/ADR-0009-Caching.md +++ b/adrs/ADR-0009-Caching.md @@ -52,9 +52,11 @@ The implementation went **straight to on-disk**, revising the decision above: edited file then correctly misses) rather than the new one. The path is derived from the lowercased app name, so the app-ID rename (done: io.github.Green2Grey2.QuickView) did not move it on Linux. -- Tesseract is currently invoked with no psm/oem flags, so `lang` is the only - setting and it is in the key. **When OCR settings become configurable - (Phase 7 hardening: psm/oem, tessdata_fast/best), they must join the key.** +- Tesseract is invoked with no psm/oem flags, so the OCR settings in the key + are `lang` and (since Phase 7's config work) the optional `tessdata_dir`, + hashed with a presence marker so `None` and empty stay distinct. **The rule + stands: any newly configurable OCR setting (psm/oem, the downscale target) + must join the key.** - Writes are atomic (temp file + rename in the same directory): concurrent QuickView processes are a designed use case. - Entries are created `0600` in `0700` directories — they hold recognized diff --git a/crates/quickview-core/Cargo.toml b/crates/quickview-core/Cargo.toml index 98d4f13..c816a5f 100644 --- a/crates/quickview-core/Cargo.toml +++ b/crates/quickview-core/Cargo.toml @@ -14,6 +14,7 @@ serde = { version = "1", features = ["derive"] } serde_json = "1" directories = "5" blake3 = "1" +toml = "0.8" [dev-dependencies] tempfile = "3" diff --git a/crates/quickview-core/src/cache.rs b/crates/quickview-core/src/cache.rs index d3ac420..76c7925 100644 --- a/crates/quickview-core/src/cache.rs +++ b/crates/quickview-core/src/cache.rs @@ -1,30 +1,26 @@ //! On-disk OCR result cache. //! //! Entries are JSON files under `/ocr/`, keyed by a blake3 hash of -//! the image path, OCR language, and file mtime+size — so an edited file is -//! simply a cache miss (no invalidation logic needed). There is no eviction in +//! the image path, the OCR settings (language, tessdata dir), and file +//! mtime+size — so an edited file is simply a cache miss (no invalidation +//! logic needed). There is no eviction in //! v1: entries are a few KB each, and users can clear the directory manually. //! Phase 8's persistent SQLite cache is the planned successor (ADR-0009). use std::path::{Path, PathBuf}; -use directories::ProjectDirs; - -use crate::ocr::models::OcrResult; +use crate::ocr::{models::OcrResult, tesseract::OcrOptions}; /// Return the XDG cache directory for QuickView. /// -/// On Linux this is typically: `~/.cache/quickview/`. +/// On Linux this is typically: `~/.cache/quickview/` (derived from the +/// lowercased app name, so the app-ID rename did not move it and pre-rename +/// entries remain valid). pub fn cache_dir() -> Option { - // qualifier, org, app — must stay in sync with the application ID - // io.github.Green2Grey2.QuickView. On Linux the path only uses the - // lowercased app name (~/.cache/quickview/), so renaming the ID did not - // move the cache and pre-rename entries remain valid. - let proj = ProjectDirs::from("io.github", "Green2Grey2", "QuickView")?; - Some(proj.cache_dir().to_path_buf()) + Some(crate::config::project_dirs()?.cache_dir().to_path_buf()) } -pub fn ocr_cache_path(cache_root: &Path, file: &Path, lang: &str) -> PathBuf { +pub fn ocr_cache_path(cache_root: &Path, file: &Path, opts: &OcrOptions) -> PathBuf { // Include file metadata to avoid stale caches. Full nanosecond mtime: // whole seconds would alias a same-second rewrite of the same path with // an unchanged byte length (rapid screenshot/editor saves). @@ -40,7 +36,20 @@ pub fn ocr_cache_path(cache_root: &Path, file: &Path, lang: &str) -> PathBuf { let mut hasher = blake3::Hasher::new(); hasher.update(file.as_os_str().as_encoded_bytes()); hasher.update(b"\0"); - hasher.update(lang.as_bytes()); + hasher.update(opts.lang.as_bytes()); + hasher.update(b"\0"); + // Every OcrOptions field joins the key (ADR-0009): a different tessdata + // set produces different text. The presence marker keeps `None` distinct + // from `Some("")`. + match &opts.tessdata_dir { + Some(dir) => { + hasher.update(&[1]); + hasher.update(dir.as_os_str().as_encoded_bytes()); + } + None => { + hasher.update(&[0]); + } + } hasher.update(b"\0"); hasher.update(&mtime.to_le_bytes()); hasher.update(&size.to_le_bytes()); @@ -134,6 +143,13 @@ mod tests { } } + fn eng() -> OcrOptions { + OcrOptions { + lang: "eng".into(), + tessdata_dir: None, + } + } + #[test] fn key_changes_with_lang_path_and_metadata() { let dir = tempfile::tempdir().unwrap(); @@ -141,37 +157,41 @@ mod tests { let img = dir.path().join("a.png"); std::fs::write(&img, b"xx").unwrap(); - let base = ocr_cache_path(root, &img, "eng"); + let base = ocr_cache_path(root, &img, &eng()); // Same inputs -> same key. - assert_eq!(base, ocr_cache_path(root, &img, "eng")); + assert_eq!(base, ocr_cache_path(root, &img, &eng())); // Different language -> different key. - assert_ne!(base, ocr_cache_path(root, &img, "deu")); + let deu = OcrOptions { + lang: "deu".into(), + ..eng() + }; + assert_ne!(base, ocr_cache_path(root, &img, &deu)); // Different path -> different key. let img2 = dir.path().join("b.png"); std::fs::write(&img2, b"xx").unwrap(); - assert_ne!(base, ocr_cache_path(root, &img2, "eng")); + assert_ne!(base, ocr_cache_path(root, &img2, &eng())); // Different size -> different key. std::fs::write(&img, b"xxxx").unwrap(); - assert_ne!(base, ocr_cache_path(root, &img, "eng")); + assert_ne!(base, ocr_cache_path(root, &img, &eng())); // Different mtime (same size) -> different key. std::fs::write(&img, b"xx").unwrap(); - let before = ocr_cache_path(root, &img, "eng"); + let before = ocr_cache_path(root, &img, &eng()); let old = std::time::SystemTime::UNIX_EPOCH + std::time::Duration::from_secs(1_000_000); std::fs::File::open(&img) .unwrap() .set_modified(old) .unwrap(); - assert_ne!(before, ocr_cache_path(root, &img, "eng")); + assert_ne!(before, ocr_cache_path(root, &img, &eng())); // Subsecond mtime change (same second, same size) -> different key. let with_key = |t| { std::fs::File::open(&img).unwrap().set_modified(t).unwrap(); - ocr_cache_path(root, &img, "eng") + ocr_cache_path(root, &img, &eng()) }; assert_ne!( with_key(old + std::time::Duration::from_nanos(1)), @@ -179,12 +199,42 @@ mod tests { ); } + #[test] + fn key_changes_with_tessdata_dir() { + let dir = tempfile::tempdir().unwrap(); + let root = dir.path(); + let img = dir.path().join("a.png"); + std::fs::write(&img, b"xx").unwrap(); + + let with_dir = |d: Option<&str>| { + let opts = OcrOptions { + lang: "eng".into(), + tessdata_dir: d.map(PathBuf::from), + }; + ocr_cache_path(root, &img, &opts) + }; + + // Some(dir) differs from None, and dirs differ from each other. + assert_ne!(with_dir(None), with_dir(Some("/opt/tessdata_fast"))); + assert_ne!( + with_dir(Some("/opt/tessdata_fast")), + with_dir(Some("/opt/tessdata_best")) + ); + // The presence marker keeps None distinct from Some(""). + assert_ne!(with_dir(None), with_dir(Some(""))); + // Same dir -> same key. + assert_eq!( + with_dir(Some("/opt/tessdata_fast")), + with_dir(Some("/opt/tessdata_fast")) + ); + } + #[test] fn store_then_load_round_trips() { let dir = tempfile::tempdir().unwrap(); let img = dir.path().join("a.png"); std::fs::write(&img, b"xx").unwrap(); - let entry = ocr_cache_path(dir.path(), &img, "eng"); + let entry = ocr_cache_path(dir.path(), &img, &eng()); let result = sample_result(); store_ocr(&entry, &result).unwrap(); @@ -217,7 +267,7 @@ mod tests { let img = dir.path().join("a.png"); std::fs::write(&img, b"xx").unwrap(); - let entry = ocr_cache_path(dir.path(), &img, "eng"); + let entry = ocr_cache_path(dir.path(), &img, &eng()); assert!(load_ocr(&entry).is_none()); } @@ -227,7 +277,7 @@ mod tests { let img = dir.path().join("a.png"); std::fs::write(&img, b"xx").unwrap(); - let entry = ocr_cache_path(dir.path(), &img, "eng"); + let entry = ocr_cache_path(dir.path(), &img, &eng()); std::fs::create_dir_all(entry.parent().unwrap()).unwrap(); std::fs::write(&entry, b"{not json").unwrap(); diff --git a/crates/quickview-core/src/config.rs b/crates/quickview-core/src/config.rs new file mode 100644 index 0000000..9314c6c --- /dev/null +++ b/crates/quickview-core/src/config.rs @@ -0,0 +1,151 @@ +//! User configuration (`~/.config/quickview/config.toml`). +//! +//! Resolution precedence for every setting: CLI flag > environment variable +//! (lang only: `QUICKVIEW_LANG`) > config file > built-in default. The +//! precedence logic lives in pure functions here so it is testable headlessly; +//! reading the actual environment is the caller's job. + +use std::path::{Path, PathBuf}; + +use anyhow::{Context, Result}; +use directories::ProjectDirs; +use serde::Deserialize; + +/// Built-in default OCR language (Tesseract `-l`). +pub const DEFAULT_OCR_LANG: &str = "eng"; + +/// Return the per-project directories for QuickView. +/// +/// The triple must stay in sync with the application ID +/// `io.github.Green2Grey2.QuickView`. On Linux the derived paths only use the +/// lowercased app name (`~/.cache/quickview/`, `~/.config/quickview/`). +pub(crate) fn project_dirs() -> Option { + // qualifier, org, app + ProjectDirs::from("io.github", "Green2Grey2", "QuickView") +} + +/// Path of the config file (`~/.config/quickview/config.toml` on Linux). +pub fn config_path() -> Option { + Some(project_dirs()?.config_dir().join("config.toml")) +} + +#[derive(Debug, Clone, Default, PartialEq, Deserialize)] +#[serde(default, deny_unknown_fields)] +pub struct Config { + pub ocr: OcrSection, +} + +#[derive(Debug, Clone, Default, PartialEq, Deserialize)] +#[serde(default, deny_unknown_fields)] +pub struct OcrSection { + /// Default OCR language (overridden by `QUICKVIEW_LANG` and `--lang`). + pub lang: Option, + /// Directory with `.traineddata` files, e.g. a `tessdata_fast` or + /// `tessdata_best` checkout (overridden by `--tessdata-dir`). + pub tessdata_dir: Option, + /// Maximum image dimension before OCR downscales (not consumed yet; + /// accepted so configs written for the guardrail don't error). + pub max_dimension: Option, +} + +/// Load the config at `path`. +/// +/// A missing file is a normal, empty configuration. A file that exists but +/// does not parse (including unknown fields, which usually mean a typo) is an +/// `Err` so the caller can warn — a broken config must never prevent viewing +/// an image, but it must not be silently ignored either. +pub fn load(path: &Path) -> Result { + let text = match std::fs::read_to_string(path) { + Ok(text) => text, + Err(err) if err.kind() == std::io::ErrorKind::NotFound => return Ok(Config::default()), + Err(err) => return Err(err).with_context(|| format!("failed to read {}", path.display())), + }; + toml::from_str(&text).with_context(|| format!("failed to parse {}", path.display())) +} + +/// Resolve the effective OCR language. +/// +/// Precedence: CLI flag > env (`QUICKVIEW_LANG`) > config file > `"eng"`. +/// Blank or whitespace-only values are treated as unset at each level. +pub fn resolve_lang(cli: Option<&str>, env: Option<&str>, config: &Config) -> String { + [cli, env, config.ocr.lang.as_deref()] + .into_iter() + .flatten() + .map(str::trim) + .find(|s| !s.is_empty()) + .unwrap_or(DEFAULT_OCR_LANG) + .to_owned() +} + +#[cfg(test)] +mod tests { + use super::*; + + fn config_with_lang(lang: &str) -> Config { + Config { + ocr: OcrSection { + lang: Some(lang.to_owned()), + ..Default::default() + }, + } + } + + #[test] + fn lang_precedence_cli_env_config_default() { + let cfg = config_with_lang("fra"); + assert_eq!(resolve_lang(Some("deu"), Some("spa"), &cfg), "deu"); + assert_eq!(resolve_lang(None, Some("spa"), &cfg), "spa"); + assert_eq!(resolve_lang(None, None, &cfg), "fra"); + assert_eq!(resolve_lang(None, None, &Config::default()), "eng"); + } + + #[test] + fn blank_values_are_unset_at_each_level() { + let cfg = config_with_lang("fra"); + assert_eq!(resolve_lang(Some(""), Some(" "), &cfg), "fra"); + assert_eq!(resolve_lang(Some(" "), None, &config_with_lang("")), "eng"); + } + + #[test] + fn load_missing_file_is_default() { + let dir = tempfile::tempdir().unwrap(); + let cfg = load(&dir.path().join("nope.toml")).unwrap(); + assert_eq!(cfg, Config::default()); + } + + #[test] + fn load_parses_partial_config() { + let dir = tempfile::tempdir().unwrap(); + let path = dir.path().join("config.toml"); + std::fs::write(&path, "[ocr]\nlang = \"deu\"\n").unwrap(); + let cfg = load(&path).unwrap(); + assert_eq!(cfg.ocr.lang.as_deref(), Some("deu")); + assert_eq!(cfg.ocr.tessdata_dir, None); + + std::fs::write( + &path, + "[ocr]\ntessdata_dir = \"/opt/tessdata_fast\"\nmax_dimension = 4000\n", + ) + .unwrap(); + let cfg = load(&path).unwrap(); + assert_eq!(cfg.ocr.lang, None); + assert_eq!( + cfg.ocr.tessdata_dir.as_deref(), + Some(Path::new("/opt/tessdata_fast")) + ); + assert_eq!(cfg.ocr.max_dimension, Some(4000)); + } + + #[test] + fn load_rejects_garbage_and_typos() { + let dir = tempfile::tempdir().unwrap(); + let path = dir.path().join("config.toml"); + + std::fs::write(&path, "not toml [ at all").unwrap(); + assert!(load(&path).is_err()); + + // Unknown field: almost certainly a typo the user wants to hear about. + std::fs::write(&path, "[ocr]\nlanguage = \"deu\"\n").unwrap(); + assert!(load(&path).is_err()); + } +} diff --git a/crates/quickview-core/src/lib.rs b/crates/quickview-core/src/lib.rs index fec6457..622948c 100644 --- a/crates/quickview-core/src/lib.rs +++ b/crates/quickview-core/src/lib.rs @@ -3,6 +3,7 @@ //! This crate intentionally has **no GTK dependencies**. pub mod cache; +pub mod config; pub mod fs; pub mod geometry; pub mod ocr; diff --git a/crates/quickview-core/src/ocr/tesseract.rs b/crates/quickview-core/src/ocr/tesseract.rs index 33c5b15..da87daf 100644 --- a/crates/quickview-core/src/ocr/tesseract.rs +++ b/crates/quickview-core/src/ocr/tesseract.rs @@ -1,19 +1,32 @@ -use std::{path::Path, process::Command}; +use std::{ + ffi::OsString, + path::{Path, PathBuf}, + process::Command, +}; use anyhow::{anyhow, Context, Result}; +/// Settings that influence what tesseract recognizes. +/// +/// Every field here changes OCR output, so every field must join the cache +/// key (ADR-0009): see [`crate::cache::ocr_cache_path`]. +#[derive(Debug, Clone, PartialEq)] +pub struct OcrOptions { + /// Language(s), tesseract `-l` (e.g. `eng`, `deu`, `eng+deu`). + pub lang: String, + /// Directory with `.traineddata` files (`--tessdata-dir`), e.g. a + /// `tessdata_fast` or `tessdata_best` checkout. `None` uses the system + /// default. + pub tessdata_dir: Option, +} + /// Run tesseract and return TSV output (written to stdout). /// /// Command used (baseline): -/// `tesseract - -l tsv quiet` -pub fn run_tesseract_tsv(input: &Path, lang: &str) -> Result { +/// `tesseract - -l [--tessdata-dir ] tsv quiet` +pub fn run_tesseract_tsv(input: &Path, opts: &OcrOptions) -> Result { let output = Command::new("tesseract") - .arg(input) - .arg("-") - .arg("-l") - .arg(lang) - .arg("tsv") - .arg("quiet") + .args(tesseract_args(input, opts)) .output() .context("failed to spawn tesseract; is it installed and on PATH?")?; @@ -25,3 +38,55 @@ pub fn run_tesseract_tsv(input: &Path, lang: &str) -> Result { let stdout = String::from_utf8(output.stdout).context("tesseract output not valid UTF-8")?; Ok(stdout) } + +/// Build the tesseract argv. Options must precede the trailing `tsv quiet` +/// config names or tesseract treats them as config files. +fn tesseract_args(input: &Path, opts: &OcrOptions) -> Vec { + let mut args: Vec = vec![input.into(), "-".into(), "-l".into(), (&opts.lang).into()]; + if let Some(dir) = &opts.tessdata_dir { + args.push("--tessdata-dir".into()); + args.push(dir.into()); + } + args.push("tsv".into()); + args.push("quiet".into()); + args +} + +#[cfg(test)] +mod tests { + use super::*; + + fn opts(lang: &str, tessdata_dir: Option<&str>) -> OcrOptions { + OcrOptions { + lang: lang.to_owned(), + tessdata_dir: tessdata_dir.map(PathBuf::from), + } + } + + #[test] + fn args_baseline() { + let args = tesseract_args(Path::new("/tmp/a.png"), &opts("eng", None)); + assert_eq!(args, ["/tmp/a.png", "-", "-l", "eng", "tsv", "quiet"]); + } + + #[test] + fn args_with_tessdata_dir_precede_config_names() { + let args = tesseract_args( + Path::new("/tmp/a.png"), + &opts("deu", Some("/opt/tessdata_fast")), + ); + assert_eq!( + args, + [ + "/tmp/a.png", + "-", + "-l", + "deu", + "--tessdata-dir", + "/opt/tessdata_fast", + "tsv", + "quiet" + ] + ); + } +} diff --git a/crates/quickview-ui/src/ipc.rs b/crates/quickview-ui/src/ipc.rs index 02fde29..803d984 100644 --- a/crates/quickview-ui/src/ipc.rs +++ b/crates/quickview-ui/src/ipc.rs @@ -6,7 +6,8 @@ //! crosses the process boundary is only this fixed, sanitized form: //! //! ```text -//! quickview --mode= --lang= --file= +//! quickview --mode= --lang= \ +//! [--tessdata-dir=] --file= //! ``` //! //! Every value is glued to its key in a single `--key=value` token: GLib's @@ -25,6 +26,8 @@ use std::path::PathBuf; use anyhow::{anyhow, bail, Result}; +use quickview_core::ocr::tesseract::OcrOptions; + use crate::{LaunchOptions, Mode}; const MODE_QUICK_PREVIEW: &str = "quick-preview"; @@ -35,17 +38,22 @@ pub(crate) fn to_argv(opts: &LaunchOptions) -> Vec { Mode::QuickPreview => MODE_QUICK_PREVIEW, Mode::FullViewer => MODE_FULL_VIEWER, }; - vec![ + let mut argv = vec![ "quickview".to_owned(), format!("--mode={mode}"), - format!("--lang={}", opts.ocr_lang), - format!("--file={}", opts.file.to_string_lossy()), - ] + format!("--lang={}", opts.ocr.lang), + ]; + if let Some(dir) = &opts.ocr.tessdata_dir { + argv.push(format!("--tessdata-dir={}", dir.to_string_lossy())); + } + argv.push(format!("--file={}", opts.file.to_string_lossy())); + argv } pub(crate) fn from_argv(argv: &[OsString]) -> Result { let mut mode = None; let mut lang = None; + let mut tessdata_dir: Option = None; let mut file: Option = None; for arg in argv.iter().skip(1) { @@ -61,6 +69,8 @@ pub(crate) fn from_argv(argv: &[OsString]) -> Result { }); } else if let Some(value) = arg.strip_prefix("--lang=") { lang = Some(value.to_owned()); + } else if let Some(value) = arg.strip_prefix("--tessdata-dir=") { + tessdata_dir = Some(PathBuf::from(value)); } else if let Some(value) = arg.strip_prefix("--file=") { file = Some(PathBuf::from(value)); } else { @@ -70,7 +80,10 @@ pub(crate) fn from_argv(argv: &[OsString]) -> Result { Ok(LaunchOptions { mode: mode.ok_or_else(|| anyhow!("missing --mode"))?, - ocr_lang: lang.ok_or_else(|| anyhow!("missing --lang"))?, + ocr: OcrOptions { + lang: lang.ok_or_else(|| anyhow!("missing --lang"))?, + tessdata_dir, + }, file: file.ok_or_else(|| anyhow!("missing --file"))?, }) } @@ -83,7 +96,10 @@ mod tests { LaunchOptions { mode, file: PathBuf::from(file), - ocr_lang: lang.to_owned(), + ocr: OcrOptions { + lang: lang.to_owned(), + tessdata_dir: None, + }, } } @@ -116,6 +132,20 @@ mod tests { } } + #[test] + fn round_trips_tessdata_dir() { + let mut original = opts(Mode::QuickPreview, "eng", "/tmp/a.png"); + original.ocr.tessdata_dir = Some(PathBuf::from("/opt/tess data/fast")); + assert_eq!(round_trip(&original), original); + + // Absent stays absent (no --tessdata-dir token emitted at all). + let without = opts(Mode::QuickPreview, "eng", "/tmp/a.png"); + assert!(!to_argv(&without) + .iter() + .any(|a| a.starts_with("--tessdata-dir"))); + assert_eq!(round_trip(&without), without); + } + #[test] fn rejects_missing_pieces() { assert!(from_argv(&os_argv(&["quickview"])).is_err()); diff --git a/crates/quickview-ui/src/lib.rs b/crates/quickview-ui/src/lib.rs index 3ea0df1..9d08d69 100644 --- a/crates/quickview-ui/src/lib.rs +++ b/crates/quickview-ui/src/lib.rs @@ -14,6 +14,7 @@ mod ipc; pub mod widgets; pub mod windows; +use quickview_core::ocr::tesseract::OcrOptions; use windows::shared::ViewerController; #[derive(Debug, Clone, Copy, PartialEq, Eq)] @@ -26,7 +27,9 @@ pub enum Mode { pub struct LaunchOptions { pub mode: Mode, pub file: PathBuf, - pub ocr_lang: String, + /// Fully resolved OCR settings: CLI/env/config precedence is applied in + /// the invoking process before these cross the instance boundary. + pub ocr: OcrOptions, } /// Windows the primary instance manages across invocations. @@ -99,8 +102,8 @@ fn dispatch(app: &adw::Application, state: &Rc, opts: &LaunchOptions) window.close(); } else { // Explicit request for a different file: show it, with - // the language this invocation asked for. - controller.set_ocr_lang(opts.ocr_lang.clone()); + // the OCR settings this invocation asked for. + controller.set_ocr_options(opts.ocr.clone()); controller.load_file(&opts.file); window.present(); } diff --git a/crates/quickview-ui/src/windows/full_viewer.rs b/crates/quickview-ui/src/windows/full_viewer.rs index 54dc136..853f5ad 100644 --- a/crates/quickview-ui/src/windows/full_viewer.rs +++ b/crates/quickview-ui/src/windows/full_viewer.rs @@ -20,7 +20,7 @@ pub fn present(app: &adw::Application, opts: &LaunchOptions) { let header = adw::HeaderBar::new(); header.set_title_widget(Some(&title)); - let viewer = ViewerController::new(opts.file.clone(), opts.ocr_lang.clone()); + let viewer = ViewerController::new(opts.file.clone(), opts.ocr.clone()); { let title = title.clone(); diff --git a/crates/quickview-ui/src/windows/quick_preview.rs b/crates/quickview-ui/src/windows/quick_preview.rs index 548da07..8f9c738 100644 --- a/crates/quickview-ui/src/windows/quick_preview.rs +++ b/crates/quickview-ui/src/windows/quick_preview.rs @@ -27,7 +27,7 @@ pub fn present( .default_height(PANEL_HEIGHT) .build(); - let viewer = ViewerController::new(opts.file.clone(), opts.ocr_lang.clone()); + let viewer = ViewerController::new(opts.file.clone(), opts.ocr.clone()); // Layer shell if supported. if gtk4_layer_shell::is_supported() { diff --git a/crates/quickview-ui/src/windows/shared.rs b/crates/quickview-ui/src/windows/shared.rs index 88c3b83..3ce1f37 100644 --- a/crates/quickview-ui/src/windows/shared.rs +++ b/crates/quickview-ui/src/windows/shared.rs @@ -9,7 +9,7 @@ use gtk4 as gtk; use quickview_core::{ cache, fs, - ocr::{tesseract, tsv}, + ocr::{tesseract, tesseract::OcrOptions, tsv}, }; use crate::widgets::image_overlay::ImageOverlayWidget; @@ -39,7 +39,7 @@ pub struct ViewerController { dir_images: Rc>>, dir_index: Rc>, - ocr_lang: Rc>, + ocr: Rc>, // Monotonic ids to ignore late results from superseded jobs. decode_job_id: Rc>, @@ -50,7 +50,7 @@ pub struct ViewerController { } impl ViewerController { - pub fn new(initial_file: PathBuf, ocr_lang: String) -> Self { + pub fn new(initial_file: PathBuf, ocr: OcrOptions) -> Self { let overlay = ImageOverlayWidget::new(); let current_file = Rc::new(RefCell::new(initial_file.clone())); @@ -61,7 +61,7 @@ impl ViewerController { current_file, dir_images: Rc::new(RefCell::new(dir_images)), dir_index: Rc::new(Cell::new(dir_index)), - ocr_lang: Rc::new(RefCell::new(ocr_lang)), + ocr: Rc::new(RefCell::new(ocr)), decode_job_id: Rc::new(Cell::new(0)), ocr_job_id: Rc::new(Cell::new(0)), on_file_loaded: Rc::new(RefCell::new(None)), @@ -84,12 +84,12 @@ impl ViewerController { self.current_file.borrow().clone() } - /// Change the OCR language for subsequent loads. + /// Change the OCR settings for subsequent loads. /// /// Takes effect on the next `load_file` (jobs already in flight keep the - /// language they started with). - pub fn set_ocr_lang(&self, lang: String) { - *self.ocr_lang.borrow_mut() = lang; + /// settings they started with). + pub fn set_ocr_options(&self, ocr: OcrOptions) { + *self.ocr.borrow_mut() = ocr; } /// Register a callback fired whenever a file finishes loading (or fails). @@ -220,7 +220,7 @@ impl ViewerController { self.overlay.set_ocr_busy(true); self.overlay.set_ocr_result(None); - let lang = self.ocr_lang.borrow().clone(); + let ocr_opts = self.ocr.borrow().clone(); let (sender, receiver) = async_channel::bounded::<( u64, @@ -241,12 +241,12 @@ impl ViewerController { // which the edited file then correctly misses. let entry = cache_root .as_deref() - .map(|root| cache::ocr_cache_path(root, &path, &lang)); + .map(|root| cache::ocr_cache_path(root, &path, &ocr_opts)); if let Some(cached) = entry.as_deref().and_then(cache::load_ocr) { tracing::debug!("OCR cache hit for {}", path.display()); return Ok(cached); } - let tsv_out = tesseract::run_tesseract_tsv(&path, &lang)?; + let tsv_out = tesseract::run_tesseract_tsv(&path, &ocr_opts)?; let parsed = tsv::parse_tesseract_tsv(&tsv_out)?; // Empty results are cached too (text-free images shouldn't // re-run tesseract); failures are not, so transient errors diff --git a/crates/quickview/Cargo.toml b/crates/quickview/Cargo.toml index c0155c9..7dc707c 100644 --- a/crates/quickview/Cargo.toml +++ b/crates/quickview/Cargo.toml @@ -16,6 +16,7 @@ tracing-subscriber.workspace = true clap = { version = "4", features = ["derive"] } +quickview-core = { path = "../quickview-core" } quickview-ui = { path = "../quickview-ui" } [dev-dependencies] diff --git a/crates/quickview/src/main.rs b/crates/quickview/src/main.rs index be0f96e..beb1c5d 100644 --- a/crates/quickview/src/main.rs +++ b/crates/quickview/src/main.rs @@ -18,8 +18,14 @@ struct Cli { quick_preview: bool, /// OCR language (Tesseract -l). Example: eng, deu, spa. - #[arg(long, default_value = "eng")] - lang: String, + /// Overrides QUICKVIEW_LANG and the config file; defaults to eng. + #[arg(long)] + lang: Option, + + /// Directory with .traineddata files, e.g. a tessdata_fast or + /// tessdata_best checkout. Overrides the config file. + #[arg(long)] + tessdata_dir: Option, /// Image file path. Use '-' (or omit) to read a path from stdin. file: Option, @@ -43,15 +49,69 @@ fn main() -> Result<()> { quickview_ui::Mode::FullViewer }; + // All resolution happens here, in the invoking process: with the + // single-instance app, the primary must never re-resolve a remote + // invocation's environment or config — only fully resolved values cross + // the process boundary (see quickview-ui's ipc module). + let ocr = resolve_ocr_options(cli.lang, cli.tessdata_dir); + let code = quickview_ui::run(quickview_ui::LaunchOptions { mode, file: file_path, - ocr_lang: cli.lang, + ocr, })?; std::process::exit(code); } +/// Resolve OCR settings from CLI > env (`QUICKVIEW_LANG`, lang only) > +/// config file > defaults. +/// +/// A config file that fails to parse is warned about and treated as absent: +/// a typo must never prevent viewing an image (NFR-004). +fn resolve_ocr_options( + cli_lang: Option, + cli_tessdata_dir: Option, +) -> quickview_core::ocr::tesseract::OcrOptions { + use quickview_core::config; + + let cfg = config::config_path() + .map(|path| { + config::load(&path).unwrap_or_else(|err| { + tracing::warn!("ignoring config file: {err:#}"); + config::Config::default() + }) + }) + .unwrap_or_default(); + + let env_lang = std::env::var("QUICKVIEW_LANG").ok(); + quickview_core::ocr::tesseract::OcrOptions { + lang: config::resolve_lang(cli_lang.as_deref(), env_lang.as_deref(), &cfg), + tessdata_dir: non_blank(cli_tessdata_dir) + .or_else(|| non_blank(cfg.ocr.tessdata_dir.clone())) + .map(absolutize), + } +} + +/// Blank is unset, at each precedence level — same rule as `resolve_lang`. +fn non_blank(path: Option) -> Option { + path.filter(|p| !p.to_string_lossy().trim().is_empty()) +} + +/// Pin a possibly-relative path to this process's cwd. +/// +/// Like the image path, the tessdata dir must be absolutized in the invoking +/// process: with the single-instance app, OCR runs in the primary instance, +/// whose cwd has nothing to do with this invocation's. Canonicalizing also +/// resolves symlinks so one directory always hashes to one cache key; a +/// nonexistent path is made absolute without touching the filesystem and +/// fails later in tesseract with a clear error. +fn absolutize(path: PathBuf) -> PathBuf { + std::fs::canonicalize(&path) + .or_else(|_| std::path::absolute(&path)) + .unwrap_or(path) +} + fn resolve_input_path(arg: Option) -> Result { let path = match arg.as_deref() { None | Some("-") => { @@ -97,4 +157,37 @@ mod tests { let p = resolve_input_path(Some("does-not-exist.png".into())).unwrap(); assert_eq!(p, PathBuf::from("does-not-exist.png")); } + + #[test] + fn blank_tessdata_dir_is_unset() { + assert_eq!(non_blank(None), None); + assert_eq!(non_blank(Some(PathBuf::new())), None); + assert_eq!(non_blank(Some(PathBuf::from(" "))), None); + assert_eq!( + non_blank(Some(PathBuf::from("/opt/tessdata"))), + Some(PathBuf::from("/opt/tessdata")) + ); + } + + #[test] + fn absolutize_resolves_existing_and_pins_missing() { + let dir = tempfile::tempdir().unwrap(); + let sub = dir.path().join("tessdata"); + std::fs::create_dir(&sub).unwrap(); + + // Existing: canonicalized (dot components resolved). + let dotted = dir.path().join(".").join("tessdata"); + assert_eq!(absolutize(dotted), std::fs::canonicalize(&sub).unwrap()); + + // Missing: still made absolute against this process's cwd. + let missing = absolutize(PathBuf::from("no-such-tessdata")); + assert!(missing.is_absolute()); + assert!(missing.ends_with("no-such-tessdata")); + + // Already absolute + missing: unchanged. + assert_eq!( + absolutize(PathBuf::from("/nonexistent/tessdata")), + PathBuf::from("/nonexistent/tessdata") + ); + } } diff --git a/docs/PHASED_PLAN.md b/docs/PHASED_PLAN.md index 67ec93e..32404db 100644 --- a/docs/PHASED_PLAN.md +++ b/docs/PHASED_PLAN.md @@ -172,8 +172,13 @@ If priorities change, you can reshuffle phases, but try to keep the “render fi process-per-invocation benefits too. - Add basic benchmarking hooks (decode + OCR timing) - Improve OCR accuracy options: - - language selection via config file / env var (CLI `--lang` already exists) - - selectable `tessdata_fast` vs `tessdata_best` + - language selection via config file / env var ✅ — precedence + `--lang` > `QUICKVIEW_LANG` > `~/.config/quickview/config.toml` > `eng` + (`quickview-core/src/config.rs`; see `templates/config.example.toml`) + - selectable `tessdata_fast` vs `tessdata_best` ✅ — as a plain + `tessdata_dir` path (config or `--tessdata-dir`) rather than an enum: + the fast/best sets have no conventional install location, users clone + them anywhere. Joins the OCR cache key per ADR-0009. - Add guardrails: - maximum image dimensions for OCR (downscale) - memory usage limits (where feasible) diff --git a/templates/config.example.toml b/templates/config.example.toml new file mode 100644 index 0000000..8aa2ad3 --- /dev/null +++ b/templates/config.example.toml @@ -0,0 +1,23 @@ +# QuickView configuration. +# Copy to: ~/.config/quickview/config.toml +# +# Precedence for each setting: CLI flag > environment variable (lang only) +# > this file > built-in default. Unknown keys are treated as typos: the +# whole file is ignored with a warning, so keep it clean. + +[ocr] +# Default OCR language (tesseract -l). Multiple: "eng+deu". +# Overridden by QUICKVIEW_LANG and --lang. Default: "eng". +#lang = "eng" + +# Directory with .traineddata files, e.g. a clone of +# https://github.com/tesseract-ocr/tessdata_fast (faster, slightly less +# accurate) or tessdata_best (slower, most accurate). Overridden by +# --tessdata-dir. Default: the system tessdata (on Arch: +# /usr/share/tessdata, from tesseract-data-*). +# Changing this re-runs OCR (it is part of the cache key). +#tessdata_dir = "/home/you/src/tessdata_fast" + +# Maximum image dimension (px) before OCR works on a downscaled copy. +# Accepted but not consumed yet; the guardrail lands in a follow-up. +#max_dimension = 4000