From e2d35de867319d3b36704a3ebf7337be2f900673 Mon Sep 17 00:00:00 2001 From: radialmonster Date: Thu, 8 Oct 2026 14:21:58 -0400 Subject: [PATCH] LR-IMP-ADD-DIALOG: file dialogs match extensions in any letter case on Linux (#342) rfd turns each extension into a *.ext glob that GTK and the desktop portal match case-sensitively, so DSC_1234.NEF was hidden. One helper (dialog_filter) gives every dialog filter the lower, UPPER and bracket forms on Linux; the Add Photos filter uses import::EXTENSIONS. --- apps/lightcraft/src/dialog_filter.rs | 145 +++++++++++++++++++++++++++ apps/lightcraft/src/main.rs | 20 ++-- docs/parity.md | 2 +- 3 files changed, 154 insertions(+), 13 deletions(-) create mode 100644 apps/lightcraft/src/dialog_filter.rs diff --git a/apps/lightcraft/src/dialog_filter.rs b/apps/lightcraft/src/dialog_filter.rs new file mode 100644 index 00000000..87760f79 --- /dev/null +++ b/apps/lightcraft/src/dialog_filter.rs @@ -0,0 +1,145 @@ +//! File-dialog filters that match extensions whatever their letter case. +//! +//! On Linux, `rfd` turns each extension into a `*.ext` glob for the GTK dialog and for the +//! desktop portal (`xdg-desktop-portal` FileChooser), and both match globs case-sensitively, so +//! a filter of `nef` hid `DSC_1234.NEF` (issue #342). Windows and macOS ignore case. Every +//! dialog filter of the app goes through [`FileDialogExt::add_filter_nocase`] (a test checks +//! that nothing else calls `add_filter`), so no extension can hit this. + +use rfd::FileDialog; + +/// The patterns to hand to `rfd` for `exts`, which are plain extensions without the dot. +/// +/// Windows and macOS take the extension literally and ignore case: lower case, de-duplicated. +/// Linux (`linux`): `rfd` formats each entry as `*.{entry}`, which GTK 3 and the portal match +/// as a glob, so an entry may carry glob syntax. Each extension gives the lower-case form, the +/// UPPER-case form (kept in case a portal backend does not do `[...]` classes) and a bracket +/// form such as `[nN][eE][fF]` that matches every mix (`.Nef`, `.nEf`, ...). +pub fn dialog_extensions_for(exts: &[&str], linux: bool) -> Vec { + let mut out: Vec = Vec::new(); + let mut push = |s: String| { + if !s.is_empty() && !out.contains(&s) { + out.push(s); + } + }; + for ext in exts { + let ext = ext.trim_start_matches('.'); + let lower = ext.to_lowercase(); + push(lower.clone()); + if !linux { + continue; + } + push(ext.to_uppercase()); + // `[` `]` `*` `?` in an extension would already be glob syntax: leave those alone + if lower.chars().any(|c| c.is_alphabetic()) && !lower.contains(['[', ']', '*', '?', '\\']) { + push( + lower + .chars() + .map(|c| { + let up: String = c.to_uppercase().collect(); + if up.chars().count() == 1 && up != c.to_string() { format!("[{c}{up}]") } else { c.to_string() } + }) + .collect(), + ); + } + } + out +} + +/// [`dialog_extensions_for`] for the platform this is built for. +pub fn dialog_extensions(exts: &[&str]) -> Vec { + dialog_extensions_for(exts, cfg!(all(unix, not(target_os = "macos")))) +} + +/// `add_filter` that matches the extensions in any letter case. +pub trait FileDialogExt { + fn add_filter_nocase(self, name: impl Into, exts: &[&str]) -> Self; +} + +impl FileDialogExt for FileDialog { + fn add_filter_nocase(self, name: impl Into, exts: &[&str]) -> Self { + self.add_filter(name, &dialog_extensions(exts)) + } +} + +#[cfg(test)] +#[allow(clippy::unwrap_used)] +mod tests { + use super::*; + + #[test] + fn other_platforms_get_lower_case_once() { + assert_eq!(dialog_extensions_for(&["NEF", "nef", ".Jpg", "jpg", ""], false), ["nef", "jpg"]); + } + + #[test] + fn linux_gets_lower_upper_and_any_mix() { + assert_eq!(dialog_extensions_for(&["nef"], true), ["nef", "NEF", "[nN][eE][fF]"]); + // digits have no case; a duplicate adds nothing + assert_eq!(dialog_extensions_for(&["mp4", "MP4"], true), ["mp4", "MP4", "[mM][pP]4"]); + assert_eq!(dialog_extensions_for(&["7z"], true), ["7z", "7Z", "7[zZ]"]); + assert_eq!(dialog_extensions_for(&["123"], true), ["123"]); + } + + #[test] + fn linux_glob_syntax_is_not_wrapped_again() { + assert_eq!(dialog_extensions_for(&["[nN]ef"], true), ["[nn]ef", "[NN]EF"]); + } + + /// The Linux patterns match like GTK's glob (`*.{entry}`): checked against a tiny matcher of + /// the subset used here (verified against real GTK 3.24 `GtkFileFilter` in the PR notes). + fn glob_ext(entry: &str, ext: &str) -> bool { + let mut pat = entry.chars().peekable(); + let mut s = ext.chars(); + while let Some(p) = pat.next() { + let Some(c) = s.next() else { return false }; + if p == '[' { + let mut hit = false; + for q in pat.by_ref() { + if q == ']' { + break; + } + hit |= q == c; + } + if !hit { + return false; + } + } else if p != c { + return false; + } + } + s.next().is_none() + } + + #[test] + fn linux_patterns_match_every_case_mix() { + let pats = dialog_extensions_for(&["nef", "cr2", "xmp"], true); + let matches = |name: &str| pats.iter().any(|p| glob_ext(p, name)); + for ok in ["nef", "NEF", "Nef", "nEF", "CR2", "Cr2", "XMP", "xMp"] { + assert!(matches(ok), "{ok}"); + } + for bad in ["nex", "ne", "nefs", "jpg", "cr3"] { + assert!(!matches(bad), "{bad}"); + } + } + + /// The app's own extension list (the one the folder scan uses) is covered whole. + #[test] + fn import_extensions_are_all_covered_in_both_cases() { + let pats = dialog_extensions_for(lightcraft_engine::import::EXTENSIONS, true); + for e in lightcraft_engine::import::EXTENSIONS { + assert!(pats.iter().any(|p| glob_ext(p, &e.to_uppercase())), "{e}"); + assert!(pats.iter().any(|p| p == e), "{e}"); + } + } + + /// Every dialog filter goes through the helper: `main.rs` must not call rfd's own + /// `add_filter`. + #[test] + fn no_dialog_bypasses_the_helper() { + for (file, src) in [("main.rs", include_str!("main.rs")), ("control_server.rs", include_str!("control_server.rs"))] { + assert!(!src.contains(".add_filter("), "{file} calls rfd's add_filter; use add_filter_nocase"); + } + assert!(include_str!("main.rs").contains(".add_filter_nocase(")); + } +} diff --git a/apps/lightcraft/src/main.rs b/apps/lightcraft/src/main.rs index 4bb1c6ff..8b399854 100644 --- a/apps/lightcraft/src/main.rs +++ b/apps/lightcraft/src/main.rs @@ -24,9 +24,11 @@ mod alloc_release; mod control_server; +mod dialog_filter; #[cfg(target_os = "macos")] mod native_menu; +use dialog_filter::FileDialogExt as _; use lightcraft_engine::Session; use lightcraft_ui_egui::panels::library_problem::LibraryProblem; use lightcraft_ui_egui::{LightcraftApp, Services, UiState}; @@ -283,13 +285,7 @@ fn services() -> Services { })), pick_files: Some(Box::new(|| { rfd::FileDialog::new() - .add_filter( - lightcraft_ui_egui::i18n::tr("Photos"), - &[ - "jpg", "jpeg", "png", "tif", "tiff", "webp", "dng", "cr2", "cr3", "nef", "nrw", "arw", "raf", "orf", "rw2", "rwl", "raw", - "pef", "psd", "jxl", "gif", "bmp", - ], - ) + .add_filter_nocase(lightcraft_ui_egui::i18n::tr("Photos"), lightcraft_engine::import::EXTENSIONS) .pick_files() .unwrap_or_default() .into_iter() @@ -299,7 +295,7 @@ fn services() -> Services { pick_preset_files: Some(Box::new(|| { rfd::FileDialog::new() .set_title(lightcraft_ui_egui::i18n::tr("Import Presets")) - .add_filter( + .add_filter_nocase( lightcraft_ui_egui::i18n::tr("Presets & Profiles"), &["lcpreset", "xmp", "lrtemplate", "zip", "dng", "lmp", "mplumpack", "cube"], ) @@ -312,7 +308,7 @@ fn services() -> Services { pick_tracklog: Some(Box::new(|| { rfd::FileDialog::new() .set_title(lightcraft_ui_egui::i18n::tr("Auto-Tag from Tracklog")) - .add_filter(lightcraft_ui_egui::i18n::tr("GPS Track Log"), &["gpx"]) + .add_filter_nocase(lightcraft_ui_egui::i18n::tr("GPS Track Log"), &["gpx"]) .pick_file() .map(|p| vec![p.to_string_lossy().to_string()]) .unwrap_or_default() @@ -320,7 +316,7 @@ fn services() -> Services { save_preset_file: Some(Box::new(|name: &str| { rfd::FileDialog::new() .set_title(lightcraft_ui_egui::i18n::tr("Export Presets")) - .add_filter(lightcraft_ui_egui::i18n::tr("LightCraft Preset"), &["lcpreset"]) + .add_filter_nocase(lightcraft_ui_egui::i18n::tr("LightCraft Preset"), &["lcpreset"]) .set_file_name(name) .save_file() .map(|p| p.to_string_lossy().to_string()) @@ -328,7 +324,7 @@ fn services() -> Services { pick_curve_preset_files: Some(Box::new(|| { rfd::FileDialog::new() .set_title(lightcraft_ui_egui::i18n::tr("Import Point Curve Presets")) - .add_filter(lightcraft_ui_egui::i18n::tr("Point Curve Presets"), &["lccurve", "json"]) + .add_filter_nocase(lightcraft_ui_egui::i18n::tr("Point Curve Presets"), &["lccurve", "json"]) .pick_files() .unwrap_or_default() .into_iter() @@ -338,7 +334,7 @@ fn services() -> Services { save_curve_preset_file: Some(Box::new(|name: &str| { rfd::FileDialog::new() .set_title(lightcraft_ui_egui::i18n::tr("Export Point Curve Presets")) - .add_filter(lightcraft_ui_egui::i18n::tr("Point Curve Presets"), &["lccurve"]) + .add_filter_nocase(lightcraft_ui_egui::i18n::tr("Point Curve Presets"), &["lccurve"]) .set_file_name(name) .save_file() .map(|p| p.to_string_lossy().to_string()) diff --git a/docs/parity.md b/docs/parity.md index d5a31ea9..0dba4450 100644 --- a/docs/parity.md +++ b/docs/parity.md @@ -113,7 +113,7 @@ under the UI command that wraps it (a no-op error); the UI command now wins. | Id | Feature | Tier | Status | Evidence | Missing / notes | |---|---|---|---|---|---| -| LR-IMP-ADD-DIALOG | Add photos/folders | P0 | ✅ | `cmd:file.addPhotos`, `cmd:library.importPreview`, `cmd:library.import` (`mode`, `album`, `albumName`, `preset`, `keywords`), `crates/ui-egui/src/import.rs` | review dialog (Import Photos): the scanned source (a scanned folder is not added to Local), candidate grid with thumbnails and checkboxes (duplicates by path/content marked and unchecked), transfer mode (add in place / copy / move, each explained), album (existing/new), preset, keywords; the folder scan runs on a background thread with a progress window and Cancel (a NAS folder no longer freezes the window); the import itself (probing, sidecars, copying / moving files) runs on a worker thread and its batches join the library between frames, with a progress window and Cancel; one undo step. Drag-and-drop goes through the same background import | +| LR-IMP-ADD-DIALOG | Add photos/folders | P0 | ✅ | `cmd:file.addPhotos`, `cmd:library.importPreview`, `cmd:library.import` (`mode`, `album`, `albumName`, `preset`, `keywords`), `crates/ui-egui/src/import.rs` | review dialog (Import Photos): the scanned source (a scanned folder is not added to Local), candidate grid with thumbnails and checkboxes (duplicates by path/content marked and unchecked), transfer mode (add in place / copy / move, each explained), album (existing/new), preset, keywords; the folder scan runs on a background thread with a progress window and Cancel (a NAS folder no longer freezes the window); the import itself (probing, sidecars, copying / moving files) runs on a worker thread and its batches join the library between frames, with a progress window and Cancel; one undo step. Drag-and-drop goes through the same background import. File dialogs list the importable extensions (`lightcraft_engine::import::EXTENSIONS`) in any letter case on Linux, where GTK and the desktop portal match globs case-sensitively (`apps/lightcraft/src/dialog_filter.rs`, issue #342) | | LR-IMP-DRAGDROP | Drop files/folders to import | P0 | ✅ | `crates/ui-egui/src/lib.rs` (dropped files → `cmd:library.import`) | dropping onto a specific album not supported | | LR-IMP-DUPES | Skip duplicates by content | P1 | ✅ | `crates/engine/src/import.rs`, `crates/engine/src/tests_import.rs` | | | LR-IMP-DEVICE | Import from camera/card | P1 | ✅ | `cmd:library.devices`, `cmd:file.addFromDevice`, `crates/engine/src/devices.rs` | mounted volumes with a DCIM folder (macOS /Volumes, Linux /media and /run/media, Windows drive letters); File → Import from Device → the import review, copying into the library by default; menus serve the last scan and rescan in the background (never blocking a frame; hot-plugs repaint); copies are verified (a new file, synced, checked against the content hash the scan computed — byte compare with the source when there is none, issue #134; a bad copy is removed and reported as failed, never counted as imported; taken names get -1, -2…, never replaced — issue #96, `crates/engine/src/import_move.rs`); no PTP/MTP (cameras that don't mount as a disk) |