diff --git a/README.md b/README.md index 4931333..111893e 100644 --- a/README.md +++ b/README.md @@ -9,10 +9,11 @@ native file/folder loading and drag-and-drop, local filename-based season/episod detection ("Match") is working, real online metadata lookups against TMDB ("Fetch Data") are working — including fuzzy show-name disambiguation, so a search that returns several similarly-named shows doesn't just take TMDB's top-ranked hit — and the -actual on-disk rename operation is wired up (same-directory rename, skip-on-conflict, -with a confirmation step showing the full old-name/new-name plan before anything -touches disk — there is no undo yet). Full media-metadata bindings are not yet ported. -See "Porting roadmap" below. +actual on-disk rename operation is wired up (skip-on-conflict, with a confirmation step +showing the full old-name/new-name plan before anything touches disk — there is no undo +yet). Rename can also organize files into subfolders — including ones that don't exist +yet — when the naming scheme includes a `/`. Full media-metadata bindings are not yet +ported. See "Porting roadmap" below. The naming scheme is fully user-editable from the app (the "Naming Scheme" button next to the New Names list) — the default is `{show name} - {season number}x{episode @@ -78,6 +79,28 @@ An undefined binding contributes an empty string rather than failing outright (s `{n}{missing}` still renders); the overall expression only errors if the *total* rendered output would be empty. +`/` between two `{ }` scriptlets (or anywhere in the literal text) needs no special +handling in the expression engine at all — it's just ordinary literal text, the same as +a space or a dash. It only becomes meaningful one layer up, at rename time (see +"Organizing into subfolders" below): a scheme like `{show name}/{episode name}` renders +to a name containing a real `/`, and the rename engine treats that as "put this file in +a subfolder" rather than rejecting it. + +### Organizing into subfolders + +A naming scheme that includes `/` (e.g. `{show name}/Season {season number}/{episode +number} - {episode name}`) moves the file into that subfolder as part of renaming, +creating any missing intermediate folders automatically +(`crates/renamr-core/src/rename/mod.rs::rename_one` → `fs::create_dir_all`). This is +still always scoped to the file's own directory tree — the target is resolved relative +to the source file's parent directory, never as an absolute path, and every path +component is validated (`validate_relative_target`) to reject empty segments (a +leading, trailing, or doubled `/` — which also catches an absolute-looking +`/etc/passwd`-style path), `.`/`..` segments, and backslashes (Renamr always uses `/` +for the naming scheme's folder separator regardless of OS). Conflict policy is +unchanged: if the resolved target already exists and isn't the same underlying file, +that item is skipped rather than overwritten. + ## Project layout ``` @@ -87,10 +110,11 @@ crates/renamr-core/ Pure Rust core: naming-expression engine (ported), local (`similarity/`, partial `EpisodeMetrics` port), a TMDB metadata client (`web/tmdb.rs` — TV search + episode lookup, disambiguated via `similarity/`, tested against a mocked HTTP server via - `mockito`), and the on-disk rename engine (ported for the - same-directory case, `rename/`) — skip-on-conflict, no - move/organize-into-folders yet. Not yet ported: full media - metadata bindings, other web datasource clients (TVDB/AniDB/etc). + `mockito`), and the on-disk rename engine (`rename/`) — + skip-on-conflict, can also organize into (auto-created) + subfolders when the naming scheme includes `/`. Not yet ported: + full media metadata bindings, other web datasource clients + (TVDB/AniDB/etc). No Tauri dependency, so it builds and unit-tests standalone. src-tauri/ Tauri app shell: exposes renamr-core as commands to the frontend (naming-scheme rendering, filename matching, TMDB metadata fetch, @@ -184,13 +208,14 @@ ported from, for anyone picking this up: OMDb, TVmaze, OpenSubtitles) are not yet started. 6. **Rename engine + CLI** — `net.filebot.cli.CmdlineOperations` (1235 lines) and `ArgumentBean` (415 lines, the full flag surface: `-rename`, `--db`, `--format`, - `--action`, `--conflict`, `-get-subtitles`, `-script`, etc.). The core same-directory - rename operation is done (`crates/renamr-core/src/rename`, fixed skip-on-conflict - policy, wired to the UI's Rename button behind a confirmation step). Not yet done: - the `--action` variants beyond rename-in-place (move/copy/symlink to a different - folder, which is what lets a naming scheme like `{n}/{s00e00}` organize files into - subfolders), the `--conflict` variants beyond skip (auto-number, override), and the - CLI flag surface itself. + `--action`, `--conflict`, `-get-subtitles`, `-script`, etc.). The rename operation is + done (`crates/renamr-core/src/rename`, fixed skip-on-conflict policy, wired to the + UI's Rename button behind a confirmation step) including the `move`-into-subfolder + `--action` variant — a naming scheme like `{n}/{s00e00}` organizes files into + subfolders, creating any missing folders automatically, always scoped to the file's + own directory tree. Not yet done: the `copy`/`symlink` `--action` variants, the + `--conflict` variants beyond skip (auto-number, override), and the CLI flag surface + itself. 7. **The rest of the UI** — the Rename panel (file loading, drag-drop, live preview, local matching, confirmed rename) is built; the other five sidebar panels (Episodes/Subtitles/SFV/Filter/List) are still stub screens, plus Align up/down diff --git a/crates/renamr-core/src/rename/mod.rs b/crates/renamr-core/src/rename/mod.rs index 6dadb42..cc6faf5 100644 --- a/crates/renamr-core/src/rename/mod.rs +++ b/crates/renamr-core/src/rename/mod.rs @@ -1,20 +1,30 @@ -//! The on-disk rename operation: given a planned `old path -> new filename` mapping, -//! actually renames the files. +//! The on-disk rename operation: given a planned `old path -> new relative target` +//! mapping, actually renames (and, when the target includes subfolders, moves) the +//! files. //! -//! Deliberately scoped for v1: same-directory renames only (no moving files into a -//! different folder tree/organizing by naming scheme yet - that's a bigger feature with -//! its own directory-creation and cross-filesystem-move concerns). Conflict policy is -//! fixed at **skip**: if the computed target filename already exists and isn't the same -//! underlying file, that item is left alone and reported as skipped rather than -//! overwritten. There is no undo yet, so the UI is expected to show the caller a -//! confirmation of the full plan before calling [`execute_renames`]. +//! Every rename stays rooted at the source file's own directory - `new_name` is always +//! resolved relative to `source`'s parent, never as an absolute path, and can never +//! escape that directory tree via `..`. Within that directory tree, though, `new_name` +//! may contain `/` to organize files into subfolders as part of renaming (e.g. a naming +//! scheme like `{show name}/{episode name}` - the naming-expression engine already +//! treats `/` between two `{ }` scriptlets as ordinary literal text, so no changes were +//! needed there; this is purely about the rename engine trusting a `/` in the computed +//! name to mean "put it in a subfolder" instead of rejecting it outright). Any missing +//! intermediate directories are created automatically. Conflict policy is fixed at +//! **skip**: if the computed target already exists and isn't the same underlying file, +//! that item is left alone and reported as skipped rather than overwritten. There is no +//! undo yet, so the UI is expected to show the caller a confirmation of the full plan +//! before calling [`execute_renames`]. use serde::{Deserialize, Serialize}; use std::fs; use std::path::Path; /// One planned rename: `source` is the file's current full path, `new_name` is the -/// filename (including extension) it should have afterward, in the same directory. +/// target it should have afterward, resolved relative to `source`'s own directory - +/// just a filename for a same-directory rename, or a `/`-separated relative path (e.g. +/// `"Breaking Bad/Ozymandias.mkv"`) to also move the file into a (possibly new) +/// subfolder. #[derive(Debug, Clone, Deserialize)] #[serde(rename_all = "camelCase")] pub struct RenamePlanItem { @@ -42,16 +52,32 @@ pub struct RenameResult { pub outcome: RenameOutcome, } -/// Rejects filenames that could escape the source file's own directory (path -/// separators, `.`/`..`) or are empty. The rename engine only ever renames a file -/// within its existing directory, so a `new_name` that isn't a plain filename is a bug -/// upstream (or a hostile input) rather than something to interpret as a move. -fn is_plain_filename(name: &str) -> bool { - !name.is_empty() - && name != "." - && name != ".." - && !name.contains('/') - && !name.contains('\\') +/// Validates a computed target, relative to the source file's own directory. `/` is +/// allowed as a subdirectory separator - that's what lets a naming scheme like +/// `{show name}/{episode name}` organize files into subfolders - but every path +/// component is checked to make sure the target can never escape that directory tree: +/// no empty components (a leading, trailing, or doubled `/` - which also catches an +/// absolute-looking `/etc/passwd`-style path, since splitting it on `/` yields a +/// leading empty component), and no `.`/`..` components. Backslashes are rejected +/// outright rather than treated as an alternate separator - Renamr always uses `/` for +/// the naming scheme's folder separator regardless of OS, so a literal `\` in a +/// computed name is almost certainly a mistake, not an intentional Windows path. +fn validate_relative_target(name: &str) -> Result<(), &'static str> { + if name.is_empty() { + return Err("empty"); + } + if name.contains('\\') { + return Err("backslashes aren't allowed - use / to organize into subfolders"); + } + for component in name.split('/') { + if component.is_empty() { + return Err("contains an empty path segment (e.g. a leading, trailing, or doubled '/')"); + } + if component == "." || component == ".." { + return Err("contains a '.' or '..' path segment"); + } + } + Ok(()) } /// True if `a` and `b` refer to the same underlying file (used to allow case-only @@ -65,25 +91,12 @@ fn same_file(a: &Path, b: &Path) -> bool { } fn rename_one(source: &Path, new_name: &str) -> RenameOutcome { - if !is_plain_filename(new_name) { + if let Err(reason) = validate_relative_target(new_name) { return RenameOutcome::Failed { - error: format!("\"{new_name}\" is not a valid filename"), + error: format!("\"{new_name}\" is not a valid target: {reason}"), }; } - let current_name = match source.file_name().and_then(|n| n.to_str()) { - Some(n) => n, - None => { - return RenameOutcome::Failed { - error: "source has no filename".to_string(), - } - } - }; - - if current_name == new_name { - return RenameOutcome::Unchanged; - } - let parent = match source.parent() { Some(p) => p, None => { @@ -94,12 +107,27 @@ fn rename_one(source: &Path, new_name: &str) -> RenameOutcome { }; let target = parent.join(new_name); + if target == source { + return RenameOutcome::Unchanged; + } + if target.exists() && !same_file(&target, source) { return RenameOutcome::SkippedConflict { existing_path: target.to_string_lossy().to_string(), }; } + // `new_name` may include subdirectory components (organizing files into folders) - + // make sure the destination directory tree exists before attempting the actual + // rename/move. `create_dir_all` is a no-op (Ok) when the directory already exists. + if let Some(target_parent) = target.parent() { + if let Err(e) = fs::create_dir_all(target_parent) { + return RenameOutcome::Failed { + error: format!("couldn't create destination folder: {e}"), + }; + } + } + match fs::rename(source, &target) { Ok(()) => RenameOutcome::Renamed { new_path: target.to_string_lossy().to_string(), @@ -246,4 +274,104 @@ mod tests { fs::remove_dir_all(&dir).ok(); } + + #[test] + fn organizes_into_a_new_subfolder() { + let (dir, file) = temp_file_setup("Breaking.Bad.S05E14.mkv", "video"); + let items = vec![RenamePlanItem { + source: file.to_string_lossy().to_string(), + new_name: "Breaking Bad/Ozymandias.mkv".to_string(), + }]; + + let results = execute_renames(&items); + match &results[0].outcome { + RenameOutcome::Renamed { new_path } => { + let expected = dir.join("Breaking Bad").join("Ozymandias.mkv"); + assert_eq!(Path::new(new_path), expected.as_path()); + assert!(expected.exists()); + assert!(!file.exists()); + } + other => panic!("expected Renamed, got {other:?}"), + } + + fs::remove_dir_all(&dir).ok(); + } + + #[test] + fn organizes_into_nested_subfolders_that_dont_exist_yet() { + let (dir, file) = temp_file_setup("ep.mkv", "video"); + let items = vec![RenamePlanItem { + source: file.to_string_lossy().to_string(), + new_name: "Breaking Bad/Season 05/S05E14.mkv".to_string(), + }]; + + let results = execute_renames(&items); + let expected = dir.join("Breaking Bad").join("Season 05").join("S05E14.mkv"); + match &results[0].outcome { + RenameOutcome::Renamed { new_path } => { + assert_eq!(Path::new(new_path), expected.as_path()); + assert!(expected.exists()); + } + other => panic!("expected Renamed, got {other:?}"), + } + + fs::remove_dir_all(&dir).ok(); + } + + #[test] + fn skips_when_the_organized_target_already_exists() { + let (dir, file) = temp_file_setup("a.txt", "aaa"); + fs::create_dir_all(dir.join("Sub")).unwrap(); + fs::write(dir.join("Sub").join("b.txt"), "bbb").unwrap(); + + let items = vec![RenamePlanItem { + source: file.to_string_lossy().to_string(), + new_name: "Sub/b.txt".to_string(), + }]; + + let results = execute_renames(&items); + assert!(matches!( + results[0].outcome, + RenameOutcome::SkippedConflict { .. } + )); + assert!(file.exists()); + assert_eq!(fs::read_to_string(dir.join("Sub").join("b.txt")).unwrap(), "bbb"); + + fs::remove_dir_all(&dir).ok(); + } + + #[test] + fn rejects_backslashes_in_new_name() { + let (dir, file) = temp_file_setup("a.txt", "aaa"); + let items = vec![RenamePlanItem { + source: file.to_string_lossy().to_string(), + new_name: "Sub\\b.txt".to_string(), + }]; + + let results = execute_renames(&items); + assert!(matches!(results[0].outcome, RenameOutcome::Failed { .. })); + assert!(file.exists()); + + fs::remove_dir_all(&dir).ok(); + } + + #[test] + fn rejects_doubled_and_leading_slashes() { + let (dir, file) = temp_file_setup("a.txt", "aaa"); + + for bad_name in ["Sub//b.txt", "/etc/passwd", "Sub/b.txt/"] { + let items = vec![RenamePlanItem { + source: file.to_string_lossy().to_string(), + new_name: bad_name.to_string(), + }]; + let results = execute_renames(&items); + assert!( + matches!(results[0].outcome, RenameOutcome::Failed { .. }), + "expected {bad_name:?} to be rejected" + ); + } + assert!(file.exists()); + + fs::remove_dir_all(&dir).ok(); + } } diff --git a/src/components/RenamePanel.tsx b/src/components/RenamePanel.tsx index 38e6e4a..d51b5ae 100644 --- a/src/components/RenamePanel.tsx +++ b/src/components/RenamePanel.tsx @@ -810,6 +810,11 @@ export default function RenamePanel() { set in TMDB Settings first. FileBot's own short field names ({"{n}"},{" "} {"{s00e00}"}, {"{t}"}, etc.) still work too, side by side with the names above.

+

+ Use {"/"} in the scheme to organize files into subfolders as part of renaming, e.g.{" "} + {"{show name}/{episode name}"} puts each show in its own folder. Missing folders are created + automatically; nothing outside the file's own folder tree is ever touched. +

)} @@ -824,6 +829,8 @@ export default function RenamePanel() { There's no undo yet — review the list below before continuing. {renamePlanSkipped > 0 && ` (${renamePlanSkipped} other file${renamePlanSkipped === 1 ? "" : "s"} excluded: no valid preview.)`} + {renamePlan.some((p) => p.newName.includes("/")) && + " Some of these also move the file into a new subfolder — any missing folders are created automatically."}

{renamePlan.map((p) => (