From f71d6d9c0b24c2511f5a4013efc1e469c87ac7d6 Mon Sep 17 00:00:00 2001 From: OpenSauce Date: Fri, 16 Oct 2026 19:47:22 +0100 Subject: [PATCH 1/2] feat(nam): group models by folder in a nested picker menu Packs ship as folders of dozens of files, and the NAM stage's pick list is a flat list, so until now every model had to sit loose in the nam folder and the picker grew with the collection. The loader now walks the nam folder recursively and keys each model by its path relative to that folder, without the extension and with `/` separators on every platform (`Pack/Clean`). A top-level file keeps its bare stem, so presets saved before this change keep resolving. The cab-from-name guess still reads the file's own name, never the folder it sits in. The picker is an `iced_aw` menu bar with a single root: each folder fans out sideways into its contents, nesting as deep as the directory tree goes, and loose files sit alongside the folders at the same level, sorted together by name. The root menu is as wide as the button so it hangs straight below it like a pick list; the bar would otherwise right-align it whenever the button sits in the right half of the window. The root button is sized through `responsive` because the bar lays its root out against an unbounded width, where a `Fill` button never gets a size. `iced_aw` is pulled in with only the `menu` feature. Co-Authored-By: Claude Fable 5.1 --- Cargo.lock | 37 ++++ rustortion-core/src/nam/loader.rs | 115 ++++++++++-- rustortion-ui/Cargo.toml | 1 + rustortion-ui/src/stages/nam.rs | 295 ++++++++++++++++++++++++++---- 4 files changed, 404 insertions(+), 44 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index 8ac48a1..b7ab029 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -1927,6 +1927,19 @@ dependencies = [ "thiserror 2.0.18", ] +[[package]] +name = "iced_aw" +version = "0.14.1" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "59b7f923642b024c415150b70ef84ee5c11626e3b0af73b954d817d573bcd0a4" +dependencies = [ + "cfg-if", + "iced_core", + "iced_fonts", + "iced_widget", + "web-time", +] + [[package]] name = "iced_baseview" version = "0.1.0" @@ -1977,6 +1990,29 @@ dependencies = [ "log", ] +[[package]] +name = "iced_fonts" +version = "0.3.0" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "214cff7c8499e328774216690e58e315a1a5f8f6fdd1035aed6298e62ffc4c1d" +dependencies = [ + "iced_core", + "iced_fonts_macros", + "iced_widget", +] + +[[package]] +name = "iced_fonts_macros" +version = "0.3.0" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "7ef5125e110cb19cd1910a28298661c98c5d9ab02eef43594968352940e8752e" +dependencies = [ + "proc-macro2", + "quote", + "syn 2.0.117", + "ttf-parser", +] + [[package]] name = "iced_futures" version = "0.14.0" @@ -3866,6 +3902,7 @@ version = "0.3.0" dependencies = [ "anyhow", "iced", + "iced_aw", "log", "rustortion-core", "serde", diff --git a/rustortion-core/src/nam/loader.rs b/rustortion-core/src/nam/loader.rs index 09493c0..6481b05 100644 --- a/rustortion-core/src/nam/loader.rs +++ b/rustortion-core/src/nam/loader.rs @@ -8,12 +8,15 @@ use nam_rs::NamModel; use super::info::ModelInfo; -/// Scans a directory for `*.nam` files and parses each into memory. +/// Scans a directory tree for `*.nam` files and parses each into memory. /// /// Parsing happens once, at construction (off the real-time thread). Models are -/// keyed by display name (the file stem). Unparseable files are skipped with a -/// warning rather than failing the whole scan, matching the IR loader's tolerant -/// behaviour. +/// keyed by their path relative to the scanned folder, without the extension and +/// with `/` separators on every platform (`Pack/Clean`), so the UI can group them +/// by folder and two packs may each ship a `Clean.nam`. A top-level file is keyed +/// by its bare stem, which keeps presets saved before folders existed resolving. +/// Unparseable files are skipped with a warning rather than failing the whole +/// scan, matching the IR loader's tolerant behaviour. pub struct NamLoader { models: BTreeMap>, /// Display summaries, extracted once here rather than per GUI frame. @@ -30,8 +33,8 @@ impl NamLoader { /// Either way the result is an empty loader, never an error: failing to create /// the folder (read-only home, permissions) must not stop the app from starting. pub fn new(directory: &Path) -> Result { - let mut models = BTreeMap::new(); - let mut info = BTreeMap::new(); + let models = BTreeMap::new(); + let info = BTreeMap::new(); if !directory.is_dir() { match std::fs::create_dir_all(directory) { @@ -44,8 +47,17 @@ impl NamLoader { return Ok(Self { models, info }); } - let entries = std::fs::read_dir(directory) - .with_context(|| format!("Failed to read NAM directory '{}'", directory.display()))?; + let mut loader = Self { models, info }; + loader.scan_dir(directory, directory)?; + Ok(loader) + } + + /// Walk `current` (a folder under `base`), parsing models and recursing into + /// subfolders. Only the top-level read is a hard error: an unreadable subfolder + /// is skipped with a warning so one bad pack cannot hide every other model. + fn scan_dir(&mut self, current: &Path, base: &Path) -> Result<()> { + let entries = std::fs::read_dir(current) + .with_context(|| format!("Failed to read NAM directory '{}'", current.display()))?; for entry in entries { let entry = match entry { @@ -56,10 +68,21 @@ impl NamLoader { } }; let path = entry.path(); + if path.is_dir() { + if let Err(e) = self.scan_dir(&path, base) { + warn!("Skipping NAM subfolder '{}': {e}", path.display()); + } + continue; + } if path.extension().and_then(|e| e.to_str()) != Some("nam") { continue; } - let Some(name) = path.file_stem().and_then(|s| s.to_str()).map(str::to_owned) else { + // The cab-from-name guess reads the file's own name: a folder called + // "Full Rig" says nothing about the files inside it. + let Some(stem) = path.file_stem().and_then(|s| s.to_str()) else { + continue; + }; + let Some(name) = Self::model_key(&path, base) else { continue; }; @@ -74,14 +97,29 @@ impl NamLoader { ); // Summarize now: `metadata_typed` re-parses the whole JSON, so // the GUI must never do it per frame. - info.insert(name.clone(), Arc::new(ModelInfo::from_model(&model, &name))); - models.insert(name, Arc::new(model)); + self.info + .insert(name.clone(), Arc::new(ModelInfo::from_model(&model, stem))); + self.models.insert(name, Arc::new(model)); } Err(e) => warn!("Skipping NAM file '{}': {e}", path.display()), } } - Ok(Self { models, info }) + Ok(()) + } + + /// The registry key for a model file: its path relative to `base`, extension + /// dropped, with `/` separators regardless of platform. + fn model_key(path: &Path, base: &Path) -> Option { + let relative = path.strip_prefix(base).ok()?.with_extension(""); + let mut key = String::new(); + for component in relative.components() { + if !key.is_empty() { + key.push('/'); + } + key.push_str(component.as_os_str().to_str()?); + } + Some(key) } /// Sorted list of available model display names. @@ -170,4 +208,57 @@ mod tests { assert!(info.is_empty()); assert!(loader.info("nonexistent").is_none()); } + /// Packs ship as a folder of files. A model inside one is keyed by its path + /// relative to the nam folder (forward slashes, no extension), so the UI can + /// group by folder and two packs can both ship a `Clean.nam`. Top-level files + /// keep their bare stem: a saved preset that names one keeps resolving. + #[test] + fn nested_directories_are_scanned_with_folder_prefixed_names() { + let tmp = tempfile::tempdir().expect("tempdir"); + let dir = tmp.path(); + let fixture = + Path::new(env!("CARGO_MANIFEST_DIR")).join("tests/fixtures/reference_standard.nam"); + + std::fs::copy(&fixture, dir.join("top.nam")).expect("copy"); + std::fs::create_dir_all(dir.join("Pack/Deep")).expect("mkdir"); + std::fs::copy(&fixture, dir.join("Pack/inner.nam")).expect("copy"); + std::fs::copy(&fixture, dir.join("Pack/Deep/leaf.nam")).expect("copy"); + // Folders without any models contribute nothing. + std::fs::create_dir_all(dir.join("Empty")).expect("mkdir"); + + let loader = NamLoader::new(dir).expect("scan"); + + assert_eq!( + loader.available_names(), + vec!["Pack/Deep/leaf", "Pack/inner", "top"] + ); + assert!(loader.get("Pack/Deep/leaf").is_some()); + assert!(loader.info("Pack/inner").is_some()); + } + + /// The name-based cab guess must look at the file's own name only. A folder + /// called "Full Rig" says nothing about a `plain.nam` inside it, while a file + /// called `…-FullRig.nam` is evidence wherever it lives. + #[test] + fn cab_guess_from_name_ignores_folder_names() { + let tmp = tempfile::tempdir().expect("tempdir"); + let dir = tmp.path(); + let fixture = + Path::new(env!("CARGO_MANIFEST_DIR")).join("tests/fixtures/reference_standard.nam"); + + std::fs::create_dir_all(dir.join("FullRig")).expect("mkdir"); + std::fs::copy(&fixture, dir.join("FullRig/plain.nam")).expect("copy"); + std::fs::create_dir_all(dir.join("Pack")).expect("mkdir"); + std::fs::copy(&fixture, dir.join("Pack/Amp-FullRig.nam")).expect("copy"); + + let loader = NamLoader::new(dir).expect("scan"); + + let plain = loader.info("FullRig/plain").expect("loaded"); + assert_eq!(plain.includes_cab, None, "folder name is not evidence"); + assert!(!plain.cab_from_name); + + let rig = loader.info("Pack/Amp-FullRig").expect("loaded"); + assert_eq!(rig.includes_cab, Some(true), "file name is evidence"); + assert!(rig.cab_from_name); + } } diff --git a/rustortion-ui/Cargo.toml b/rustortion-ui/Cargo.toml index 449bd40..e6cdade 100644 --- a/rustortion-ui/Cargo.toml +++ b/rustortion-ui/Cargo.toml @@ -5,6 +5,7 @@ edition = "2024" [dependencies] iced = { version = "0.14", features = ["tokio"] } +iced_aw = { version = "0.14.1", default-features = false, features = ["menu"] } rustortion-core = { path = "../rustortion-core" } serde = { version = "1.0", features = ["derive"] } anyhow = "1.0" diff --git a/rustortion-ui/src/stages/nam.rs b/rustortion-ui/src/stages/nam.rs index f21f26f..7c629f6 100644 --- a/rustortion-ui/src/stages/nam.rs +++ b/rustortion-ui/src/stages/nam.rs @@ -1,12 +1,13 @@ -use iced::widget::{button, column, container, pick_list, row, rule, text}; -use iced::{Alignment, Element, Length, Padding}; +use iced::widget::{button, column, container, responsive, row, rule, text}; +use iced::{Alignment, Border, Color, Element, Length, Padding}; +use iced_aw::menu::{self, Item, Menu, MenuBar}; use rustortion_core::amp::stages::nam::NamConfig; use rustortion_core::nam::registry; use crate::components::widgets::common::{ - COLOR_SUBTLE, COLOR_WARNING, SPACING_NORMAL, SPACING_TIGHT, SPACING_WIDE, StageViewState, TEXT_SIZE_SMALL, - labeled_slider, stage_card, + BORDER_RADIUS_CARD, COLOR_SUBTLE, COLOR_WARNING, PADDING_NORMAL, PADDING_SMALL, SPACING_NORMAL, + SPACING_TIGHT, SPACING_WIDE, StageViewState, TEXT_SIZE_SMALL, labeled_slider, stage_card, }; use crate::messages::Message; use crate::tr; @@ -25,32 +26,171 @@ pub enum NamMessage { Rescan, /// Reveal the NAM models directory in the file manager. OpenFolder, + /// The picker's root button was pressed. The menu bar opens itself on the + /// release; this exists only so the button has a press handler. + MenuOpened, } -/// A pick-list entry: either a named model or an explicit "no model" choice that -/// clears the selection back to passthrough. +// --- Model tree --- + +/// One entry of the model picker menu: a folder that fans out into its contents, or +/// a model that can be selected. Built from the registry's flat `Folder/Model` keys. #[derive(Debug, Clone, PartialEq, Eq)] -enum NamModelChoice { - None, - Model(String), +enum ModelNode { + Folder { + label: String, + children: Vec, + }, + Model { + /// Shown in the menu: the file's own name. + label: String, + /// Stored in the preset and looked up in the registry: the full key. + key: String, + }, } -impl NamModelChoice { - fn into_option(self) -> Option { +impl ModelNode { + fn label(&self) -> &str { match self { - Self::None => None, - Self::Model(name) => Some(name), + Self::Folder { label, .. } | Self::Model { label, .. } => label, } } } -impl std::fmt::Display for NamModelChoice { - fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result { - match self { - Self::None => f.write_str(tr!(nam_no_model)), - Self::Model(name) => f.write_str(name), +/// The last path segment of a registry key: what the closed picker shows, since +/// the folder is visible in the menu and the full key would not fit the card. +fn leaf_label(key: &str) -> &str { + key.rsplit('/').next().unwrap_or(key) +} + +/// Group registry keys into a folder tree. Folders and loose files sort together +/// by name, ignoring case, at every level. +fn model_tree(keys: &[String]) -> Vec { + let mut root = Vec::new(); + for key in keys { + let mut segments = key.split('/').peekable(); + let mut level = &mut root; + while let Some(segment) = segments.next() { + if segments.peek().is_none() { + level.push(ModelNode::Model { + label: segment.to_owned(), + key: key.clone(), + }); + break; + } + let position = level + .iter() + .position(|node| matches!(node, ModelNode::Folder { label, .. } if label == segment)) + .unwrap_or_else(|| { + level.push(ModelNode::Folder { + label: segment.to_owned(), + children: Vec::new(), + }); + level.len() - 1 + }); + let ModelNode::Folder { children, .. } = &mut level[position] else { + unreachable!("position was found by matching a Folder"); + }; + level = children; } } + sort_tree(&mut root); + root +} + +fn sort_tree(nodes: &mut [ModelNode]) { + nodes.sort_by_cached_key(|node| node.label().to_lowercase()); + for node in nodes { + if let ModelNode::Folder { children, .. } = node { + sort_tree(children); + } + } +} + +/// Pointing right: this entry fans out into a child menu. +const MENU_CARET_RIGHT: &str = "\u{25B8}"; +/// Pointing down: the closed picker, as a pick list draws it. +const MENU_CARET_DOWN: &str = "\u{25BE}"; +/// Wide enough for the long capture names packs use without wrapping every row. +const MENU_WIDTH: f32 = 380.0; + +type MenuItem<'a> = Item<'a, Message, iced::Theme, iced::Renderer>; +type ModelMenu<'a> = Menu<'a, Message, iced::Theme, iced::Renderer>; + +/// A child menu, sized and spaced the same at every depth. +fn model_menu(items: Vec>) -> ModelMenu<'_> { + Menu::new(items) + .width(MENU_WIDTH) + .offset(PADDING_SMALL) + .spacing(2.0) +} + +/// One selectable row. The selected model is marked in the warning colour rather +/// than a check mark, which would shift every other label over by a column. +fn menu_entry<'a>(label: String, selected: bool, on_press: Message) -> Element<'a, Message> { + button(text(label).style(move |theme: &iced::Theme| iced::widget::text::Style { + color: Some(if selected { COLOR_WARNING } else { theme.palette().text }), + })) + .width(Length::Fill) + .padding([PADDING_SMALL, PADDING_NORMAL]) + .style(menu_row_style) + .on_press(on_press) + .into() +} + +/// A folder row. It has no press action: hovering opens the child menu, and the +/// menu bar draws the highlight for the open path itself. +fn menu_folder<'a>(label: String) -> Element<'a, Message> { + button( + row![text(label).width(Length::Fill), text(MENU_CARET_RIGHT)].spacing(SPACING_TIGHT), + ) + .width(Length::Fill) + .padding([PADDING_SMALL, PADDING_NORMAL]) + .style(menu_row_style) + .into() +} + +/// Flat and transparent at rest, so a column of rows reads as one list rather than +/// a stack of buttons. Disabled is the folder rows' resting state, not an error. +fn menu_row_style(theme: &iced::Theme, status: button::Status) -> button::Style { + let palette = theme.extended_palette(); + let base = button::Style { + text_color: palette.background.base.text, + border: Border::default().rounded(BORDER_RADIUS_CARD), + ..button::Style::default() + }; + match status { + button::Status::Active | button::Status::Disabled => { + base.with_background(Color::TRANSPARENT) + } + button::Status::Hovered => base.with_background(palette.background.strong.color), + button::Status::Pressed => base.with_background(palette.primary.weak.color), + } +} + +/// Menu items for one level of the tree, recursing into folders. +fn menu_items<'a>( + nodes: &[ModelNode], + idx: usize, + selected: Option<&str>, +) -> Vec> { + nodes + .iter() + .map(|node| match node { + ModelNode::Folder { label, children } => Item::with_menu( + menu_folder(label.clone()), + model_menu(menu_items(children, idx, selected)), + ), + ModelNode::Model { label, key } => Item::new(menu_entry( + label.clone(), + selected == Some(key.as_str()), + Message::Stage( + idx, + StageMessage::Nam(NamMessage::ModelSelected(Some(key.clone()))), + ), + )), + }) + .collect() } // --- Apply --- @@ -76,6 +216,7 @@ pub fn apply(cfg: &mut NamConfig, msg: NamMessage) -> Option { } NamMessage::Rescan => Some(ParamUpdate::RescanNamModels), NamMessage::OpenFolder => Some(ParamUpdate::OpenNamModelsDir), + NamMessage::MenuOpened => None, } } @@ -97,23 +238,59 @@ pub fn view(idx: usize, cfg: &NamConfig, state: StageViewState) -> Element<'_, M let ir_bypassed = state.ir_bypassed; stage_card(tr!(stage_nam), idx, state, move || { - // "(None)" first so a selected model can be cleared back to passthrough. - let mut choices = vec![NamModelChoice::None]; - choices.extend(registry::available_names().into_iter().map(NamModelChoice::Model)); - let selected = model_name - .clone() - .map_or(NamModelChoice::None, NamModelChoice::Model); + // The picker is a one-root menu bar rather than a pick list: a pick list is a + // flat list, and packs ship as folders of dozens of files. Each folder fans + // out sideways; loose files sit alongside the folders at the same level. + // + // The bar lays its root out against an unbounded width, so a `Fill` button + // inside it never gets a size. `responsive` hands over the width the row + // actually allotted, and the button takes exactly that. + let tree = model_tree(®istry::available_names()); + let picker_model = model_name.clone(); + let closed_label = model_name + .as_deref() + .map_or_else(|| tr!(nam_no_model).to_owned(), |key| leaf_label(key).to_owned()); + let model_picker = responsive(move |size| { + // "(None)" first so a selected model can be cleared back to passthrough. + let mut items = vec![Item::new(menu_entry( + tr!(nam_no_model).to_owned(), + picker_model.is_none(), + Message::Stage(idx, StageMessage::Nam(NamMessage::ModelSelected(None))), + ))]; + items.extend(menu_items(&tree, idx, picker_model.as_deref())); + + MenuBar::new(vec![Item::with_menu( + button( + row![ + text(closed_label.clone()).width(Length::Fill), + text(MENU_CARET_DOWN), + ] + .spacing(SPACING_TIGHT), + ) + .width(size.width) + .style(button::secondary) + // Opening is handled by the menu bar; the press itself does nothing, + // but a button without `on_press` renders as disabled. + .on_press(Message::Stage(idx, StageMessage::Nam(NamMessage::MenuOpened))), + // As wide as the button, so it hangs straight below it like a pick + // list. Left free, the bar would right-align it whenever the button + // sits in the right half of the window. + model_menu(items).width(size.width), + )]) + // Behave like a pick list: stay open until something is clicked. + .close_on_item_click_global(true) + .close_on_background_click_global(true) + .safe_bounds_margin(f32::MAX) + // Highlights the folder whose child menu is open. + .draw_path(menu::DrawPath::Backdrop) + .into() + }) + .width(Length::FillPortion(7)) + .height(Length::Shrink); let model_selector = row![ text(tr!(nam_model)).width(Length::FillPortion(3)), - pick_list(choices, Some(selected), move |choice| { - Message::Stage( - idx, - StageMessage::Nam(NamMessage::ModelSelected(choice.into_option())), - ) - }) - .placeholder(tr!(nam_no_model)) - .width(Length::FillPortion(7)), + model_picker, ] .spacing(SPACING_NORMAL) .align_y(Alignment::Center); @@ -328,3 +505,57 @@ pub fn view(idx: usize, cfg: &NamConfig, state: StageViewState) -> Element<'_, M .into() }) } + +#[cfg(test)] +mod tests { + use super::*; + + fn folder(node: &ModelNode) -> (&str, &[ModelNode]) { + match node { + ModelNode::Folder { label, children } => (label.as_str(), children.as_slice()), + ModelNode::Model { .. } => panic!("expected a folder, got {node:?}"), + } + } + + fn model(node: &ModelNode) -> (&str, &str) { + match node { + ModelNode::Model { label, key } => (label.as_str(), key.as_str()), + ModelNode::Folder { .. } => panic!("expected a model, got {node:?}"), + } + } + + /// Registry keys are `Folder/Sub/Model`. The menu groups them by folder, keeps + /// the full key on each leaf (that is what gets selected), and sorts folders + /// and loose files together by name, ignoring case, at every level. + #[test] + fn model_tree_groups_keys_by_folder() { + let names = [ + "Pack/Deep/leaf", + "Pack/inner", + "alpha", + "Zeta/one", + "beta", + ] + .map(str::to_owned); + + let tree = model_tree(&names); + + let labels: Vec<&str> = tree.iter().map(ModelNode::label).collect(); + assert_eq!(labels, ["alpha", "beta", "Pack", "Zeta"]); + assert_eq!(model(&tree[0]), ("alpha", "alpha")); + + let (_, pack) = folder(&tree[2]); + let (deep_label, deep) = folder(&pack[0]); + assert_eq!(deep_label, "Deep"); + assert_eq!(model(&deep[0]), ("leaf", "Pack/Deep/leaf")); + assert_eq!(model(&pack[1]), ("inner", "Pack/inner")); + } + + /// What the closed picker shows: only the file's own name, since the folder is + /// visible in the menu and the full key would not fit the card. + #[test] + fn leaf_label_drops_the_folder_prefix() { + assert_eq!(leaf_label("Pack/Deep/leaf"), "leaf"); + assert_eq!(leaf_label("alpha"), "alpha"); + } +} From 2e3ae3946a46ec04dafd30eac7730f9e7c8e2a42 Mon Sep 17 00:00:00 2001 From: OpenSauce Date: Fri, 16 Oct 2026 19:59:01 +0100 Subject: [PATCH 2/2] fix(nam): keep folder rows from closing the picker, skip symlinked and hidden folders Review findings on the folder picker: - Clicking a folder row collapsed the whole menu. `Item::with_menu` inherits the bar's close-on-item-click, and people do click a folder to open it. Folder rows now opt out; only picking a model or clicking away closes. - The recursive scan followed symlinks, so a link pointing back up the tree was walked until the OS refused, parsing every model dozens of times. The walk now checks the entry's own file type, which does not follow links, and skips symlinked directories. - Dot-folders left by sync tools and desktop trash (`.stfolder`, `.Trash-1000`) were scanned, so a trashed model came back as a picker entry. They are skipped. - A file whose path is not valid UTF-8 was dropped silently; it now warns like every other skipped file. - The tree test also pins that a folder and a loose file sharing a name both survive side by side. Co-Authored-By: Claude Fable 5.1 --- rustortion-core/src/nam/loader.rs | 67 ++++++++++++++++++++++++++++--- rustortion-ui/src/stages/nam.rs | 15 ++++++- 2 files changed, 75 insertions(+), 7 deletions(-) diff --git a/rustortion-core/src/nam/loader.rs b/rustortion-core/src/nam/loader.rs index 6481b05..651d749 100644 --- a/rustortion-core/src/nam/loader.rs +++ b/rustortion-core/src/nam/loader.rs @@ -68,7 +68,21 @@ impl NamLoader { } }; let path = entry.path(); - if path.is_dir() { + // `file_type` does not follow symlinks, so a link to a directory is + // neither a directory nor a file here and is skipped: a link pointing + // back up the tree would otherwise be walked until the OS refuses, + // parsing every model dozens of times. + let is_dir = entry.file_type().is_ok_and(|t| t.is_dir()); + if is_dir { + // Sync tools and desktop trash keep state in dot-folders + // (`.stfolder`, `.Trash-1000`); nothing in them is the collection. + let hidden = entry + .file_name() + .to_str() + .is_some_and(|n| n.starts_with('.')); + if hidden { + continue; + } if let Err(e) = self.scan_dir(&path, base) { warn!("Skipping NAM subfolder '{}': {e}", path.display()); } @@ -79,10 +93,14 @@ impl NamLoader { } // The cab-from-name guess reads the file's own name: a folder called // "Full Rig" says nothing about the files inside it. - let Some(stem) = path.file_stem().and_then(|s| s.to_str()) else { - continue; - }; - let Some(name) = Self::model_key(&path, base) else { + let (Some(stem), Some(name)) = ( + path.file_stem().and_then(|s| s.to_str()), + Self::model_key(&path, base), + ) else { + warn!( + "Skipping NAM file '{}': path is not valid UTF-8", + path.display() + ); continue; }; @@ -261,4 +279,43 @@ mod tests { assert_eq!(rig.includes_cab, Some(true), "file name is evidence"); assert!(rig.cab_from_name); } + /// Sync tools and desktop trash leave dot-folders (`.stfolder`, `.Trash-1000`) + /// inside the nam folder. Models in them are not the user's collection: a + /// trashed file must not come back as a picker entry. + #[test] + fn hidden_directories_are_skipped() { + let tmp = tempfile::tempdir().expect("tempdir"); + let dir = tmp.path(); + let fixture = + Path::new(env!("CARGO_MANIFEST_DIR")).join("tests/fixtures/reference_standard.nam"); + + std::fs::create_dir_all(dir.join(".Trash-1000")).expect("mkdir"); + std::fs::copy(&fixture, dir.join(".Trash-1000/deleted.nam")).expect("copy"); + std::fs::copy(&fixture, dir.join("kept.nam")).expect("copy"); + + let loader = NamLoader::new(dir).expect("scan"); + + assert_eq!(loader.available_names(), vec!["kept"]); + } + + /// A symlink pointing back up the tree would otherwise be followed until the + /// kernel refuses, parsing every model dozens of times into memory. Symlinked + /// directories are skipped outright: the models they point at are reachable + /// through their real path when they live inside the nam folder. + #[cfg(unix)] + #[test] + fn symlinked_directories_are_not_followed() { + let tmp = tempfile::tempdir().expect("tempdir"); + let dir = tmp.path(); + let fixture = + Path::new(env!("CARGO_MANIFEST_DIR")).join("tests/fixtures/reference_standard.nam"); + + std::fs::create_dir_all(dir.join("Pack")).expect("mkdir"); + std::fs::copy(&fixture, dir.join("Pack/inner.nam")).expect("copy"); + std::os::unix::fs::symlink(dir, dir.join("Pack/loop")).expect("symlink"); + + let loader = NamLoader::new(dir).expect("scan"); + + assert_eq!(loader.available_names(), vec!["Pack/inner"]); + } } diff --git a/rustortion-ui/src/stages/nam.rs b/rustortion-ui/src/stages/nam.rs index 7c629f6..ea5d145 100644 --- a/rustortion-ui/src/stages/nam.rs +++ b/rustortion-ui/src/stages/nam.rs @@ -180,7 +180,10 @@ fn menu_items<'a>( ModelNode::Folder { label, children } => Item::with_menu( menu_folder(label.clone()), model_menu(menu_items(children, idx, selected)), - ), + ) + // The bar closes on any item click by default, and people do click a + // folder to open it. Only picking a model, or clicking away, closes. + .close_on_click(false), ModelNode::Model { label, key } => Item::new(menu_entry( label.clone(), selected == Some(key.as_str()), @@ -280,6 +283,10 @@ pub fn view(idx: usize, cfg: &NamConfig, state: StageViewState) -> Element<'_, M // Behave like a pick list: stay open until something is clicked. .close_on_item_click_global(true) .close_on_background_click_global(true) + // Leaving the menu must not close it either. The bar pads its bounds + // by this margin and tests containment, and `width + 2 * MAX` + // overflows to infinity, so the cursor is never outside. A large + // finite value would depend on the monitor size. .safe_bounds_margin(f32::MAX) // Highlights the folder whose child menu is open. .draw_path(menu::DrawPath::Backdrop) @@ -529,22 +536,26 @@ mod tests { /// and loose files together by name, ignoring case, at every level. #[test] fn model_tree_groups_keys_by_folder() { + // `Pack` is both a folder and a loose file: both must survive, side by side. let names = [ "Pack/Deep/leaf", "Pack/inner", "alpha", "Zeta/one", "beta", + "Pack", ] .map(str::to_owned); let tree = model_tree(&names); let labels: Vec<&str> = tree.iter().map(ModelNode::label).collect(); - assert_eq!(labels, ["alpha", "beta", "Pack", "Zeta"]); + assert_eq!(labels, ["alpha", "beta", "Pack", "Pack", "Zeta"]); assert_eq!(model(&tree[0]), ("alpha", "alpha")); + assert_eq!(model(&tree[3]), ("Pack", "Pack")); let (_, pack) = folder(&tree[2]); + assert_eq!(pack.len(), 2); let (deep_label, deep) = folder(&pack[0]); assert_eq!(deep_label, "Deep"); assert_eq!(model(&deep[0]), ("leaf", "Pack/Deep/leaf"));