From 02177af2b3cd8d94f6df2fc3af18cf08f7fffff7 Mon Sep 17 00:00:00 2001 From: Thijs Herman <64640608+Turbootzz@users.noreply.github.com> Date: Fri, 18 Sep 2026 18:18:54 +0200 Subject: [PATCH 1/2] Let people name and star their clips Every card in the library was headed by the time it was saved, which makes one clip look like the next. A clip can now carry a name and a star: the name heads its card and its detail page (click either the title or the Rename button to change it), the star sits in the corner of every thumbnail, and a Favourites chip narrows the grid to the starred ones. Search looks through names too. The name is metadata, not the file name. Renaming the file would break the rewynd-*.mp4 filter that makes a file a clip, the stamp that dates it, the rewynd://clip/ link a save toast opens, and the upload history's identity, and Windows holds the file open while the player has it. Both live in clip-meta.json beside config.toml, keyed by file name so they survive a move between game folders and a trim in place, written 0600 under the same lock and atomic writer as the upload history (now shared, in paths.rs). Edits show up at once and are written behind the scenes, one write per clip at a time so two quick stars cannot land out of order, and a rescan arriving mid-write keeps what is on screen. Deleting a clip clears its entry through that same queue. A trimmed copy inherits " (trimmed)", and the upload title starts from the clip's name. Details in docs/adr/0024. --- README.md | 2 +- crates/config/src/clip_meta.rs | 349 ++++++++++ crates/config/src/lib.rs | 2 + crates/config/src/paths.rs | 26 + crates/config/src/upload_history.rs | 27 +- .../settings/assets/star/star-filled-48.png | Bin 0 -> 983 bytes .../settings/assets/star/star-outline-48.png | Bin 0 -> 1560 bytes crates/settings/src/library.rs | 650 ++++++++++++++++-- crates/settings/src/theme.rs | 24 + docs/adr/0024-clip-names-and-favourites.md | 63 ++ 10 files changed, 1059 insertions(+), 84 deletions(-) create mode 100644 crates/config/src/clip_meta.rs create mode 100644 crates/settings/assets/star/star-filled-48.png create mode 100644 crates/settings/assets/star/star-outline-48.png create mode 100644 docs/adr/0024-clip-names-and-favourites.md diff --git a/README.md b/README.md index 0025a1c..b7075b6 100644 --- a/README.md +++ b/README.md @@ -65,7 +65,7 @@ The app checks for updates on launch; one click updates both binaries in place. ## What you get - A background recorder that sits in the tray and stays out of the way while you play. -- A clip library and settings app: browse, trim, and manage your clips. +- A clip library and settings app: browse, name, star, trim, and manage your clips. - A first-run wizard that sets up capture, the hotkey, and the replay length in a minute. - Optional uploads to [ganked.tv](https://ganked.tv) or YouTube, only when you ask. diff --git a/crates/config/src/clip_meta.rs b/crates/config/src/clip_meta.rs new file mode 100644 index 0000000..eee7630 --- /dev/null +++ b/crates/config/src/clip_meta.rs @@ -0,0 +1,349 @@ +//! What the user has told us about a clip: the name they gave it and whether they starred it. +//! A small JSON file beside `config.toml` (see docs/adr/0024), written 0600 like the upload +//! history, because a clip name can quote whatever was said in the recording. +//! +//! Keyed by file name alone, not by size or mtime: a clip's name has to survive moving between +//! game folders and a trim that rewrites the file in place. File names carry a millisecond +//! stamp plus a per-process sequence, so they are unique within the store. + +use std::collections::BTreeMap; +use std::path::{Path, PathBuf}; + +use serde::{Deserialize, Serialize}; + +/// How long a clip name may be. Long enough for a sentence, short enough that a card's title +/// line stays one line. +pub const MAX_NAME_CHARS: usize = 80; + +/// What we remember about one clip. An entry equal to this default is dropped from the store +/// rather than written out. +#[derive(Debug, Clone, Default, PartialEq, Eq, Serialize, Deserialize)] +#[serde(default)] +pub struct ClipMeta { + #[serde(skip_serializing_if = "Option::is_none")] + pub name: Option, + #[serde(skip_serializing_if = "is_false")] + pub favourite: bool, +} + +impl ClipMeta { + fn is_empty(&self) -> bool { + *self == Self::default() + } +} + +fn is_false(b: &bool) -> bool { + !*b +} + +/// Every clip we know something about, keyed by file name. A `BTreeMap` so the file stays in a +/// stable order instead of reshuffling on every write. +#[derive(Debug, Clone, Default, PartialEq, Eq, Serialize, Deserialize)] +#[serde(transparent)] +pub struct ClipMetaStore(BTreeMap); + +impl ClipMetaStore { + /// What we know about the clip at `path`, if anything. + #[must_use] + pub fn get(&self, path: &Path) -> Option<&ClipMeta> { + self.0.get(file_name_of(path)?) + } + + /// The name the user gave this clip, if they gave it one. + #[must_use] + pub fn name_of(&self, path: &Path) -> Option<&str> { + self.get(path)?.name.as_deref() + } + + /// Whether this clip is starred. + #[must_use] + pub fn is_favourite(&self, path: &Path) -> bool { + self.get(path).is_some_and(|m| m.favourite) + } + + /// Apply `edit` to this clip's entry, creating it if needed. An entry left at its default + /// (no name, not starred) is removed, so clearing a name does not leave a husk behind. + pub fn set(&mut self, file_name: &str, edit: impl FnOnce(&mut ClipMeta)) { + let mut meta = self.0.get(file_name).cloned().unwrap_or_default(); + edit(&mut meta); + if meta.is_empty() { + self.0.remove(file_name); + } else { + self.0.insert(file_name.to_owned(), meta); + } + } + + /// This clip's entry by store key, or a default one when it has none. + #[must_use] + pub fn entry_of(&self, file_name: &str) -> ClipMeta { + self.0.get(file_name).cloned().unwrap_or_default() + } + + /// Whether the store holds nothing at all. + #[must_use] + pub fn is_empty(&self) -> bool { + self.0.is_empty() + } +} + +/// The store's key for a clip path: its file name. +#[must_use] +pub fn file_name_of(path: &Path) -> Option<&str> { + path.file_name()?.to_str() +} + +/// Characters that are invisible or reorder what follows them. A clip name is drawn as a card +/// title and a heading, so one of these could hide text or flip it right to left. +fn is_invisible(c: char) -> bool { + c.is_control() + || matches!(c, + '\u{00ad}' // soft hyphen + | '\u{200b}'..='\u{200f}' // zero width spaces, joiners, LTR/RTL marks + | '\u{202a}'..='\u{202e}' // bidi embedding and overrides + | '\u{2060}'..='\u{2064}' // word joiner and invisible operators + | '\u{2066}'..='\u{2069}' // bidi isolates + | '\u{feff}' // byte order mark + ) +} + +/// A clip name as it will be stored: whitespace collapsed, invisible characters dropped, capped +/// at [`MAX_NAME_CHARS`]. `None` when nothing usable is left, which means "no name". +#[must_use] +pub fn clean_name(raw: &str) -> Option { + let mapped: String = raw + .chars() + .map(|c| if is_invisible(c) { ' ' } else { c }) + .collect(); + let mut name = String::with_capacity(mapped.len()); + for word in mapped.split_whitespace() { + if !name.is_empty() { + name.push(' '); + } + name.push_str(word); + } + if name.chars().count() > MAX_NAME_CHARS { + name = name.chars().take(MAX_NAME_CHARS).collect(); + name = name.trim_end().to_owned(); + } + (!name.is_empty()).then_some(name) +} + +/// The name a trimmed copy inherits from its source. Re-cleaned, so a name already at the cap +/// makes room for the suffix instead of overflowing it. +#[must_use] +pub fn trimmed_copy_name(name: &str) -> Option { + const SUFFIX: &str = " (trimmed)"; + let room = MAX_NAME_CHARS.saturating_sub(SUFFIX.chars().count()); + let stem: String = clean_name(name)?.chars().take(room).collect(); + clean_name(&format!("{}{SUFFIX}", stem.trim_end())) +} + +/// The store's path, beside `config.toml`. +#[must_use] +pub fn store_path() -> Option { + Some(crate::config_path()?.with_file_name("clip-meta.json")) +} + +/// Everything we know about every clip (empty when the file is missing or unreadable). +#[must_use] +pub fn load() -> ClipMetaStore { + store_path().map(|p| load_at(&p)).unwrap_or_default() +} + +/// Apply `edit` to one clip's entry and write the store back. The read-modify-write runs under +/// an exclusive file lock, so the settings window and a second instance can't clobber each +/// other's edits. An edit that leaves the entry at its default removes it, which is how a +/// deleted clip is forgotten. +pub fn update(file_name: &str, edit: impl FnOnce(&mut ClipMeta)) -> std::io::Result<()> { + let path = store_path().ok_or_else(no_path)?; + with_store_lock(&path, || { + let mut store = load_at(&path); + store.set(file_name, edit); + save_at(&path, &store) + }) +} + +fn with_store_lock( + path: &Path, + body: impl FnOnce() -> std::io::Result, +) -> std::io::Result { + crate::lock::with_exclusive_lock(&path.with_extension("json.lock"), body) +} + +fn no_path() -> std::io::Error { + std::io::Error::new( + std::io::ErrorKind::NotFound, + "no config directory to store clip names", + ) +} + +fn load_at(path: &Path) -> ClipMetaStore { + std::fs::read(path) + .ok() + .and_then(|bytes| serde_json::from_slice(&bytes).ok()) + .unwrap_or_default() +} + +fn save_at(path: &Path, store: &ClipMetaStore) -> std::io::Result<()> { + let bytes = serde_json::to_vec_pretty(store) + .map_err(|e| std::io::Error::new(std::io::ErrorKind::InvalidData, e))?; + crate::paths::write_private_atomic(path, &bytes) +} + +#[cfg(test)] +mod tests { + use super::*; + + fn named(store: &mut ClipMetaStore, file: &str, name: &str) { + store.set(file, |m| m.name = clean_name(name)); + } + + #[test] + fn set_stores_and_compacts_entries() { + let mut store = ClipMetaStore::default(); + named(&mut store, "rewynd-1-0.mp4", "Clutch ace"); + store.set("rewynd-1-0.mp4", |m| m.favourite = true); + let clip = Path::new("/clips/Elden Ring/rewynd-1-0.mp4"); + assert_eq!(store.name_of(clip), Some("Clutch ace")); + assert!( + store.is_favourite(clip), + "the folder is not part of the key" + ); + + // Clearing both fields drops the entry rather than leaving an empty husk. + store.set("rewynd-1-0.mp4", |m| m.name = None); + assert!(!store.is_empty(), "still starred"); + store.set("rewynd-1-0.mp4", |m| m.favourite = false); + assert!(store.is_empty()); + assert_eq!(store.name_of(clip), None); + assert!(!store.is_favourite(clip)); + } + + #[test] + fn round_trips_through_the_file() { + let dir = tempfile::tempdir().expect("tempdir"); + let path = dir.path().join("clip-meta.json"); + assert!(load_at(&path).is_empty(), "missing file loads empty"); + + let mut store = ClipMetaStore::default(); + named(&mut store, "rewynd-2-0.mp4", "Triple kill"); + store.set("rewynd-3-0.mp4", |m| m.favourite = true); + save_at(&path, &store).expect("save"); + assert_eq!(load_at(&path), store); + + // Only the fields that carry information are written. + let text = std::fs::read_to_string(&path).expect("read"); + assert!(text.contains("Triple kill"), "{text}"); + assert!(!text.contains("\"favourite\": false"), "{text}"); + + #[cfg(unix)] + { + use std::os::unix::fs::PermissionsExt; + let mode = std::fs::metadata(&path).unwrap().permissions().mode(); + assert_eq!(mode & 0o777, 0o600, "clip names are owner-only"); + } + } + + #[test] + fn garbage_loads_empty() { + let dir = tempfile::tempdir().expect("tempdir"); + let bad = dir.path().join("bad.json"); + std::fs::write(&bad, b"not json").expect("write"); + assert!(load_at(&bad).is_empty()); + } + + #[test] + fn with_store_lock_runs_the_body() { + let dir = tempfile::tempdir().expect("tempdir"); + let path = dir.path().join("clip-meta.json"); + let out = with_store_lock(&path, || { + let mut store = ClipMetaStore::default(); + named(&mut store, "rewynd-1-0.mp4", "Locked"); + save_at(&path, &store)?; + Ok::<_, std::io::Error>(7) + }) + .expect("locked body"); + assert_eq!(out, 7); + assert_eq!( + load_at(&path).name_of(Path::new("rewynd-1-0.mp4")), + Some("Locked") + ); + #[cfg(unix)] + assert!(path.with_extension("json.lock").exists()); + } + + #[test] + fn clearing_an_entry_through_update_forgets_the_clip() { + let dir = tempfile::tempdir().expect("tempdir"); + let path = dir.path().join("clip-meta.json"); + let mut store = ClipMetaStore::default(); + named(&mut store, "rewynd-1-0.mp4", "Gone soon"); + named(&mut store, "rewynd-2-0.mp4", "Stays"); + save_at(&path, &store).expect("save"); + + // What the delete handler does: leave the entry at its default. + let mut store = load_at(&path); + store.set("rewynd-1-0.mp4", |m| *m = ClipMeta::default()); + save_at(&path, &store).expect("save"); + + let after = load_at(&path); + assert_eq!(after.name_of(Path::new("rewynd-1-0.mp4")), None); + assert_eq!(after.name_of(Path::new("rewynd-2-0.mp4")), Some("Stays")); + } + + #[test] + fn clean_name_drops_invisible_and_direction_flipping_characters() { + // A name is drawn as a heading, so nothing in it may hide text or reverse it. + let sneaky = "Ace\u{202e}gpj.exe\u{200b} \u{feff}clip"; + let cleaned = clean_name(sneaky).expect("name"); + assert_eq!(cleaned, "Ace gpj.exe clip"); + assert!(!cleaned.chars().any(is_invisible), "{cleaned:?}"); + assert_eq!(clean_name("\u{200b}\u{202e}\u{feff}"), None); + } + + #[test] + fn clean_name_tidies_and_caps() { + assert_eq!( + clean_name(" Clutch ace "), + Some("Clutch ace".to_owned()) + ); + assert_eq!( + clean_name("line\nbreak\tand\u{0}nul"), + Some("line break and nul".to_owned()) + ); + assert_eq!(clean_name(" "), None); + assert_eq!(clean_name(""), None); + + // The cap counts characters, not bytes, so a multibyte name is never cut mid-character. + let long = "é".repeat(MAX_NAME_CHARS + 20); + let capped = clean_name(&long).expect("name"); + assert_eq!(capped.chars().count(), MAX_NAME_CHARS); + // A name capped mid-word does not keep a dangling space. + let words = format!("{} tail", "x".repeat(MAX_NAME_CHARS - 1)); + assert_eq!( + clean_name(&words).expect("name"), + "x".repeat(MAX_NAME_CHARS - 1) + ); + } + + #[test] + fn trimmed_copy_name_fits_under_the_cap() { + assert_eq!( + trimmed_copy_name("Clutch ace"), + Some("Clutch ace (trimmed)".to_owned()) + ); + let long = trimmed_copy_name(&"x".repeat(MAX_NAME_CHARS)).expect("name"); + assert!(long.chars().count() <= MAX_NAME_CHARS, "{long}"); + assert!(long.ends_with(" (trimmed)"), "{long}"); + assert_eq!(trimmed_copy_name(" "), None); + } + + #[test] + fn file_name_of_is_the_key() { + assert_eq!( + file_name_of(Path::new("/clips/Elden Ring/rewynd-5-0.mp4")), + Some("rewynd-5-0.mp4") + ); + assert_eq!(file_name_of(Path::new("/")), None); + } +} diff --git a/crates/config/src/lib.rs b/crates/config/src/lib.rs index 1146a95..f6a6fe3 100644 --- a/crates/config/src/lib.rs +++ b/crates/config/src/lib.rs @@ -21,6 +21,7 @@ //! are called). mod activation; +pub mod clip_meta; mod clips; mod desktop; mod devices; @@ -35,6 +36,7 @@ mod status; pub mod upload_history; pub use activation::{send_settings_activation, take_settings_activation}; +pub use clip_meta::{ClipMeta, ClipMetaStore}; pub use clips::{ CLIP_URL_PREFIX, CLIP_URL_SCHEME, ClipEntry, clip_deeplink, clip_from_deeplink, clip_output_path, clips_dir, ensure_private_dir, folder_name, list_clips, newest_clip_in, diff --git a/crates/config/src/paths.rs b/crates/config/src/paths.rs index c8c6040..041bc8b 100644 --- a/crates/config/src/paths.rs +++ b/crates/config/src/paths.rs @@ -182,6 +182,32 @@ pub(crate) fn write_file_atomic(path: &Path, contents: &[u8]) -> std::io::Result result } +/// Write `bytes` to `path` atomically and owner-only (0600 on unix), creating parent +/// directories. Used for the small JSON stores beside `config.toml` that hold private data: +/// upload history (remote ids and share links) and clip names. +pub(crate) fn write_private_atomic(path: &Path, bytes: &[u8]) -> std::io::Result<()> { + use std::io::Write; + if let Some(parent) = path.parent() { + std::fs::create_dir_all(parent)?; + } + let staged = staged_path(path); + let mut options = std::fs::OpenOptions::new(); + options.write(true).create(true).truncate(true); + #[cfg(unix)] + { + use std::os::unix::fs::OpenOptionsExt; + options.mode(0o600); + } + let result = options + .open(&staged) + .and_then(|mut file| file.write_all(bytes)) + .and_then(|()| std::fs::rename(&staged, path)); + if result.is_err() { + let _ = std::fs::remove_file(&staged); + } + result +} + /// `name` beside `exe`, with the platform's executable suffix. The testable core of /// [`sibling_binary`]. fn sibling_of(exe: &Path, name: &str) -> Option { diff --git a/crates/config/src/upload_history.rs b/crates/config/src/upload_history.rs index 59e5795..20d743a 100644 --- a/crates/config/src/upload_history.rs +++ b/crates/config/src/upload_history.rs @@ -155,32 +155,7 @@ fn load_at(path: &Path) -> Vec { fn save_at(path: &Path, records: &[UploadRecord]) -> std::io::Result<()> { let bytes = serde_json::to_vec_pretty(records) .map_err(|e| std::io::Error::new(std::io::ErrorKind::InvalidData, e))?; - write_private_atomic(path, &bytes) -} - -/// Write `bytes` to `path` atomically (temp + rename), owner-only: a crash can't leave a truncated -/// history, and the remote ids/links never become group/world readable. -fn write_private_atomic(path: &Path, bytes: &[u8]) -> std::io::Result<()> { - use std::io::Write; - if let Some(parent) = path.parent() { - std::fs::create_dir_all(parent)?; - } - let tmp = path.with_extension("json.tmp"); - let mut options = std::fs::OpenOptions::new(); - options.write(true).create(true).truncate(true); - #[cfg(unix)] - { - use std::os::unix::fs::OpenOptionsExt; - options.mode(0o600); - } - let result = options - .open(&tmp) - .and_then(|mut file| file.write_all(bytes)) - .and_then(|()| std::fs::rename(&tmp, path)); - if result.is_err() { - let _ = std::fs::remove_file(&tmp); - } - result + crate::paths::write_private_atomic(path, &bytes) } #[cfg(test)] diff --git a/crates/settings/assets/star/star-filled-48.png b/crates/settings/assets/star/star-filled-48.png new file mode 100644 index 0000000000000000000000000000000000000000..4a984c20e2a2c6c8cce36c2f3ac472bf27c3e2e1 GIT binary patch literal 983 zcmV;|11S87P)klXlo|Z#LPW%pQ$uu z{=8e>Bd;(YJb1i&?{~iMoqOIpZw5N(Ai<~nsLm>{vRE}reB zM=ql5+lsQnx{feBb>r^HKsPV^FW$kOcbn($jtu-Z-_4%d1i@)QIN1wI|=%r$BHZ~ zmnL@hDfGwOASoS183kqnQGQqe&LEViONj?^%h4KeDZa8eMBQYujAHmttdd0n(YuKj zHIx#<8qmI~WBwb`fSe@_@e`oD< zN*Xp*ks#&I?Qg)!Ved7exfze&fcOWAliTelDo%?qDnc!M-B~9!h5g{Nch21Nnse_t=VpOf%%XYWn8s=`a4o_v7|%Fnp2ZOJM(LqO2xUIq zR zGcmm!Ruq&lgcPm5EdbwGd-jN4pFq*8wK)S_MXPV~|C)td_u_Ts@=%7D#MmW_^36N0 z#nbmp#N(K`V*pipBLaGvcn=e2Rkbx6i7fm(R0l+e2hG0D641p!hTvrwl%cAj84%hh z;`4~^)b-@I6TP!6;|_)Sn~4>oA3&CYUIJ$sN|u1^9H4h@yyGkpm9FT3sG_d5@Ew3x zbF~z6>PG8y6*BY2SR#F(_B<*R5Uaf)uM9wy$b%sK&K|NN zv;?J>fvgJ7w#Abov||0V;X5eR3swhAqqAJyo*9PMRoSWvGGK_rFjrzPS8SAR9;?7H zjjaIhlbmqDctA^}zQR?6Q^?;9!e;;?KB)}QHZ7j~aLP0)7BKU}V&D0s2na`uR{s90 zd6Pr6ddtQ?KGygtvry5>uVuyoDTG5r*c8=MhZ|-&fTEeZmqCXBwE3iAX4(=9>ZI$2d9JTG!0oZWr>0#1Qkz))7k%04sbQjd5V0 zkn0r5sLt&1xGiK^7j7yRatA%ocSKkxhL{VQcsI@fZlcN=c8^Lo@2reQNzaY@rW$?QHv*U3*7-`R*IHkOSdE$0|3A4 zQF)K9FTOOD9LqYtKw)(kK+j5jzYI63Cu7qskkfXczmQ*2CWwC<$tZ*If$Dj?+vhJD z*#^^*_s@mSvZCw;;n6WaB0yyD}0NVNNo>lynx`*5~=M% zhL9z_8uoRC52s+Pa?Jc**D_XyE!(w>l|r`A5w`4E%wiU3Ui=HhV1reTrK`9A0000< KMNUMnLSTY9m*+VE literal 0 HcmV?d00001 diff --git a/crates/settings/src/library.rs b/crates/settings/src/library.rs index 27831ce..3427bdf 100644 --- a/crates/settings/src/library.rs +++ b/crates/settings/src/library.rs @@ -12,6 +12,7 @@ use iced::widget::{ }; use iced::{Background, Border, Element, Length, Task, Theme}; +use rewynd_config::clip_meta::{self, ClipMeta, ClipMetaStore}; use rewynd_config::upload_history::{self, ClipKey, UploadRecord}; use rewynd_config::{ClipEntry, Config}; use rewynd_upload::youtube::{ @@ -33,6 +34,9 @@ use crate::thumbs; use crate::trimbar; use crate::video; +/// The name field on the detail page, so committing a rename can hand it the keyboard. +const RENAME_INPUT: &str = "clip-rename"; + /// Cards per grid row (the body column is width-capped, so a fixed count stays balanced). Four /// across suits the wider default window while staying readable if it is narrowed. const GRID_COLUMNS: usize = 4; @@ -213,9 +217,22 @@ const POLL_MAX_READS: u32 = 60; pub enum Message { SearchEdited(String), GameFilterPicked(Option), - /// A directory rescan finished: the clips found plus the upload history read alongside - /// (both come off one blocking task, so neither read stalls the UI thread). - Scanned(Vec, Vec), + /// A directory rescan finished: the clips found plus the upload history and the clip + /// names read alongside (all off one blocking task, so no read stalls the UI thread). + Scanned(Vec, Vec, ClipMetaStore), + /// Start naming the open clip (the title, or the Rename button). + RenameStarted, + RenameEdited(String), + /// Keep what was typed (Enter, or the Save button); an empty name clears it. + RenameCommitted, + /// Throw the edit away (Escape, or the Cancel button). + RenameCancelled, + /// Star or unstar a clip, from its card or from its detail page. + FavouriteToggled(PathBuf), + /// Narrow the grid to starred clips, or stop doing that. + FavouritesFilterToggled, + /// A name or star finished being written to disk, for the clip with this file name. + MetaSaved(String, Result<(), String>), /// The open clip's header was read: its duration in seconds (the trim range's ceiling). SummaryLoaded(PathBuf, f32), ThumbDone(PathBuf, SystemTime, Result), @@ -392,6 +409,17 @@ pub struct Library { /// Remembered successful uploads (per clip, per destination), for badges + the duplicate /// guard. Reloaded on each scan and after a record/forget. history: Vec, + /// The names and stars the user gave their clips. Held in memory and edited optimistically; + /// the write to disk follows, and a reload arrives with the next scan. + meta: ClipMetaStore, + /// The name being typed for the open clip, or `None` when the title is not being edited. + rename: Option, + /// Whether the grid is narrowed to starred clips. + favourites_only: bool, + /// Clips whose edit has not reached disk yet, and whether the value moved on again while + /// that write was running. Only one write per clip is in flight, so two quick stars cannot + /// land in the wrong order, and a rescan in between keeps the in-memory value. + meta_pending: HashMap, } impl Library { @@ -449,6 +477,10 @@ impl Library { // Filled by the first scan (the boot task); reading it here would block the UI // thread during startup. history: Vec::new(), + meta: ClipMetaStore::default(), + rename: None, + favourites_only: false, + meta_pending: HashMap::new(), } } @@ -457,6 +489,7 @@ impl Library { self.open = None; self.confirm_delete = false; self.action_error = None; + self.rename = None; self.clear_strip(); self.reset_preview(); } @@ -474,6 +507,52 @@ impl Library { self.fullscreen = false; } + /// The upload title to suggest for `path`: the name the user gave the clip, else the + /// detected game plus the date, else just the date. + fn suggested_title(&self, path: &Path) -> String { + if let Some(name) = self.meta.name_of(path) { + return name.to_owned(); + } + match self.entry(path).and_then(|e| e.game.as_deref()) { + Some(game) => titled(game), + None => default_title(), + } + } + + /// Apply an edit to the in-memory store, so the grid and the detail page show it before the + /// write lands, and put it on its way to disk. A clip that is already being written is + /// only marked instead; the follow-up write goes out when that one reports back. + fn edit_meta(&mut self, path: &Path, edit: impl FnOnce(&mut ClipMeta)) -> Task { + let Some(file_name) = clip_meta::file_name_of(path).map(str::to_owned) else { + return Task::none(); + }; + self.meta.set(&file_name, edit); + if let Some(again) = self.meta_pending.get_mut(&file_name) { + *again = true; + return Task::none(); + } + self.meta_pending.insert(file_name.clone(), false); + self.write_meta(file_name) + } + + /// Hand this clip's entry, as it stands now, to a blocking write. The whole entry goes out + /// rather than the one field that changed: the in-memory value is what the user is looking + /// at, and only this window edits the store while it runs. + fn write_meta(&self, file_name: String) -> Task { + let meta = self.meta.entry_of(&file_name); + let saved_for = file_name.clone(); + Task::perform( + async move { + tokio::task::spawn_blocking(move || { + clip_meta::update(&file_name, |entry| *entry = meta).map_err(|e| e.to_string()) + }) + .await + .unwrap_or_else(|e| Err(e.to_string())) + }, + move |result| Message::MetaSaved(saved_for.clone(), result), + ) + } + /// The upload record for `entry` at `dest`, if the clip was uploaded there. fn record_for(&self, entry: &ClipEntry, dest: Dest) -> Option<&UploadRecord> { let key = clip_key(entry)?; @@ -503,12 +582,16 @@ impl Library { Task::perform( async move { tokio::task::spawn_blocking(move || { - (rewynd_config::list_clips(&dir), upload_history::load()) + ( + rewynd_config::list_clips(&dir), + upload_history::load(), + clip_meta::load(), + ) }) .await .unwrap_or_default() }, - |(entries, history)| Message::Scanned(entries, history), + |(entries, history, meta)| Message::Scanned(entries, history, meta), ) } @@ -516,7 +599,9 @@ impl Library { match message { Message::SearchEdited(q) => self.search = q, Message::GameFilterPicked(game) => self.game_filter = game, - Message::Scanned(entries, history) => return self.scanned(entries, history), + Message::Scanned(entries, history, meta) => { + return self.scanned(entries, history, meta); + } Message::ThumbDone(path, modified, result) => { // Free the decode slot, unless a newer decode for the same path superseded it. if self.decoding.get(&path) == Some(&modified) { @@ -547,11 +632,10 @@ impl Library { self.trim_end = 0.0; self.trim = TrimState::Idle; self.reset_preview(); - // The suggested title leads with the game when one was detected. - self.title_hint = match self.entry(&path).and_then(|e| e.game.as_deref()) { - Some(game) => titled(game), - None => default_title(), - }; + // The name the user gave the clip is the best upload title there is; failing + // that the suggestion leads with the game when one was detected. + self.title_hint = self.suggested_title(&path); + self.rename = None; let detected = self.entry(&path).and_then(|e| e.game.clone()); self.open = Some(path.clone()); self.confirm_delete = false; @@ -624,6 +708,9 @@ impl Library { if let Some(entry) = self.entries.iter().find(|e| e.path == path) { thumbs::remove_cached(&path, entry.modified); } + // Nor should its name and star. Clearing the entry goes through the same queue + // as every other edit, so a star still being written can't bring it back. + let forgotten = self.edit_meta(&path, |m| *m = ClipMeta::default()); self.entries.retain(|e| e.path != path); self.thumbs.remove(&path); self.pending_thumbs.retain(|(p, _)| p != &path); @@ -633,6 +720,7 @@ impl Library { self.reset_preview(); } self.action_error = None; + return forgotten; } Message::Deleted(Err(e)) => { self.action_error = Some(format!("Could not delete the clip: {e}")); @@ -666,11 +754,31 @@ impl Library { let end = Duration::from_secs_f32(self.trim_end); self.trim = TrimState::Saving; let work_src = src.clone(); + // A copy is a new file: give it the original's name before the rescan reads the + // directory, or it would show up as a date. The whole entry is written, not just + // the name: a copy deleted outside the app leaves its entry behind, and the next + // copy of the same clip takes that file name back. + let copy_meta = matches!(mode, SaveMode::Copy).then(|| ClipMeta { + name: self + .meta + .name_of(&src) + .and_then(clip_meta::trimmed_copy_name), + favourite: false, + }); return Task::perform( async move { - tokio::task::spawn_blocking(move || save_trim(&work_src, mode, start, end)) - .await - .unwrap_or_else(|e| Err(e.to_string())) + tokio::task::spawn_blocking(move || { + let saved = save_trim(&work_src, mode, start, end); + if let (Ok(dst), Some(meta)) = (&saved, copy_meta) + && let Some(file_name) = clip_meta::file_name_of(dst) + && let Err(e) = clip_meta::update(file_name, |m| *m = meta) + { + tracing::warn!(error = %e, "could not name the trimmed copy"); + } + saved + }) + .await + .unwrap_or_else(|e| Err(e.to_string())) }, move |result| Message::TrimSaved { src: src.clone(), @@ -704,6 +812,56 @@ impl Library { Err(_) => {} } } + Message::RenameStarted => { + let Some(path) = self.open.clone() else { + return Task::none(); + }; + self.rename = Some(self.meta.name_of(&path).unwrap_or_default().to_owned()); + return iced::advanced::widget::operate( + iced::advanced::widget::operation::focusable::focus(RENAME_INPUT.into()), + ); + } + Message::RenameEdited(s) => { + if self.rename.is_some() { + self.rename = Some(s); + } + } + Message::RenameCancelled => self.rename = None, + Message::RenameCommitted => { + let (Some(path), Some(draft)) = (self.open.clone(), self.rename.take()) else { + return Task::none(); + }; + let name = clip_meta::clean_name(&draft); + // An untouched upload title follows the clip's name; one the user typed over + // stays theirs. + let untouched = self.title == self.title_hint; + let saved = self.edit_meta(&path, move |m| m.name = name); + self.title_hint = self.suggested_title(&path); + if untouched { + self.title = self.title_hint.clone(); + } + return saved; + } + Message::FavouriteToggled(path) => { + let starred = self.meta.is_favourite(&path); + return self.edit_meta(&path, move |m| m.favourite = !starred); + } + Message::FavouritesFilterToggled => { + self.favourites_only = !self.favourites_only; + } + Message::MetaSaved(file_name, result) => { + let moved_on = self.meta_pending.remove(&file_name).unwrap_or(false); + if let Err(e) = result { + // The detail page shows this; a star toggled from the grid has nowhere to + // put it, so the log is the backstop. + tracing::warn!(error = %e, clip = %file_name, "could not save a clip's name or star"); + self.action_error = Some(format!("Could not save that: {e}")); + } + if moved_on { + self.meta_pending.insert(file_name.clone(), false); + return self.write_meta(file_name); + } + } Message::TitleEdited(s) => self.title = s, Message::DescriptionEdited(action) => { if let Some(action) = self.within_description_cap(action) { @@ -927,7 +1085,12 @@ impl Library { /// Store a fresh scan and queue thumbnail decodes for entries whose (path, mtime) slot is /// missing or stale. The queue is rebuilt from this scan; decodes already in flight keep /// their slot and are not restarted. - fn scanned(&mut self, entries: Vec, history: Vec) -> Task { + fn scanned( + &mut self, + entries: Vec, + history: Vec, + meta: ClipMetaStore, + ) -> Task { self.scanning = false; self.thumbs .retain(|path, _| entries.iter().any(|e| &e.path == path)); @@ -948,6 +1111,15 @@ impl Library { } self.entries = entries; self.history = history; + // The reload brings in what another window (or a previous run) wrote. Edits of ours + // still on their way to disk win over it, or a rescan landing mid-write would show the + // user their own rename undone. + let mut meta = meta; + for file_name in self.meta_pending.keys() { + let ours = self.meta.entry_of(file_name); + meta.set(file_name, |m| *m = ours); + } + self.meta = meta; // Drop a game filter whose section vanished (its last clip was deleted or moved). let stale = self .game_filter @@ -1088,6 +1260,20 @@ impl Library { if self.animating() { subs.push(iced::window::frames().map(Message::Tick)); } + if self.rename.is_some() { + // The field itself swallows Escape (it drops focus), so this listener takes the key + // whether or not the event was captured, and only exists while a rename is open. + subs.push(iced::event::listen_with(|event, _status, _id| { + matches!( + event, + iced::Event::Keyboard(iced::keyboard::Event::KeyPressed { + key: iced::keyboard::Key::Named(iced::keyboard::key::Named::Escape), + .. + }) + ) + .then_some(Message::RenameCancelled) + })); + } if let (Some(path), Some((start, end))) = (&self.open, self.play_range) { let key = ( path.clone(), @@ -1656,10 +1842,10 @@ impl Library { if groups.is_empty() { // The empty result can come from the search box, the game chips, or both; word it // for whichever the user actually touched. - let reason = if self.search.trim().is_empty() { - "No clips in this section." - } else { - "No clips match your search." + let reason = match (self.favourites_only, self.search.trim().is_empty()) { + (_, false) => "No clips match your search.", + (true, true) => "No favourites here yet. Star a clip to keep it close.", + (false, true) => "No clips in this section.", }; sections = sections.push( container(hint(reason)) @@ -1693,11 +1879,18 @@ impl Library { .width(Length::Fixed(260.0)); let labels = self.game_labels(); - let mut chips = row![chip( - "All", - self.game_filter.is_none(), - Message::GameFilterPicked(None) - )] + let mut chips = row![ + chip( + "Favourites", + self.favourites_only, + Message::FavouritesFilterToggled + ), + chip( + "All", + self.game_filter.is_none(), + Message::GameFilterPicked(None) + ) + ] .spacing(8) .align_y(iced::Alignment::Center); if labels.len() > 1 { @@ -1772,8 +1965,12 @@ impl Library { labels } - /// Whether `entry` passes the active game filter and the search query. + /// Whether `entry` passes the favourites filter, the active game filter and the search + /// query. fn matches(&self, entry: &ClipEntry) -> bool { + if self.favourites_only && !self.meta.is_favourite(&entry.path) { + return false; + } if let Some(filter) = &self.game_filter && group_label(entry) != filter { @@ -1783,25 +1980,33 @@ impl Library { if query.is_empty() { return true; } - let name = entry + let file_name = entry .path .file_name() .and_then(|n| n.to_str()) .unwrap_or_default(); - fuzzy_match(query, group_label(entry)) + self.meta + .name_of(&entry.path) + .is_some_and(|name| fuzzy_match(query, name)) + || fuzzy_match(query, group_label(entry)) || fuzzy_match(query, &saved_at_label(entry.saved_at)) - || fuzzy_match(query, name) + || fuzzy_match(query, file_name) } fn clip_card<'a>(&'a self, entry: &'a ClipEntry) -> Element<'a, Message> { // Stack the chips over the "duration · size" line rather than inlining them: a narrow card // (four across) can't fit a game chip, an upload badge, and the readout on one row, and the // squeezed row wrapped unevenly. Stacking keeps every card the same height at any width. + let named = self.meta.name_of(&entry.path); let mut info = column![ - text(saved_at_label(entry.saved_at)) - .size(12) - .font(UI_SEMIBOLD) - .style(tinted(palette::TEXT)), + text(named.map_or_else( + || saved_at_label(entry.saved_at), + std::string::ToString::to_string + )) + .size(12) + .font(UI_SEMIBOLD) + .wrapping(iced::widget::text::Wrapping::None) + .style(tinted(palette::TEXT)), ] .spacing(7); let chips = self.meta_chips(entry); @@ -1813,9 +2018,17 @@ impl Library { info = info.push(chip_row); } let info = info.push( - text(size_label(entry.size_bytes)) - .size(10) - .style(tinted(palette::MUTED)), + text(match named { + Some(_) => format!( + "{} · {}", + saved_at_label(entry.saved_at), + size_label(entry.size_bytes) + ), + None => size_label(entry.size_bytes), + }) + .size(10) + .wrapping(iced::widget::text::Wrapping::None) + .style(tinted(palette::MUTED)), ); let mut layers = vec![self.thumbnail(entry, 148.0)]; @@ -1828,18 +2041,23 @@ impl Library { .height(Length::Fixed(148.0)), ) .clip(true); - button( - column![ - iced::widget::hover(thumb, play_hint()), - container(info).padding([11, 12]), - ] - .spacing(0), - ) - .on_press(Message::Open(entry.path.clone())) - .style(clip_card_style) - .padding(0) + // The star rides above the hover layer so it stays clickable while the play hint shows; + // pressing it never opens the clip, because the card button ignores a captured press. + let framed: Element<'_, Message> = layered(vec![ + iced::widget::hover(thumb, play_hint()), + self.star_overlay(entry), + ]) .width(Length::Fill) - .into() + .height(Length::Fixed(148.0)) + .into(); + // Clipped, so a long name runs out of the card's edge instead of widening it. + let info = container(info).padding([11, 12]).clip(true); + button(column![framed, info].spacing(0)) + .on_press(Message::Open(entry.path.clone())) + .style(clip_card_style) + .padding(0) + .width(Length::Fill) + .into() } /// The clip's chips in order (per-game first, then one per uploaded destination). Empty when @@ -1890,6 +2108,23 @@ impl Library { } } + /// The favourite star in the thumbnail's top-right corner: filled when the clip is starred, + /// an outline otherwise, so an unstarred card still shows where to click. + fn star_overlay<'a>(&self, entry: &'a ClipEntry) -> Element<'a, Message> { + let starred = self.meta.is_favourite(&entry.path); + let star = button(theme::star(15.0, starred)) + .on_press(Message::FavouriteToggled(entry.path.clone())) + .style(theme::overlay_button) + .padding([5, 5]); + container(star) + .align_x(iced::Alignment::End) + .align_y(iced::Alignment::Start) + .width(Length::Fill) + .height(Length::Fill) + .padding(6) + .into() + } + /// The clip's duration as a badge for the thumbnail's corner (arena.md duration badge: /// dark scrim, warm off-white text), once the decode has reported it. fn duration_badge<'a>(&self, entry: &ClipEntry) -> Option> { @@ -2055,17 +2290,18 @@ impl Library { .style(link_button) .padding(0); - // The heading leads with the game when one was detected. - let heading = match &entry.game { - Some(game) => format!("{game} · {}", saved_at_label(entry.saved_at)), - None => saved_at_label(entry.saved_at), - }; - let mut facts = column![ - text(heading.to_uppercase()).size(26).font(DISPLAY_BLACK), - self.meta_row(entry, 11.0, palette::TEXT_SECONDARY), - hint(entry.path.display().to_string()), - ] - .spacing(10); + let mut facts = column![self.heading(entry)].spacing(10); + // A named clip keeps its date in the meta row, where the heading no longer carries it. + if self.meta.name_of(&entry.path).is_some() { + facts = facts.push( + text(saved_at_label(entry.saved_at)) + .size(11) + .font(UI_SEMIBOLD) + .style(tinted(palette::TEXT_SECONDARY)), + ); + } + facts = facts.push(self.meta_row(entry, 11.0, palette::TEXT_SECONDARY)); + facts = facts.push(hint(entry.path.display().to_string())); facts = facts.push(self.actions()); if let Some(e) = &self.action_error { facts = facts.push(text(e.clone()).size(12).style(tinted(palette::DANGER))); @@ -2087,6 +2323,49 @@ impl Library { .into() } + /// The detail page's heading: the clip's name when it has one, else the game and the date. + /// Click it to rename, or edit it in place while a rename is running. + fn heading<'a>(&'a self, entry: &'a ClipEntry) -> Element<'a, Message> { + let fallback = match &entry.game { + Some(game) => format!("{game} · {}", saved_at_label(entry.saved_at)), + None => saved_at_label(entry.saved_at), + }; + let Some(draft) = &self.rename else { + let label = self + .meta + .name_of(&entry.path) + .map_or(fallback, std::string::ToString::to_string); + return button(text(label.to_uppercase()).size(26).font(DISPLAY_BLACK)) + .on_press(Message::RenameStarted) + .style(link_button) + .padding(0) + .into(); + }; + column![ + row![ + text_input(&fallback, draft) + .id(RENAME_INPUT) + .on_input(Message::RenameEdited) + .on_submit(Message::RenameCommitted) + .style(theme::arena_input) + .width(Length::Fill), + button(text("Save").size(11).font(UI_SEMIBOLD)) + .on_press(Message::RenameCommitted) + .style(primary_button) + .padding([9, 14]), + button(text("Cancel").size(11).font(UI_SEMIBOLD)) + .on_press(Message::RenameCancelled) + .style(link_button) + .padding([9, 4]), + ] + .spacing(10) + .align_y(iced::Alignment::Center), + hint("Leave it empty to go back to the date."), + ] + .spacing(6) + .into() + } + /// The open clip's duration in seconds (read from its header on open), the trim ceiling. fn open_duration(&self) -> f32 { self.open_dur @@ -2257,11 +2536,33 @@ impl Library { .align_y(iced::Alignment::Center) .into(); } + let starred = self + .open + .as_ref() + .is_some_and(|path| self.meta.is_favourite(path)); + let favourite = button( + row![ + theme::star(13.0, starred), + text(if starred { "Favourited" } else { "Favourite" }) + .size(11) + .font(UI_SEMIBOLD), + ] + .spacing(7) + .align_y(iced::Alignment::Center), + ) + .on_press_maybe(self.open.clone().map(Message::FavouriteToggled)) + .style(move |theme, status| favourite_button(theme, status, starred)) + .padding([9, 14]); row![ button(text("Open in player").size(11).font(UI_SEMIBOLD)) .on_press(Message::Play) .style(secondary_button) .padding([9, 14]), + favourite, + button(text("Rename").size(11).font(UI_SEMIBOLD)) + .on_press(Message::RenameStarted) + .style(secondary_button) + .padding([9, 14]), button(text("Show in folder").size(11).font(UI_SEMIBOLD)) .on_press(Message::ShowInFolder) .style(secondary_button) @@ -2273,6 +2574,7 @@ impl Library { ] .spacing(10) .align_y(iced::Alignment::Center) + .wrap() .into() } @@ -2984,6 +3286,27 @@ fn name_is_shared(results: &[Game], game: &Game) -> bool { .is_some() } +/// The favourite button: the secondary outline, already wearing the accent once the clip is +/// starred so the state reads without hovering. +fn favourite_button( + theme: &Theme, + status: iced::widget::button::Status, + starred: bool, +) -> iced::widget::button::Style { + let style = secondary_button(theme, status); + if !starred || matches!(status, iced::widget::button::Status::Hovered) { + return style; + } + iced::widget::button::Style { + text_color: palette::ACCENT, + border: Border { + color: palette::ACCENT_BORDER, + ..style.border + }, + ..style + } +} + /// One row of a search dropdown: quiet until hovered, then the accent tint. fn suggestion_style( _theme: &Theme, @@ -3431,6 +3754,219 @@ mod tests { assert_eq!(disk_label(12_300_000_000), "12.3 GB"); } + fn clip_at(path: &str, game: Option<&str>) -> ClipEntry { + ClipEntry { + path: PathBuf::from(path), + game: game.map(str::to_owned), + saved_at: SystemTime::UNIX_EPOCH, + modified: SystemTime::UNIX_EPOCH, + size_bytes: 1, + } + } + + /// A library holding one named, starred clip and one plain one. Nothing here touches disk: + /// the store is filled in memory, and the write tasks the handlers return are dropped. + fn named_library() -> (Library, ClipEntry, ClipEntry) { + let named = clip_at("/c/Elden Ring/rewynd-1-0.mp4", Some("Elden Ring")); + let plain = clip_at("/c/rewynd-2-0.mp4", None); + let mut lib = Library::new(); + lib.meta.set("rewynd-1-0.mp4", |m| { + m.name = Some("Clutch ace".to_owned()); + m.favourite = true; + }); + lib.entries = vec![named.clone(), plain.clone()]; + (lib, named, plain) + } + + #[test] + fn the_favourites_filter_and_the_search_both_see_clip_names() { + let (mut lib, named, plain) = named_library(); + assert!(lib.matches(&named) && lib.matches(&plain), "unfiltered"); + + lib.favourites_only = true; + assert!(lib.matches(&named)); + assert!(!lib.matches(&plain), "an unstarred clip is filtered out"); + + lib.favourites_only = false; + lib.search = "clutch".to_owned(); + assert!( + lib.matches(&named), + "the name is searchable, case-insensitively" + ); + assert!(!lib.matches(&plain)); + + // The old handles still work: the game, the date and the file name. + lib.search = "elden".to_owned(); + assert!(lib.matches(&named)); + lib.search = "rewynd-2".to_owned(); + assert!(lib.matches(&plain)); + } + + #[test] + fn a_clips_name_becomes_its_suggested_upload_title() { + let (lib, named, plain) = named_library(); + assert_eq!(lib.suggested_title(&named.path), "Clutch ace"); + // Without a name the old suggestion stands: the game (or just the date) plus the stamp. + assert!( + lib.suggested_title(&plain.path).starts_with("rewynd "), + "{}", + lib.suggested_title(&plain.path) + ); + } + + #[test] + fn renaming_carries_an_untouched_upload_title_along() { + let (mut lib, named, _) = named_library(); + let config = Config::default(); + lib.open = Some(named.path.clone()); + lib.title_hint = lib.suggested_title(&named.path); + lib.title = lib.title_hint.clone(); + + lib.rename = Some(" Triple kill ".to_owned()); + drop(lib.update(Message::RenameCommitted, &config)); + assert_eq!( + lib.meta.name_of(&named.path), + Some("Triple kill"), + "cleaned up" + ); + assert_eq!(lib.title, "Triple kill", "the title followed the name"); + assert!(lib.rename.is_none(), "the field closed"); + assert!( + lib.meta_pending.contains_key("rewynd-1-0.mp4"), + "the write is owed" + ); + + // A title the user typed over is theirs, and a rename must not overwrite it. + lib.title = "My own title".to_owned(); + lib.rename = Some("Quad kill".to_owned()); + drop(lib.update(Message::RenameCommitted, &config)); + assert_eq!(lib.meta.name_of(&named.path), Some("Quad kill")); + assert_eq!(lib.title, "My own title"); + } + + #[test] + fn an_empty_name_clears_it_and_escape_throws_the_edit_away() { + let (mut lib, named, _) = named_library(); + let config = Config::default(); + lib.open = Some(named.path.clone()); + + lib.rename = Some(" ".to_owned()); + drop(lib.update(Message::RenameCommitted, &config)); + assert_eq!(lib.meta.name_of(&named.path), None, "blank means no name"); + assert!(lib.meta.is_favourite(&named.path), "the star is untouched"); + + lib.rename = Some("Never saved".to_owned()); + drop(lib.update(Message::RenameCancelled, &config)); + assert!(lib.rename.is_none()); + assert_eq!( + lib.meta.name_of(&named.path), + None, + "cancelling stores nothing" + ); + } + + #[test] + fn the_star_toggles_from_either_side_of_the_library() { + let (mut lib, named, plain) = named_library(); + let config = Config::default(); + + drop(lib.update(Message::FavouriteToggled(named.path.clone()), &config)); + assert!( + !lib.meta.is_favourite(&named.path), + "starred clip unstarred" + ); + assert_eq!( + lib.meta.name_of(&named.path), + Some("Clutch ace"), + "name kept" + ); + + drop(lib.update(Message::FavouriteToggled(plain.path.clone()), &config)); + assert!(lib.meta.is_favourite(&plain.path)); + } + + #[test] + fn a_rescan_never_undoes_an_edit_still_on_its_way_to_disk() { + let (mut lib, named, _) = named_library(); + let config = Config::default(); + lib.open = Some(named.path.clone()); + lib.rename = Some("Just typed".to_owned()); + drop(lib.update(Message::RenameCommitted, &config)); + + // The scan reads the store from before the write landed. + let stale = ClipMetaStore::default(); + drop(lib.scanned(vec![named.clone()], Vec::new(), stale)); + assert_eq!( + lib.meta.name_of(&named.path), + Some("Just typed"), + "our own unwritten edit survives the reload" + ); + + // Once the write is confirmed, a later scan is free to bring back whatever is on disk. + drop(lib.update( + Message::MetaSaved("rewynd-1-0.mp4".to_owned(), Ok(())), + &config, + )); + assert!(lib.meta_pending.is_empty()); + drop(lib.scanned(vec![named.clone()], Vec::new(), ClipMetaStore::default())); + assert_eq!(lib.meta.name_of(&named.path), None); + } + + #[test] + fn a_second_star_waits_for_the_first_write_to_report_back() { + let (mut lib, named, _) = named_library(); + let config = Config::default(); + let key = "rewynd-1-0.mp4"; + + drop(lib.update(Message::FavouriteToggled(named.path.clone()), &config)); + assert_eq!(lib.meta_pending.get(key), Some(&false), "one write is out"); + + // A second press while that write runs only marks the clip; two writes racing could + // otherwise land on disk in the wrong order. + drop(lib.update(Message::FavouriteToggled(named.path.clone()), &config)); + assert_eq!(lib.meta_pending.get(key), Some(&true)); + assert!( + lib.meta.is_favourite(&named.path), + "the UI followed both presses" + ); + + drop(lib.update(Message::MetaSaved(key.to_owned(), Ok(())), &config)); + assert_eq!( + lib.meta_pending.get(key), + Some(&false), + "the follow-up write went out with the value as it now stands" + ); + drop(lib.update(Message::MetaSaved(key.to_owned(), Ok(())), &config)); + assert!(lib.meta_pending.is_empty(), "nothing left owed"); + } + + #[test] + fn deleting_a_clip_forgets_its_name_and_star() { + let (mut lib, named, _) = named_library(); + let config = Config::default(); + drop(lib.update(Message::Deleted(Ok(named.path.clone())), &config)); + assert_eq!(lib.meta.name_of(&named.path), None); + assert!(!lib.meta.is_favourite(&named.path)); + assert!( + lib.meta_pending.contains_key("rewynd-1-0.mp4"), + "the clearing write is on its way out, ordered behind any other edit" + ); + } + + #[test] + fn a_failed_write_says_so() { + let (mut lib, _, _) = named_library(); + let config = Config::default(); + drop(lib.update( + Message::MetaSaved("rewynd-1-0.mp4".to_owned(), Err("disk full".to_owned())), + &config, + )); + assert_eq!( + lib.action_error.as_deref(), + Some("Could not save that: disk full") + ); + } + #[test] fn group_label_falls_back_to_the_root_section() { let with_game = ClipEntry { diff --git a/crates/settings/src/theme.rs b/crates/settings/src/theme.rs index ab27bcd..6187a35 100644 --- a/crates/settings/src/theme.rs +++ b/crates/settings/src/theme.rs @@ -119,6 +119,30 @@ pub fn play_badge<'a, M: 'a>(size: f32) -> Element<'a, M> { iced::widget::image(handle).width(size).height(size).into() } +// The favourite star, in the two states a clip can be in. Its own small PNGs for the same +// reason as the play badge: the GUI deliberately builds without iced's svg backend. +static STAR_FILLED: LazyLock = LazyLock::new(|| { + iced::widget::image::Handle::from_bytes( + include_bytes!("../assets/star/star-filled-48.png").as_slice(), + ) +}); +static STAR_OUTLINE: LazyLock = LazyLock::new(|| { + iced::widget::image::Handle::from_bytes( + include_bytes!("../assets/star/star-outline-48.png").as_slice(), + ) +}); + +/// The favourite star at `size` logical pixels: mint when the clip is starred, a light outline +/// when it is not. +pub fn star<'a, M: 'a>(size: f32, filled: bool) -> Element<'a, M> { + let handle = if filled { + STAR_FILLED.clone() + } else { + STAR_OUTLINE.clone() + }; + iced::widget::image(handle).width(size).height(size).into() +} + /// The window icon, decoded from the shipped PNG render of the mark (X11/Windows; see the /// `window::Settings` note for Wayland). pub fn window_icon() -> Option { diff --git a/docs/adr/0024-clip-names-and-favourites.md b/docs/adr/0024-clip-names-and-favourites.md new file mode 100644 index 0000000..3a4cd95 --- /dev/null +++ b/docs/adr/0024-clip-names-and-favourites.md @@ -0,0 +1,63 @@ +# 0024 — Clip names and favourites: a metadata store, not a file rename + +## Status + +Accepted. + +## Context + +The library shows every clip by its save time, which makes one clip look like the next. People +want to name the ones worth keeping and to star their best, then find those again. + +ADR 0013 made the filesystem the model: a clip is a `rewynd--.mp4` under the +output directory, and everything the library shows is derived from the directory listing. The +only per-clip state that survives a restart today is the upload history, a small JSON file +beside `config.toml`, keyed by file name plus size plus mtime so a record dies when the file is +rewritten. + +## Decision + +- **A name is metadata; the file on disk keeps its generated name.** Renaming the file would + break four things at once: the `rewynd-*.mp4` filter that makes a file a clip at all, the + millisecond stamp that dates it, the `rewynd://clip/` deep link a save toast opens, and + the upload history's identity. Windows also holds the file open while the player has it. The + library shows the name, "Show in folder" still leads to the real file. + +- **A second store beside `config.toml`: `clip-meta.json`.** A map of file name to + `{ name, favourite }`, `BTreeMap`-ordered so the file is stable, entries at their default + dropped rather than written. Same handling as the upload history: 0600, atomic temp plus + rename, read-modify-write under an exclusive lock, unreadable or corrupt means empty. The + private writer both stores use now lives in `paths.rs`. + +- **Keyed by file name alone, unlike the upload history.** A name has to survive a clip moving + between game folders and a trim that rewrites the file in place; mtime in the key would throw + it away on exactly the edit a user expects to keep it. Recorded clips carry a millisecond + stamp plus a per-process sequence, so they do not collide. Trimmed copies are the exception: + they are named after their source, so a copy deleted and made again takes the same name back. + Every write for a copy therefore replaces the whole entry rather than one field, so a stale + name or star can never attach itself to a new file. + +- **No sidecar file per clip.** Clips live in the user's Videos folder, which is theirs; a + second file next to every recording is litter, and a copy of a clip elsewhere would leave its + sidecar behind. + +- **Edits apply in memory first, then write.** The grid updates on the keystroke; the write runs + on a blocking task. A rescan landing mid-write keeps the in-memory value for clips whose write + is still owed, so a directory watch cannot show someone their own rename undone. One write per + clip is in flight at a time: a further edit marks the clip, and its write goes out when the + running one reports back, carrying whatever the value is by then. Deleting a clip clears its + entry through that same queue, so an edit still in flight cannot bring it back. + +- **No manual ordering.** The grid is newest-first and grouped by game, both derived. A + hand-placed order has nowhere to live in that model, and iced has no drag-and-drop to build it + on. Favourites plus search answer "show me the good ones" without inventing a third ordering. + +## Consequences + +- A clip deleted outside the app leaves its entry behind. Harmless: entries are looked up by + the clips actually on disk, so a stale one is never shown, and the only file name that can + come back is a trimmed copy's, which is written over in full. In-app deletes clear it. +- The upload title now defaults to the clip's name when it has one, which is what a person + would have typed anyway. +- A trimmed copy inherits "`` (trimmed)", written before the rescan sees the new file, so + the copy never flashes up as a bare date. From a9a5247033636a760b8b2ecbbb8842fc621beca1 Mon Sep 17 00:00:00 2001 From: Thijs Herman <64640608+Turbootzz@users.noreply.github.com> Date: Fri, 18 Sep 2026 18:17:42 +0200 Subject: [PATCH 2/2] Apply clip edits under the store lock, and flush them Three things the review caught. An edit now travels as data (name it, star it, forget it) and is applied to whatever the store holds once its lock is taken, instead of replacing the entry with a copy taken before the lock. Two writers, one renaming and one starring, no longer overwrite each other's field. The settings window is normally the only writer, but its instance lock can fail open. The in-flight edits are also what a rescan replays over what it read, so a reload keeps both sides. The shared private writer now flushes the staged file and the directory entry before it reports success, so a crash straight after a save cannot leave the store on its previous contents. Both it and the upload history are written when someone acts, never in a hot path. U+061C is a bidi control like the ones already filtered out of a name, but it is a format character rather than a control one, so is_control missed it. --- crates/config/src/clip_meta.rs | 90 +++++++++++--- crates/config/src/paths.rs | 29 ++++- crates/settings/src/library.rs | 130 ++++++++++++++------- docs/adr/0024-clip-names-and-favourites.md | 21 +++- 4 files changed, 205 insertions(+), 65 deletions(-) diff --git a/crates/config/src/clip_meta.rs b/crates/config/src/clip_meta.rs index eee7630..e9befc6 100644 --- a/crates/config/src/clip_meta.rs +++ b/crates/config/src/clip_meta.rs @@ -36,6 +36,27 @@ fn is_false(b: &bool) -> bool { !*b } +/// One change to a clip's entry. Carried as data rather than a closure so the same change can +/// be shown on screen now and applied to the stored entry later, under the store's lock. +#[derive(Debug, Clone, PartialEq, Eq)] +pub enum ClipEdit { + /// Name it, or (with `None`) take its name away. + Name(Option), + Favourite(bool), + /// Forget the clip: what a delete leaves behind, and what a trimmed copy starts from. + Reset, +} + +impl ClipEdit { + fn apply_to(&self, meta: &mut ClipMeta) { + match self { + Self::Name(name) => meta.name.clone_from(name), + Self::Favourite(on) => meta.favourite = *on, + Self::Reset => *meta = ClipMeta::default(), + } + } +} + /// Every clip we know something about, keyed by file name. A `BTreeMap` so the file stays in a /// stable order instead of reshuffling on every write. #[derive(Debug, Clone, Default, PartialEq, Eq, Serialize, Deserialize)] @@ -73,10 +94,13 @@ impl ClipMetaStore { } } - /// This clip's entry by store key, or a default one when it has none. - #[must_use] - pub fn entry_of(&self, file_name: &str) -> ClipMeta { - self.0.get(file_name).cloned().unwrap_or_default() + /// Apply `edits` in order to this clip's entry. + pub fn apply(&mut self, file_name: &str, edits: &[ClipEdit]) { + self.set(file_name, |meta| { + for edit in edits { + edit.apply_to(meta); + } + }); } /// Whether the store holds nothing at all. @@ -98,6 +122,7 @@ fn is_invisible(c: char) -> bool { c.is_control() || matches!(c, '\u{00ad}' // soft hyphen + | '\u{061c}' // Arabic letter mark | '\u{200b}'..='\u{200f}' // zero width spaces, joiners, LTR/RTL marks | '\u{202a}'..='\u{202e}' // bidi embedding and overrides | '\u{2060}'..='\u{2064}' // word joiner and invisible operators @@ -150,15 +175,15 @@ pub fn load() -> ClipMetaStore { store_path().map(|p| load_at(&p)).unwrap_or_default() } -/// Apply `edit` to one clip's entry and write the store back. The read-modify-write runs under -/// an exclusive file lock, so the settings window and a second instance can't clobber each -/// other's edits. An edit that leaves the entry at its default removes it, which is how a -/// deleted clip is forgotten. -pub fn update(file_name: &str, edit: impl FnOnce(&mut ClipMeta)) -> std::io::Result<()> { +/// Apply `edits` to one clip's entry and write the store back. The read-modify-write runs under +/// an exclusive file lock, and each edit touches only its own field of whatever the store holds +/// by then, so a second writer's rename and this one's star don't overwrite each other. An edit +/// that leaves the entry at its default removes it, which is how a deleted clip is forgotten. +pub fn update(file_name: &str, edits: &[ClipEdit]) -> std::io::Result<()> { let path = store_path().ok_or_else(no_path)?; with_store_lock(&path, || { let mut store = load_at(&path); - store.set(file_name, edit); + store.apply(file_name, edits); save_at(&path, &store) }) } @@ -281,9 +306,9 @@ mod tests { named(&mut store, "rewynd-2-0.mp4", "Stays"); save_at(&path, &store).expect("save"); - // What the delete handler does: leave the entry at its default. + // What the delete handler does: clear the entry. let mut store = load_at(&path); - store.set("rewynd-1-0.mp4", |m| *m = ClipMeta::default()); + store.apply("rewynd-1-0.mp4", &[ClipEdit::Reset]); save_at(&path, &store).expect("save"); let after = load_at(&path); @@ -294,13 +319,52 @@ mod tests { #[test] fn clean_name_drops_invisible_and_direction_flipping_characters() { // A name is drawn as a heading, so nothing in it may hide text or reverse it. - let sneaky = "Ace\u{202e}gpj.exe\u{200b} \u{feff}clip"; + let sneaky = "Ace\u{202e}gpj.exe\u{200b} \u{061c}\u{feff}clip"; let cleaned = clean_name(sneaky).expect("name"); assert_eq!(cleaned, "Ace gpj.exe clip"); assert!(!cleaned.chars().any(is_invisible), "{cleaned:?}"); assert_eq!(clean_name("\u{200b}\u{202e}\u{feff}"), None); } + #[test] + fn an_edit_leaves_every_other_field_alone() { + let mut store = ClipMetaStore::default(); + store.apply( + "rewynd-1-0.mp4", + &[ClipEdit::Name(Some("Clutch ace".to_owned()))], + ); + store.apply("rewynd-1-0.mp4", &[ClipEdit::Favourite(true)]); + let clip = Path::new("rewynd-1-0.mp4"); + assert_eq!(store.name_of(clip), Some("Clutch ace")); + assert!(store.is_favourite(clip)); + + // Starring a clip someone else renamed in the meantime keeps their name: the edit is + // applied to the entry as it stands, not to a snapshot taken before the lock. + store.apply( + "rewynd-1-0.mp4", + &[ClipEdit::Name(Some("Their name".to_owned()))], + ); + store.apply("rewynd-1-0.mp4", &[ClipEdit::Favourite(false)]); + assert_eq!(store.name_of(clip), Some("Their name")); + assert!(!store.is_favourite(clip)); + + // Reset drops the whole entry, whatever is in it. + store.apply("rewynd-1-0.mp4", &[ClipEdit::Reset]); + assert!(store.is_empty()); + + // Several edits in one go run in order. + store.apply( + "rewynd-2-0.mp4", + &[ + ClipEdit::Favourite(true), + ClipEdit::Name(Some("Kept".to_owned())), + ClipEdit::Favourite(false), + ], + ); + assert_eq!(store.name_of(Path::new("rewynd-2-0.mp4")), Some("Kept")); + assert!(!store.is_favourite(Path::new("rewynd-2-0.mp4"))); + } + #[test] fn clean_name_tidies_and_caps() { assert_eq!( diff --git a/crates/config/src/paths.rs b/crates/config/src/paths.rs index 041bc8b..3b9510d 100644 --- a/crates/config/src/paths.rs +++ b/crates/config/src/paths.rs @@ -185,6 +185,10 @@ pub(crate) fn write_file_atomic(path: &Path, contents: &[u8]) -> std::io::Result /// Write `bytes` to `path` atomically and owner-only (0600 on unix), creating parent /// directories. Used for the small JSON stores beside `config.toml` that hold private data: /// upload history (remote ids and share links) and clip names. +/// +/// The staged file reaches the disk before the rename, and on unix the directory entry follows +/// it, so a crash straight after a save can't leave the store empty or holding the previous +/// version. These files are written when someone renames or stars a clip, not in any hot path. pub(crate) fn write_private_atomic(path: &Path, bytes: &[u8]) -> std::io::Result<()> { use std::io::Write; if let Some(parent) = path.parent() { @@ -200,14 +204,35 @@ pub(crate) fn write_private_atomic(path: &Path, bytes: &[u8]) -> std::io::Result } let result = options .open(&staged) - .and_then(|mut file| file.write_all(bytes)) - .and_then(|()| std::fs::rename(&staged, path)); + .and_then(|mut file| { + file.write_all(bytes)?; + file.sync_all() + }) + .and_then(|()| std::fs::rename(&staged, path)) + .and_then(|()| sync_dir_of(path)); if result.is_err() { let _ = std::fs::remove_file(&staged); } result } +/// Flush the directory entry a rename just created, so the file is findable after a crash. +/// Only unix can open a directory for this; Windows has no equivalent call. +#[cfg(unix)] +fn sync_dir_of(path: &Path) -> std::io::Result<()> { + let dir = path.parent().filter(|p| !p.as_os_str().is_empty()); + match dir { + Some(dir) => std::fs::File::open(dir)?.sync_all(), + None => Ok(()), + } +} + +#[cfg(not(unix))] +#[expect(clippy::unnecessary_wraps, reason = "matches the unix signature")] +fn sync_dir_of(_path: &Path) -> std::io::Result<()> { + Ok(()) +} + /// `name` beside `exe`, with the platform's executable suffix. The testable core of /// [`sibling_binary`]. fn sibling_of(exe: &Path, name: &str) -> Option { diff --git a/crates/settings/src/library.rs b/crates/settings/src/library.rs index 3427bdf..583d054 100644 --- a/crates/settings/src/library.rs +++ b/crates/settings/src/library.rs @@ -12,7 +12,7 @@ use iced::widget::{ }; use iced::{Background, Border, Element, Length, Task, Theme}; -use rewynd_config::clip_meta::{self, ClipMeta, ClipMetaStore}; +use rewynd_config::clip_meta::{self, ClipEdit, ClipMetaStore}; use rewynd_config::upload_history::{self, ClipKey, UploadRecord}; use rewynd_config::{ClipEntry, Config}; use rewynd_upload::youtube::{ @@ -306,6 +306,23 @@ pub enum Message { TrimReset, } +/// The edits to one clip that are not on disk yet: the ones handed to the running write, and +/// the ones made since, waiting for it to report back. +#[derive(Debug, Default)] +struct PendingMeta { + writing: Vec, + waiting: Vec, +} + +impl PendingMeta { + /// Everything still owed, oldest first. + fn all(&self) -> Vec { + let mut edits = self.writing.clone(); + edits.extend(self.waiting.iter().cloned()); + edits + } +} + /// What a trim save does with the result. #[derive(Debug, Clone, Copy, PartialEq, Eq)] pub enum SaveMode { @@ -416,10 +433,10 @@ pub struct Library { rename: Option, /// Whether the grid is narrowed to starred clips. favourites_only: bool, - /// Clips whose edit has not reached disk yet, and whether the value moved on again while - /// that write was running. Only one write per clip is in flight, so two quick stars cannot - /// land in the wrong order, and a rescan in between keeps the in-memory value. - meta_pending: HashMap, + /// Clips whose edits have not reached disk yet, keyed by file name. Only one write per clip + /// runs at a time, so two quick stars cannot land in the wrong order, and a rescan in + /// between replays what is still owed over what it read. + meta_pending: HashMap, } impl Library { @@ -519,32 +536,35 @@ impl Library { } } - /// Apply an edit to the in-memory store, so the grid and the detail page show it before the - /// write lands, and put it on its way to disk. A clip that is already being written is - /// only marked instead; the follow-up write goes out when that one reports back. - fn edit_meta(&mut self, path: &Path, edit: impl FnOnce(&mut ClipMeta)) -> Task { + /// Show an edit at once and put it on its way to disk. A clip that is already being written + /// keeps the edit back until that write reports, so its writes stay in order. + fn edit_meta(&mut self, path: &Path, edit: ClipEdit) -> Task { let Some(file_name) = clip_meta::file_name_of(path).map(str::to_owned) else { return Task::none(); }; - self.meta.set(&file_name, edit); - if let Some(again) = self.meta_pending.get_mut(&file_name) { - *again = true; + self.meta.apply(&file_name, std::slice::from_ref(&edit)); + if let Some(pending) = self.meta_pending.get_mut(&file_name) { + pending.waiting.push(edit); return Task::none(); } - self.meta_pending.insert(file_name.clone(), false); - self.write_meta(file_name) + self.write_meta(file_name, vec![edit]) } - /// Hand this clip's entry, as it stands now, to a blocking write. The whole entry goes out - /// rather than the one field that changed: the in-memory value is what the user is looking - /// at, and only this window edits the store while it runs. - fn write_meta(&self, file_name: String) -> Task { - let meta = self.meta.entry_of(&file_name); + /// Hand a clip's owed edits to a blocking write. They are applied to whatever the store + /// holds once its lock is taken, so another writer's other field survives ours. + fn write_meta(&mut self, file_name: String, edits: Vec) -> Task { + self.meta_pending.insert( + file_name.clone(), + PendingMeta { + writing: edits.clone(), + waiting: Vec::new(), + }, + ); let saved_for = file_name.clone(); Task::perform( async move { tokio::task::spawn_blocking(move || { - clip_meta::update(&file_name, |entry| *entry = meta).map_err(|e| e.to_string()) + clip_meta::update(&file_name, &edits).map_err(|e| e.to_string()) }) .await .unwrap_or_else(|e| Err(e.to_string())) @@ -710,7 +730,7 @@ impl Library { } // Nor should its name and star. Clearing the entry goes through the same queue // as every other edit, so a star still being written can't bring it back. - let forgotten = self.edit_meta(&path, |m| *m = ClipMeta::default()); + let forgotten = self.edit_meta(&path, ClipEdit::Reset); self.entries.retain(|e| e.path != path); self.thumbs.remove(&path); self.pending_thumbs.retain(|(p, _)| p != &path); @@ -758,20 +778,20 @@ impl Library { // directory, or it would show up as a date. The whole entry is written, not just // the name: a copy deleted outside the app leaves its entry behind, and the next // copy of the same clip takes that file name back. - let copy_meta = matches!(mode, SaveMode::Copy).then(|| ClipMeta { - name: self + let copy_edits = matches!(mode, SaveMode::Copy).then(|| { + let name = self .meta .name_of(&src) - .and_then(clip_meta::trimmed_copy_name), - favourite: false, + .and_then(clip_meta::trimmed_copy_name); + vec![ClipEdit::Reset, ClipEdit::Name(name)] }); return Task::perform( async move { tokio::task::spawn_blocking(move || { let saved = save_trim(&work_src, mode, start, end); - if let (Ok(dst), Some(meta)) = (&saved, copy_meta) + if let (Ok(dst), Some(edits)) = (&saved, copy_edits) && let Some(file_name) = clip_meta::file_name_of(dst) - && let Err(e) = clip_meta::update(file_name, |m| *m = meta) + && let Err(e) = clip_meta::update(file_name, &edits) { tracing::warn!(error = %e, "could not name the trimmed copy"); } @@ -835,7 +855,7 @@ impl Library { // An untouched upload title follows the clip's name; one the user typed over // stays theirs. let untouched = self.title == self.title_hint; - let saved = self.edit_meta(&path, move |m| m.name = name); + let saved = self.edit_meta(&path, ClipEdit::Name(name)); self.title_hint = self.suggested_title(&path); if untouched { self.title = self.title_hint.clone(); @@ -844,22 +864,25 @@ impl Library { } Message::FavouriteToggled(path) => { let starred = self.meta.is_favourite(&path); - return self.edit_meta(&path, move |m| m.favourite = !starred); + return self.edit_meta(&path, ClipEdit::Favourite(!starred)); } Message::FavouritesFilterToggled => { self.favourites_only = !self.favourites_only; } Message::MetaSaved(file_name, result) => { - let moved_on = self.meta_pending.remove(&file_name).unwrap_or(false); + let waiting = self + .meta_pending + .remove(&file_name) + .map(|pending| pending.waiting) + .unwrap_or_default(); if let Err(e) = result { // The detail page shows this; a star toggled from the grid has nowhere to // put it, so the log is the backstop. tracing::warn!(error = %e, clip = %file_name, "could not save a clip's name or star"); self.action_error = Some(format!("Could not save that: {e}")); } - if moved_on { - self.meta_pending.insert(file_name.clone(), false); - return self.write_meta(file_name); + if !waiting.is_empty() { + return self.write_meta(file_name, waiting); } } Message::TitleEdited(s) => self.title = s, @@ -1115,9 +1138,8 @@ impl Library { // still on their way to disk win over it, or a rescan landing mid-write would show the // user their own rename undone. let mut meta = meta; - for file_name in self.meta_pending.keys() { - let ours = self.meta.entry_of(file_name); - meta.set(file_name, |m| *m = ours); + for (file_name, pending) in &self.meta_pending { + meta.apply(file_name, &pending.all()); } self.meta = meta; // Drop a game filter whose section vanished (its last clip was deleted or moved). @@ -3919,27 +3941,47 @@ mod tests { let key = "rewynd-1-0.mp4"; drop(lib.update(Message::FavouriteToggled(named.path.clone()), &config)); - assert_eq!(lib.meta_pending.get(key), Some(&false), "one write is out"); + let pending = lib.meta_pending.get(key).expect("one write is out"); + assert_eq!(pending.writing, [ClipEdit::Favourite(false)]); + assert!(pending.waiting.is_empty()); - // A second press while that write runs only marks the clip; two writes racing could + // A second press while that write runs waits its turn; two writes racing could // otherwise land on disk in the wrong order. drop(lib.update(Message::FavouriteToggled(named.path.clone()), &config)); - assert_eq!(lib.meta_pending.get(key), Some(&true)); + let pending = lib.meta_pending.get(key).expect("still owed"); + assert_eq!(pending.waiting, [ClipEdit::Favourite(true)]); assert!( lib.meta.is_favourite(&named.path), "the UI followed both presses" ); drop(lib.update(Message::MetaSaved(key.to_owned(), Ok(())), &config)); - assert_eq!( - lib.meta_pending.get(key), - Some(&false), - "the follow-up write went out with the value as it now stands" - ); + let pending = lib.meta_pending.get(key).expect("the follow-up went out"); + assert_eq!(pending.writing, [ClipEdit::Favourite(true)]); + assert!(pending.waiting.is_empty()); + drop(lib.update(Message::MetaSaved(key.to_owned(), Ok(())), &config)); assert!(lib.meta_pending.is_empty(), "nothing left owed"); } + #[test] + fn a_rescan_replays_only_what_is_still_owed() { + let (mut lib, named, _) = named_library(); + let config = Config::default(); + drop(lib.update(Message::FavouriteToggled(named.path.clone()), &config)); + + // Another window renamed the same clip while our star was being written. The scan + // brings their name back, and our star is replayed on top of it: neither is lost. + let mut theirs = ClipMetaStore::default(); + theirs.apply( + "rewynd-1-0.mp4", + &[ClipEdit::Name(Some("Their name".to_owned()))], + ); + drop(lib.scanned(vec![named.clone()], Vec::new(), theirs)); + assert_eq!(lib.meta.name_of(&named.path), Some("Their name")); + assert!(!lib.meta.is_favourite(&named.path), "our unstar survived"); + } + #[test] fn deleting_a_clip_forgets_its_name_and_star() { let (mut lib, named, _) = named_library(); diff --git a/docs/adr/0024-clip-names-and-favourites.md b/docs/adr/0024-clip-names-and-favourites.md index 3a4cd95..80a93fe 100644 --- a/docs/adr/0024-clip-names-and-favourites.md +++ b/docs/adr/0024-clip-names-and-favourites.md @@ -27,7 +27,10 @@ rewritten. `{ name, favourite }`, `BTreeMap`-ordered so the file is stable, entries at their default dropped rather than written. Same handling as the upload history: 0600, atomic temp plus rename, read-modify-write under an exclusive lock, unreadable or corrupt means empty. The - private writer both stores use now lives in `paths.rs`. + private writer both stores use now lives in `paths.rs`, and it flushes the staged file and + the directory entry before reporting success, so a crash right after a save cannot leave the + store holding the previous version. These writes happen when someone renames or stars a clip, + never in a hot path. - **Keyed by file name alone, unlike the upload history.** A name has to survive a clip moving between game folders and a trim that rewrites the file in place; mtime in the key would throw @@ -41,12 +44,18 @@ rewritten. second file next to every recording is litter, and a copy of a clip elsewhere would leave its sidecar behind. +- **An edit travels as data, not as a new copy of the entry.** "Name it this", "star it", + "forget it": each is applied to whatever the store holds once its lock is taken, so a writer + that only renamed and one that only starred cannot overwrite each other's field. The settings + window is normally the single writer, but its instance lock can fail open, and the store is + shared state either way. + - **Edits apply in memory first, then write.** The grid updates on the keystroke; the write runs - on a blocking task. A rescan landing mid-write keeps the in-memory value for clips whose write - is still owed, so a directory watch cannot show someone their own rename undone. One write per - clip is in flight at a time: a further edit marks the clip, and its write goes out when the - running one reports back, carrying whatever the value is by then. Deleting a clip clears its - entry through that same queue, so an edit still in flight cannot bring it back. + on a blocking task. One write per clip is in flight at a time: a further edit waits for the + running one to report back and goes out behind it, so a clip's writes stay in order. A rescan + landing mid-write replays the edits still owed over what it read, so a directory watch cannot + show someone their own rename undone while keeping anything another writer changed. Deleting a + clip clears its entry through that same queue, so an edit in flight cannot bring it back. - **No manual ordering.** The grid is newest-first and grouped by game, both derived. A hand-placed order has nowhere to live in that model, and iced has no drag-and-drop to build it