From 0289b50606536e89c716af39171ffc698788b1f2 Mon Sep 17 00:00:00 2001 From: clover caruso Date: Fri, 2 Oct 2026 14:39:43 -0700 Subject: [PATCH] feat: Notebook Properties can rename the notebook's folder too OneNote 2010's Notebook Properties changes only the display name. A "Rename the folder too" box, on by default, shows under Display name once the name changes; checked, OK renames the notebook's folder. In the Windows 7 lab OneNote 2010, finding a notebook folder renamed outside it, drops the notebook from its list (even while running) and opens the renamed folder again with every file as it was: the TOC keeps its name (Open Notebook.onetoc2) and no byte changes, and the colour stays with it. So only the folder is renamed. Notebook::rename_folder renames it through the notebook's storage. On a share it first takes and lets go of OneNote's writer opening and locks on every section and TOC, refusing while another writer holds one; the server itself refuses a folder whose files are open. The notebook's replicas (copies' included) and its listing then follow, so edits queued offline publish into the renamed folder. The app stops the notebook's background (Background::stop now waits for its thread), closes the open section, renames on a thread and opens the notebook again in its place in the sidebar at the section it showed; display names, read state, recents, the sidebar's folds and last pages follow the new location, and settings save it. A refusal reopens the notebook as it was and says why; nothing changes. The dialog warns, for a notebook on a share, that other computers must reopen it from the new folder. In the app's iCloud Drive folder, a listing taken mid-rename neither closes the renamed notebook nor opens the old name, and a listing that finds the renamed folder first is folded into the same entry. Assisted-by: claude-opus-5.5 --- arc/sync.md | 5 +- crates/notebook/README.md | 4 + crates/notebook/src/background.rs | 21 ++- crates/notebook/src/session.rs | 104 ++++++++++++- crates/notebook/src/session/tests.rs | 3 + crates/notebook/src/sidecar/tests.rs | 4 + crates/notebook/src/smb/mod.rs | 6 + crates/notebook/src/smb/tests.rs | 66 ++++++++ crates/notebook/tests/location.rs | 53 +++++++ crates/snowbound/src/library.rs | 220 ++++++++++++++++++++++++++- crates/snowbound/src/main.rs | 7 + crates/snowbound/src/manage.rs | 135 +++++++++++++++- crates/snowbound/src/navigation.rs | 11 ++ crates/snowbound/src/properties.rs | 33 +++- crates/snowbound/src/sidebar.rs | 5 +- crates/snowbound/src/unread.rs | 8 + 16 files changed, 663 insertions(+), 22 deletions(-) diff --git a/arc/sync.md b/arc/sync.md index 2c2e936ada2948daa32c0fd3568b6425ec6d2936..1cd5b296b6e85c476fee6cc313b15357623208d0 100644 --- a/arc/sync.md +++ b/arc/sync.md @@ -80,8 +80,9 @@ by its own path instead, the original being the one the folder's TOC lists. When a notebook moves, its queue follows by one of three roads: -- **the app moved it** (iOS Rename): `location::moved` moves the folder's - replicas to the new location; +- **the app moved it** (iOS Rename, or Notebook Properties' "Rename the folder + too"): `location::moved` moves the folder's replicas to the new location, the + latter through `Notebook::rename_folder` once no other writer holds a file; - **something else moved it** (Finder): on open, a section with no replica takes one from the folder of a local location that no longer exists, if the file stands as that replica's base or has moved on from it (a higher header diff --git a/crates/notebook/README.md b/crates/notebook/README.md index 23699b632c0432db56f7a074f2748385b85c2f53..eb3bdf08bbd503943cf90c4f473d17a209f502a0 100644 --- a/crates/notebook/README.md +++ b/crates/notebook/README.md @@ -330,6 +330,10 @@ unavailable bin refuses the delete (`corpus/recycle-bin-repair`). `empty_recycle Empty Recycle Bin: Deleted Pages loses every page in one revision and each binned section's file goes, the bin's TOC left as it was (`corpus/recycle-bin-view`). An entry a TOC still lists for a file gone from its folder gives way to the file an edit gives its name. +`rename_folder` renames the notebook's own folder, which OneNote 2010 opens again from its +new name with no file inside changed (it drops the old one from its list): on a share it +first takes and lets go of OneNote's writer locks on every section and TOC, refusing with +`WouldBlock` while another writer holds one, then the replicas and listing follow. Edits keep entries in the order their ordering numbers give; a rename or colour keeps the numbers. Every created, renamed or moved file is placed with `onestore::place_file`, which sets the header's ancestor to the parent TOC's diff --git a/crates/notebook/src/background.rs b/crates/notebook/src/background.rs index 7f9909aaafd8b81d41022f04ecc6a38976d12fbd..931b12540c81375be10bfacfff8506c1d2305331 100644 --- a/crates/notebook/src/background.rs +++ b/crates/notebook/src/background.rs @@ -42,7 +42,7 @@ const SETTLE: Duration = Duration::from_secs(1); /// its offline copy, where the notebook keeps them. A section a session holds is left to that /// session's worker, which the watch wakes. Dropping requests cancellation without waiting for /// the step in flight. -pub struct Background(Arc); +pub struct Background(Arc, Mutex>>); /// A section for `Background::watch`, as its notebook knows it. pub struct Known { @@ -138,7 +138,7 @@ impl Background { }); let weak = Arc::downgrade(&shared); let owner = Arc::clone(&shared); - crate::task::spawn("onestore-background", move || async move { + let thread = crate::task::spawn("onestore-background", move || async move { let signal = &owner.signal; let mut bound: Option<(B, L)> = None; let mut news = false; @@ -295,7 +295,7 @@ impl Background { } } })?; - Ok(Self(shared)) + Ok(Self(shared, Mutex::new(Some(thread)))) } /// Keeps the sections of a notebook on a share in sync while they are not open, with an @@ -448,6 +448,21 @@ impl Background { self.0.signal.wake(); } + /// Stops for good, waiting for the step in flight, so that no replica stays open and no + /// watch holds the notebook's folder. + pub fn stop(&self) { + self.0.signal.stopped.store(true, Ordering::Release); + self.0.signal.wake(); + let thread = self.1.lock().ok().and_then(|mut thread| thread.take()); + // In the browser no step is in flight while another task runs. + #[cfg(not(target_arch = "wasm32"))] + if let Some(thread) = thread { + let _ = thread.join(); + } + #[cfg(target_arch = "wasm32")] + drop(thread); + } + /// Working offline, nothing is checked until `wake`, or until working online again, which /// lists every folder again. pub fn set_offline(&self, offline: bool) { diff --git a/crates/notebook/src/session.rs b/crates/notebook/src/session.rs index 90e4cda27b0385cd206331fad3d25ea8d799d67c..0886810517e89deb59c3f592018397696073fc4c 100644 --- a/crates/notebook/src/session.rs +++ b/crates/notebook/src/session.rs @@ -92,6 +92,9 @@ pub trait Storage: Send + Sync { fn hide(&self, path: &str) -> Result<()>; /// Renames or moves a file or directory; an existing target is an error. fn rename(&self, from: &str, to: &str) -> Result<()>; + /// Renames the notebook's own folder to `name` beside it once no other writer holds any + /// of `files`; the location it then has. + fn rename_root(&self, name: &str, files: &[String]) -> Result; /// Renames a file over another, replacing it. fn replace(&self, from: &str, to: &str) -> Result<()>; /// Deletes a file or an empty directory. @@ -211,6 +214,18 @@ impl Storage for Directory { Ok(fs::rename(self.path(from), self.path(to))?) } + /// A local file system shows no other writer's hold; one that refuses to rename a folder + /// whose files are open says so as the rename fails. + fn rename_root(&self, name: &str, _: &[String]) -> Result { + let to = self.0.with_file_name(name); + // A change of case alone finds the folder itself on a case-insensitive volume. + if fs::metadata(&to).is_ok() && fs::canonicalize(&to)? != self.0 { + return Err(io::Error::from(io::ErrorKind::AlreadyExists).into()); + } + fs::rename(&self.0, &to)?; + Ok(to.to_string_lossy().into_owned()) + } + fn replace(&self, from: &str, to: &str) -> Result<()> { Ok(fs::rename(self.path(from), self.path(to))?) } @@ -306,6 +321,29 @@ impl Storage for Share { Ok(self.client.rename(&self.path(from), &self.path(to))?) } + fn rename_root(&self, name: &str, files: &[String]) -> Result { + if self.root.is_empty() { + return Err(io::Error::from(io::ErrorKind::InvalidInput).into()); + } + for file in files { + self.client.unheld(&self.path(file))?; + } + let (parent, _) = split(&self.root); + let taken = self + .client + .read_dir(parent, LIMITS.entries)? + .iter() + .any(|entry| { + entry.name.eq_ignore_ascii_case(name) && entry.name != split(&self.root).1 + }); + if taken { + return Err(io::Error::from(io::ErrorKind::AlreadyExists).into()); + } + let to = catalog_path(parent, name); + self.client.rename(&self.root, &to)?; + Ok(self.client.location(&to)) + } + fn replace(&self, from: &str, to: &str) -> Result<()> { Ok(self.client.replace(&self.path(from), &self.path(to))?) } @@ -408,13 +446,8 @@ impl Notebook { cache: impl AsRef, ) -> Result { let cache = cache.as_ref().to_path_buf(); - let listings = cache.join("listings"); - fs::create_dir_all(&listings)?; - let name: String = ::digest(storage.location())[..16] - .iter() - .map(|byte| format!("{byte:02x}")) - .collect(); - let listing = listings.join(format!("{name}.json")); + let listing = listing(&cache, &storage.location()); + fs::create_dir_all(listing.parent().unwrap_or(&cache))?; let mut read = fs::read(&listing) .ok() .and_then(|bytes| serde_json::from_slice(&bytes).ok()) @@ -738,6 +771,45 @@ impl Notebook { Ok(renamed) } + /// Renames the notebook's folder to `name`, as OneNote 2010 finds a notebook folder renamed + /// outside it: no file inside changes, and it opens the folder again by its new name. Refused, + /// `WouldBlock`, while another writer holds one of its sections or tables of contents, and + /// `AlreadyExists` where `name` is taken. The replicas and the catalog's listing follow, so + /// edits waiting publish to the renamed folder; every replica must be closed. Returns the + /// notebook's new location, as `crate::location` names it. + pub fn rename_folder(mut self, name: &str) -> Result { + if !folder_name(name) { + return Err(io::Error::from(io::ErrorKind::InvalidInput).into()); + } + let from = self.storage.location(); + let files: Vec = (self.catalog.folders()) + .flat_map(|folder| { + let toc = folder.toc.as_ref().map(|toc| &toc.filename); + (toc.map(|toc| catalog_path(&folder.path, toc)).into_iter()) + .chain(folder.sections.iter().map(|section| section.path.clone())) + }) + .collect(); + let to = self.storage.rename_root(name, &files)?; + let mut moves = vec![(from.clone(), to.clone())]; + moves.extend( + (self.catalog.sections()) + .filter(|section| section.copy) + .map(|section| { + ( + replica_location(&from, section), + replica_location(&to, section), + ) + }), + ); + for (from, to) in moves { + crate::location::moved(&self.cache, &from, &to)?; + } + let _ = fs::remove_file(&self.listing); + self.listing = listing(&self.cache, &to); + self.keep_listing(); + Ok(to) + } + /// Sets a section's colour (COLORREF) in its own metadata, where OneNote keeps it. pub fn set_section_color(&mut self, path: &str, color: Option) -> Result<()> { let path = self.section_path(path)?.path.clone(); @@ -1678,6 +1750,24 @@ pub(crate) fn lists(image: &[u8], file: [u8; 16]) -> Result { })) } +/// Where the cache keeps what reading the catalog of the notebook at `location` took. +fn listing(cache: &Path, location: &str) -> PathBuf { + let name: String = ::digest(location)[..16] + .iter() + .map(|byte| format!("{byte:02x}")) + .collect(); + cache.join("listings").join(format!("{name}.json")) +} + +/// Whether `name` can name a folder on every system a notebook's readers use, Windows's +/// included. +fn folder_name(name: &str) -> bool { + component(name) + && !name.contains(['<', '>', ':', '"', '|', '?', '*']) + && !name.chars().any(char::is_control) + && !name.ends_with(['.', ' ']) +} + fn split(path: &str) -> (&str, &str) { match path.rsplit_once('/') { Some((folder, name)) => (folder, name), diff --git a/crates/notebook/src/session/tests.rs b/crates/notebook/src/session/tests.rs index 7559087d22808d42cf4e16069d354a7b1f5501eb..af7fb86ffd3ec07d7eeb8547c8ca13929bec8769 100644 --- a/crates/notebook/src/session/tests.rs +++ b/crates/notebook/src/session/tests.rs @@ -49,6 +49,9 @@ impl Storage for Racing { fn rename(&self, from: &str, to: &str) -> Result<()> { self.inner.rename(from, to) } + fn rename_root(&self, name: &str, files: &[String]) -> Result { + self.inner.rename_root(name, files) + } fn replace(&self, from: &str, to: &str) -> Result<()> { self.inner.replace(from, to) } diff --git a/crates/notebook/src/sidecar/tests.rs b/crates/notebook/src/sidecar/tests.rs index a5d3d72c08955711f659846212ad10acbec053b9..0baf9b10e578b1533a87aea6e8361521f62814f7 100644 --- a/crates/notebook/src/sidecar/tests.rs +++ b/crates/notebook/src/sidecar/tests.rs @@ -139,6 +139,10 @@ impl Storage for Folder { Ok(()) } + fn rename_root(&self, _: &str, _: &[String]) -> Result { + unreachable!() + } + fn replace(&self, from: &str, to: &str) -> Result<()> { let mut files = self.files.lock().unwrap(); let bytes = files.remove(from).ok_or_else(missing)?; diff --git a/crates/notebook/src/smb/mod.rs b/crates/notebook/src/smb/mod.rs index a290414060cf65ce538c6ec8023bf5057bc7b098..a5ca48a740bd257b1905793625813b357f67186a 100644 --- a/crates/notebook/src/smb/mod.rs +++ b/crates/notebook/src/smb/mod.rs @@ -329,6 +329,12 @@ impl Client { file.close() } + /// Takes and lets go of OneNote 2010's writer opening and locks on the file at `path`, as a + /// commit does; `WouldBlock` while another writer holds them. + pub(crate) fn unheld(&self, path: &str) -> io::Result<()> { + self.open(path, true)?.coordinate(path, true, &[])?.close() + } + /// Gives a file or directory the hidden attribute, keeping its others, as OneNote 2010 /// skips a hidden folder. pub(crate) fn hide(&self, path: &str) -> io::Result<()> { diff --git a/crates/notebook/src/smb/tests.rs b/crates/notebook/src/smb/tests.rs index 9ec2e446c8516c7c468f7e9940ab1b95631c4853..d2bb9a6a4bcdfcc2666e183476a5aa7c1d593228 100644 --- a/crates/notebook/src/smb/tests.rs +++ b/crates/notebook/src/smb/tests.rs @@ -835,6 +835,72 @@ fn live_structure() { client.delete(&root).unwrap(); } +/// Renames a notebook folder of its own on the share, signing in as `ONESTORE_SMB_LAB_USER` +/// where set: refused while another client holds OneNote's writer locks on a section, then +/// done with every file as it was. +#[test] +#[ignore = "requires an owned Samba share at ONESTORE_SMB_LAB"] +fn live_folder_rename() { + use crate::session::Notebook; + let (user, password) = ( + std::env::var("ONESTORE_SMB_LAB_USER").unwrap_or_default(), + std::env::var("ONESTORE_SMB_LAB_PASSWORD").unwrap_or_default(), + ); + let connect = || { + let credentials = Credentials { + username: &user, + password: &password, + domain: "", + }; + let address = std::env::var("ONESTORE_SMB_LAB").unwrap(); + Client::connect(&address, "agent", credentials, Duration::from_secs(10)).unwrap() + }; + let (client, other) = (std::sync::Arc::new(connect()), connect()); + let parent = format!("rename-{}", std::process::id()); + let (root, renamed) = (format!("{parent}/Before"), format!("{parent}/After")); + client.create_directory(&parent).unwrap(); + client.create_directory(&root).unwrap(); + let section = onestore::create_section("First.one", "First page", "Author").unwrap(); + client + .create(&format!("{root}/First.one"), §ion) + .unwrap(); + let id = onestore::Store::parse(§ion).unwrap().header.file_id; + let toc = + onestore::create_table_of_contents("Open Notebook.onetoc2", &[("First.one", id)]).unwrap(); + client + .create(&format!("{root}/Open Notebook.onetoc2"), &toc) + .unwrap(); + let cache = tempfile::tempdir().unwrap(); + let open = || Notebook::open_smb(std::sync::Arc::clone(&client), &root, cache.path()).unwrap(); + let held = format!("{root}/First.one"); + let writer = other + .open(&held, true) + .unwrap() + .coordinate(&held, true, &[]) + .unwrap(); + let refused = open().rename_folder("After").unwrap_err(); + assert!( + matches!(&refused, crate::Error::Io(error) if error.kind() == io::ErrorKind::WouldBlock), + "{refused}" + ); + assert!(client.read_dir(&root, 10).is_ok()); + writer.close().unwrap(); + let to = open().rename_folder("After").unwrap(); + assert_eq!(to, client.location(&renamed)); + assert!(client.read_dir(&root, 10).is_err()); + assert_eq!( + client + .read_storage(&format!("{renamed}/First.one"), 1 << 20) + .unwrap(), + section + ); + for path in ["First.one", "Open Notebook.onetoc2"] { + client.delete(&format!("{renamed}/{path}")).unwrap(); + } + client.delete(&renamed).unwrap(); + client.delete(&parent).unwrap(); +} + /// Signs in as `ONESTORE_SMB_LAB_USER` with `ONESTORE_SMB_LAB_PASSWORD` where set, else as a /// guest, to a server with a share `agent`; only lists and reads. #[test] diff --git a/crates/notebook/tests/location.rs b/crates/notebook/tests/location.rs index 76d0e2e1c0aeb984e171155e8d5b150241f78ca5..81f9cb33b4fef70cd1a593336e6838898ae59159 100644 --- a/crates/notebook/tests/location.rs +++ b/crates/notebook/tests/location.rs @@ -167,6 +167,59 @@ fn a_notebook_the_app_moves_keeps_its_queued_edits() { assert_eq!(published(&root, &cache), 0); } +#[test] +fn renaming_a_notebook_folder_keeps_its_queued_edits_and_files() { + let directory = tempfile::tempdir().unwrap(); + let cache = directory.path().join("cache"); + let root = notebook(directory.path(), "Notebook"); + let space = queued(&root, &cache, "Queued "); + let before = std::fs::read(root.join("Second.one")).unwrap(); + let renamed = directory.path().join("Renamed"); + let to = Notebook::open(&root, &cache) + .unwrap() + .rename_folder("Renamed") + .unwrap(); + assert_eq!(to, notebook::location::local(&renamed).unwrap()); + assert!(std::fs::metadata(&root).is_err()); + // OneNote finds nothing inside changed. + assert_eq!(std::fs::read(renamed.join("Second.one")).unwrap(), before); + // A new notebook where the old one was takes nothing of the queue. + notebook(directory.path(), "Notebook"); + assert_eq!(published(&renamed, &cache), 1); + assert!(stored(&renamed.join("First.one"), space).starts_with("Queued ")); + assert_eq!(published(&root, &cache), 0); +} + +#[test] +fn a_notebook_folder_rename_refuses_a_taken_or_unportable_name_and_changes_nothing() { + let directory = tempfile::tempdir().unwrap(); + let cache = directory.path().join("cache"); + let root = notebook(directory.path(), "Notebook"); + notebook(directory.path(), "Taken"); + for (name, kind) in [ + ("Taken", std::io::ErrorKind::AlreadyExists), + ("Notes: 2026", std::io::ErrorKind::InvalidInput), + ("a/b", std::io::ErrorKind::InvalidInput), + ("Trailing.", std::io::ErrorKind::InvalidInput), + ] { + let error = Notebook::open(&root, &cache) + .unwrap() + .rename_folder(name) + .unwrap_err(); + assert!( + matches!(&error, notebook::Error::Io(error) if error.kind() == kind), + "{name}: {error}" + ); + assert!(std::fs::metadata(root.join("First.one")).is_ok()); + } + // A change of case alone renames the folder itself. + let to = Notebook::open(&root, &cache) + .unwrap() + .rename_folder("NOTEBOOK") + .unwrap(); + assert!(to.ends_with("NOTEBOOK")); +} + #[test] fn a_notebook_moved_outside_the_app_takes_its_replicas_along() { let directory = tempfile::tempdir().unwrap(); diff --git a/crates/snowbound/src/library.rs b/crates/snowbound/src/library.rs index 89fb89ad48b89b1a6d0071b90272f0ad0608dbf8..a30f2453931d09468ee97de748b85df880c0aadb 100644 --- a/crates/snowbound/src/library.rs +++ b/crates/snowbound/src/library.rs @@ -452,12 +452,76 @@ impl Library { /// Names the notebook `name` on this computer, leaving its folder as it is, as OneNote /// 2010's Notebook Properties does; the notebook read again with `with` shows it. pub fn set_display_name(&self, name: &str) -> io::Result<()> { - let file = display_names(&self.cache); - let mut names = read_display_names(&file); - names.insert(self.location.clone(), name.to_owned()); - let partial = file.with_extension("partial"); - notebook::fs::write(&partial, serde_json::to_vec_pretty(&names)?)?; - notebook::fs::rename(partial, file) + edit_display_names(&self.cache, |names| { + names.insert(self.location.clone(), name.to_owned()); + }) + } + + /// Where the notebook would be with its folder named `name`; none for a section opened on + /// its own, or a notebook at the top of its share. + pub fn renamed_location(&self, name: &str) -> Option { + self.catalog()?; + if let Some(mut mount) = server_address(&self.location) { + let (parent, _) = mount.root.rsplit_once('/').unwrap_or(("", &mount.root)); + mount.root = match (parent, mount.root.is_empty()) { + (_, true) => return None, + ("", false) => name.to_owned(), + (parent, false) => format!("{parent}/{name}"), + }; + return Some(mount.url()); + } + let parent = Path::new(&self.location).parent()?; + Some(parent.join(name).to_string_lossy().into_owned()) + } + + /// Renames the notebook's folder to `name` once nothing of this notebook holds its files: + /// its background stopped and its kept sections closed, as the open section must be too. + /// The replicas, and edits waiting in them, follow, and the folder's own name takes the + /// place of a display name. Refused, with nothing changed, while another writer holds a file + /// of it. Returns the notebook's new location, where it opens again, or why not, as its + /// reader is told. + pub fn rename_folder(&self, name: &str) -> Result { + let to = self + .renamed_location(name) + .ok_or("This notebook’s folder can’t be renamed.")?; + if let Some(background) = &self.background { + background.stop(); + } + self.close_kept(); + let renamed = self + .reopen() + .and_then(|notebook| Ok(notebook.rename_folder(name)?)); + if let Err(error) = renamed { + let kind = match error.downcast_ref::() { + Some(notebook::Error::Io(error) | notebook::Error::RemoteIo(error)) => { + Some(error.kind()) + } + _ => error.downcast_ref::().map(io::Error::kind), + }; + return Err(match kind { + Some(io::ErrorKind::WouldBlock | io::ErrorKind::ResourceBusy) => { + "Another computer is saving to this notebook. Try again in a moment.".into() + } + Some(io::ErrorKind::PermissionDenied) => "Its files are open on another \ + computer, or you can’t rename folders there. Close the notebook in OneNote \ + on other computers, then try again." + .into(), + Some(io::ErrorKind::AlreadyExists) => { + format!("A folder named “{name}” is already there. Choose another name.") + } + Some(io::ErrorKind::InvalidInput) => "A folder name can’t contain \\ / : * ? \" \ + < > | or end with a dot or space." + .into(), + _ => error.to_string(), + }); + } + // The folder's own name shows now. + if let Err(error) = edit_display_names(&self.cache, |names| { + names.remove(&self.location); + }) { + eprintln!("{}: {error}", self.location); + } + Ok(to) } /// The art the notebook's tags draw with. @@ -795,6 +859,11 @@ impl Library { .then(|| Path::new(&self.location)) } + /// Whether the notebook is on an SMB share, however it is reached. + pub fn on_smb(&self) -> bool { + self.server.is_some() || server_address(&self.location).is_some() || self.notice.is_some() + } + /// Whether iCloud Drive keeps the notebook, or the section opened on its own. pub fn in_icloud(&self) -> bool { self.folder().is_some_and(crate::icloud::ubiquitous) @@ -1038,6 +1107,18 @@ fn read_display_names(file: &Path) -> std::collections::BTreeMap .unwrap_or_default() } +fn edit_display_names( + cache: &Path, + edit: impl FnOnce(&mut std::collections::BTreeMap), +) -> io::Result<()> { + let file = display_names(cache); + let mut names = read_display_names(&file); + edit(&mut names); + let partial = file.with_extension("partial"); + notebook::fs::write(&partial, serde_json::to_vec_pretty(&names)?)?; + notebook::fs::rename(partial, file) +} + fn display_name(cache: &Path, location: &str) -> Option { read_display_names(&display_names(cache)).remove(location) } @@ -1428,6 +1509,133 @@ mod tests { } } + /// Renames a notebook folder on the lab share (`ONESTORE_SMB_LAB`, as above) with an edit + /// queued offline, which publishes into the renamed folder; `notebook`'s + /// `live_folder_rename` covers the refusal while another writer holds a section. + #[test] + #[ignore = "requires an owned Samba share at ONESTORE_SMB_LAB"] + fn a_notebook_folder_on_a_share_renames_with_its_queue() { + let address = std::env::var("ONESTORE_SMB_LAB").unwrap(); + let parent = format!("snowbound-rename-{}", std::process::id()); + let user = std::env::var("ONESTORE_SMB_LAB_USER").ok(); + let mount = |root: &str| Mount { + server: address.clone(), + share: "agent".into(), + user: user.clone(), + domain: String::new(), + root: format!("{parent}/{root}"), + }; + let login = Login { + user: user.clone().unwrap_or_default(), + password: std::env::var("ONESTORE_SMB_LAB_PASSWORD").unwrap_or_default(), + domain: String::new(), + }; + let connect = || { + Server { + mount: mount("Before"), + login: login.clone(), + } + .connect() + .unwrap() + }; + let client = Arc::new(connect()); + remove_tree(&client, &parent); + client.create_directory(&parent).unwrap(); + client.create_directory(&mount("Before").root).unwrap(); + let cache = std::env::temp_dir().join(&parent); + let page = onestore::PageCreation::new(None, Some(""), "Rust Author").unwrap(); + Notebook::open_smb(Arc::clone(&client), &mount("Before").root, &cache) + .unwrap() + .create_section("", "Queued", &page) + .unwrap(); + let open = |root: &str| { + let location = mount(root).url(); + Library::on_share(&location, mount(root), login.clone(), &cache).unwrap() + }; + let library = open("Before"); + let section = library.open("Queued.one", || {}).unwrap(); + section.set_offline(true); + let (space, ..) = section.pages().unwrap()[0].clone(); + let title = (section.page(space).unwrap().objects.iter()) + .find_map(|object| match object { + onestore::page::PageObject::Title(title) => { + title.outlines[0].paragraphs[0].text().map(|text| text.id) + } + _ => None, + }) + .unwrap(); + let typed = onestore::op::Edit { + at: crate::filetime(), + ops: vec![onestore::op::Op::Page { + space, + op: onestore::op::PageOp::Text { + text: title, + range: 0..0, + with: "Renamed over SMB".into(), + }, + }], + }; + section.apply("Rust Author", typed).unwrap(); + section.close().unwrap(); + assert_eq!( + library.rename_folder("After").unwrap(), + mount("After").url() + ); + assert!(client.read_dir(&mount("Before").root, 10).is_err()); + let renamed = open("After"); + let section = renamed.open("Queued.one", || {}).unwrap(); + let file = format!("{}/Queued.one", mount("After").root); + let deadline = std::time::Instant::now() + Duration::from_secs(60); + loop { + let titles = client + .read_storage(&file, LIMIT) + .ok() + .and_then(|bytes| { + let arena = onestore::Arena::default(); + onestore::Section::open(&arena, bytes).ok()?.pages().ok() + }) + .unwrap_or_default(); + if titles + .iter() + .any(|(_, title, _)| title == "Renamed over SMB") + { + break; + } + assert!(std::time::Instant::now() < deadline, "{titles:?}"); + section.wake(); + std::thread::sleep(Duration::from_millis(200)); + } + section.close().unwrap(); + drop((library, renamed)); + let _ = notebook::fs::remove_dir_all(&cache); + remove_tree(&client, &parent); + } + + #[test] + fn a_notebook_folder_renames_unless_its_name_is_taken() { + let directory = + std::env::temp_dir().join(format!("snowbound-rename-{}", std::process::id())); + let cache = directory.join("cache"); + let page = onestore::PageCreation::new(None, Some(""), "Author").unwrap(); + notebook::fs::create_dir_all(&directory).unwrap(); + for name in ["Mine", "Taken"] { + Notebook::create(directory.join(name), &cache, Notebook::NEW_COLOR, &page).unwrap(); + } + let location = |name: &str| directory.join(name).to_string_lossy().into_owned(); + let library = Library::notebook(&location("Mine"), &cache); + library.set_display_name("Shown").unwrap(); + let refused = library.rename_folder("Taken").unwrap_err(); + assert!(refused.starts_with("A folder named “Taken”"), "{refused}"); + let library = Library::notebook(&location("Mine"), &cache); + assert_eq!(library.name, "Shown"); + assert_eq!(library.rename_folder("Ours").unwrap(), location("Ours")); + assert!(notebook::fs::metadata(location("Mine")).is_err()); + let renamed = Library::notebook(&location("Ours"), &cache); + assert_eq!(renamed.name, "Ours"); + assert_eq!(renamed.color(), Some(Notebook::NEW_COLOR)); + notebook::fs::remove_dir_all(&directory).unwrap(); + } + #[test] fn share_relative_sections_show_under_their_mount() { let library = |root: &str| Library { diff --git a/crates/snowbound/src/main.rs b/crates/snowbound/src/main.rs index b15b401fe89c227f5b91c551af59edcaa5ce1fee..ce1f294ad0ac219fd2ae0e4b70e94063cfef2b0d 100644 --- a/crates/snowbound/src/main.rs +++ b/crates/snowbound/src/main.rs @@ -811,6 +811,8 @@ struct State { /// Notebooks in iCloud Drive a thread is reading, or following the download of, by /// location. icloud_reading: HashSet, + /// The notebook locations a folder rename takes a notebook from and to, while it does. + folder_rename: Option<(String, String)>, /// The user's tag list, which the toolbar, menus and Ctrl+1 to Ctrl+9 apply. tags: Vec, /// The Customize Tags dialog's list while it is open. @@ -1197,6 +1199,7 @@ impl State { peers: None, server: None, icloud_reading: HashSet::new(), + folder_rename: None, tags: stored .tags .unwrap_or_else(canvas::editor::NoteTag::defaults), @@ -3160,6 +3163,10 @@ impl State { fn apply(&mut self, command: Command) -> Result<(), Box> { match command { Command::OpenSection(library, path) => { + // Its files are on their way to the renamed folder. + if self.folder_renaming(&library.location) { + return Ok(()); + } if library.locked(&path) { return self.show_locked(library, path); } diff --git a/crates/snowbound/src/manage.rs b/crates/snowbound/src/manage.rs index bccb6087d49318d55210df6d7845a9420db80480..4c5d8964e20de98fd66cc741dde01a31ef8d8374 100644 --- a/crates/snowbound/src/manage.rs +++ b/crates/snowbound/src/manage.rs @@ -298,7 +298,11 @@ impl State { .notebooks .iter() .filter(|library| { - in_icloud_folder(&library.location) && !listed.contains(&library.location) + in_icloud_folder(&library.location) + && !listed.contains(&library.location) + && !self.folder_renaming(&library.location) + // A listing taken before a rename names the folder by its old name. + && notebook::fs::metadata(&library.location).is_err() }) .cloned() .collect(); @@ -307,16 +311,21 @@ impl State { } for location in listed { if self.notebooks.iter().any(|open| open.location == location) + || self.folder_renaming(&location) || !self.icloud_reading.insert(location.clone()) { continue; } let (cache, proxy) = (self.cache.clone(), self.proxy.clone()); crate::spawn(move || { + let gone = notebook::fs::metadata(&location).is_err(); let library = Arc::new(Library::notebook(&location, &cache)); let _ = proxy.send_event(crate::UserEvent::Then(Box::new(move |state| { state.icloud_reading.remove(&location); - if state.notebooks.iter().any(|open| open.location == location) { + if gone + || state.notebooks.iter().any(|open| open.location == location) + || state.folder_renaming(&location) + { return Ok(()); } state.notebooks.push(Arc::clone(&library)); @@ -329,6 +338,13 @@ impl State { } } + /// Whether a folder rename is taking the notebook from or to `location`. + pub(crate) fn folder_renaming(&self, location: &str) -> bool { + self.folder_rename + .as_ref() + .is_some_and(|(from, to)| from == location || to == location) + } + /// Shows `library`, a notebook that just arrived or whose sections did, where no other /// notebook is shown: at its first section, or without one while none is here yet. pub(crate) fn show_arrived(&mut self, library: &Arc) { @@ -502,6 +518,121 @@ impl State { self.save_settings(); } + /// Renames `library`'s folder to `name` on a thread of its own, the notebook closed + /// meanwhile, then opens it from there in its place, at the section it showed, with `color` + /// given it. What this computer keeps by the notebook's location follows it. Refused, the + /// notebook opens again as it was. + pub(crate) fn rename_notebook( + &mut self, + library: Arc, + name: String, + color: Option, + ) { + let Some(to) = library.renamed_location(&name) else { + return; + }; + let ours = |shown: &Arc| Arc::ptr_eq(shown, &library); + let shown = self + .session + .as_ref() + .filter(|session| ours(&session.library)) + .map(|session| session.tabs[session.tab].path.clone()); + if shown.is_some() { + let closed = self.persist().and_then(|()| match self.session.take() { + Some(session) => Ok(session.section.close()?), + None => Ok(()), + }); + if let Err(error) = closed { + return platform::alert("Couldn't rename the folder", &error.to_string()); + } + } + let showing = shown.is_some() || self.sectionless.as_ref().is_some_and(ours); + if showing { + self.sectionless = Some(Arc::clone(&library)); + self.title(); + } + self.folder_rename = Some((library.location.clone(), to.clone())); + let (cache, proxy) = (self.cache.clone(), self.proxy.clone()); + crate::spawn(move || { + let renamed = library.rename_folder(&name); + // A failure after the folder moved, as its replicas followed, leaves it there. + let moved = library + .folder() + .is_some_and(|folder| notebook::fs::metadata(folder).is_err()); + let location = match &renamed { + Ok(to) => to, + Err(_) if moved => &to, + Err(_) => &library.location, + }; + let mut reopened = Library::notebook(location, &cache); + let mut problem = renamed.err(); + if let (None, Some(color)) = (&problem, color) { + let colored = reopened.reopen().and_then(|mut notebook| { + notebook.set_color(color)?; + Ok(reopened.with(notebook)) + }); + match colored { + Ok(colored) => reopened = colored, + Err(error) => problem = Some(error.to_string()), + } + } + let reopened = Arc::new(reopened); + let _ = proxy.send_event(crate::UserEvent::Then(Box::new(move |state| { + state.renamed_notebook(&library, reopened, showing.then_some(shown)); + if let Some(problem) = problem { + platform::alert("Couldn't rename the folder", &problem); + } + Ok(()) + }))); + }); + } + + /// Lists `reopened` in the place of `old`, the notebook `rename_notebook` closed, moving + /// what this computer keeps by its location, and shows it again where `shown`: at the + /// section it showed, or its first. + fn renamed_notebook( + &mut self, + old: &Arc, + reopened: Arc, + shown: Option>, + ) { + self.folder_rename = None; + let (from, to) = (old.location.clone(), reopened.location.clone()); + match (self.notebooks.iter_mut()).find(|open| Arc::ptr_eq(open, old)) { + Some(open) => *open = Arc::clone(&reopened), + None => self.notebooks.push(Arc::clone(&reopened)), + } + // iCloud Drive's listing may have found the renamed folder first. + let mut listed = false; + self.notebooks + .retain(|open| open.location != to || !std::mem::replace(&mut listed, true)); + if from != to { + self.undo.close(&from); + self.trail.moved(&from, &to); + self.reads.moved(&from, &to); + let (from, to) = (crate::library::key(&from, ""), crate::library::key(&to, "")); + let rekey = |key: &String| match key.strip_prefix(&from) { + Some(rest) => format!("{to}{rest}"), + None => key.clone(), + }; + self.folded = self.folded.iter().map(rekey).collect(); + self.last_pages = (self.last_pages.iter()) + .map(|(key, space)| (rekey(key), *space)) + .collect(); + } + if let Some(shown) = shown { + self.sectionless = Some(Arc::clone(&reopened)); + self.title(); + if let Some(path) = shown + .filter(|path| reopened.contains(path)) + .or_else(|| reopened.first_section()) + { + self.commands.push(Command::OpenSection(reopened, path)); + } + } + self.save_settings(); + } + /// Changes `library`'s sections and groups on a thread of its own, then shows the /// section the change leaves open: a new or moved one, or the one shown before. pub(crate) fn restructure(&mut self, library: Arc, change: Structure) { diff --git a/crates/snowbound/src/navigation.rs b/crates/snowbound/src/navigation.rs index 843cc45ec1bdb1f773a001f1e43070019e8aa0fe..b37c3f008bc80dff941c1e8fb4aa0b17e591271e 100644 --- a/crates/snowbound/src/navigation.rs +++ b/crates/snowbound/src/navigation.rs @@ -31,6 +31,17 @@ const KEPT: usize = 100; const RECENT: usize = 8; impl Trail { + /// Follows the notebook at `from` to `to`, where it moved. + pub fn moved(&mut self, from: &str, to: &str) { + let places = (self.back.iter_mut()) + .chain(&mut self.here) + .chain(&mut self.forward) + .chain(&mut self.recent); + for place in places.filter(|place| place.notebook == from) { + place.notebook = to.to_owned(); + } + } + /// Notes `place` shown. Arriving anywhere but where Back or Forward went drops the pages /// ahead, as a browser does. pub fn visit(&mut self, place: Place) { diff --git a/crates/snowbound/src/properties.rs b/crates/snowbound/src/properties.rs index c6fb26081a46655fdaace7b0023a9604dd07bad2..f342415fbd79b2b53d857b3f74f203653c140bdb 100644 --- a/crates/snowbound/src/properties.rs +++ b/crates/snowbound/src/properties.rs @@ -15,6 +15,8 @@ pub struct Properties { name: String, /// A colour picked, COLORREF. color: Option, + /// Renames the notebook's folder to a name changed. + rename_folder: bool, } fn id() -> Id { @@ -37,6 +39,7 @@ impl State { name: library.name.clone(), library, color: None, + rename_folder: true, }); self.ui.open_popup(id()); self.ui.focus_all(name_field()); @@ -126,7 +129,28 @@ impl State { if let Some(node) = ui.access(name_field()) { node.set_label("Display name"); } - dim(ui, "hint", "Doesn’t change the notebook’s folder name"); + let renamable = dialog.name.trim() != dialog.library.name + && (dialog.library.renamed_location(dialog.name.trim())).is_some(); + if renamable { + let rename = ui::check_box(ui, "rename", "Rename the folder too", dialog.rename_folder); + if rename.clicked { + dialog.rename_folder = !dialog.rename_folder; + } + if dialog.rename_folder && dialog.library.on_smb() { + ui.leaf( + "shared", + Spec { + flags: ui::Flags::CLIP, + size: [fill(), px(row * 0.8)], + text: Some("Other computers must reopen it from the new folder"), + icon: Some(art::WARNING), + ..Spec::default() + }, + ); + } + } else { + dim(ui, "hint", "Doesn’t change the notebook’s folder name"); + } label(ui, "Color:"); let shown = dialog.color.or(dialog.library.color()); let named = SECTION_COLORS @@ -161,6 +185,13 @@ impl State { ui.close(); ui.close(); let name = dialog.name.trim(); + if ok && !name.is_empty() && renamable && dialog.rename_folder { + let (library, name, color) = + (Arc::clone(&dialog.library), name.to_owned(), dialog.color); + self.ui.close_popup(id()); + self.properties = None; + return self.rename_notebook(library, name, color); + } if ok && !name.is_empty() { if name != dialog.library.name || dialog.color.is_some() { self.commands.push(Command::Structure( diff --git a/crates/snowbound/src/sidebar.rs b/crates/snowbound/src/sidebar.rs index 6157f96272572afcbfdd8d0324dfc2d0dbe398cd..ebd1eb7057ea8d15956921b4c11bec44afb1734d 100644 --- a/crates/snowbound/src/sidebar.rs +++ b/crates/snowbound/src/sidebar.rs @@ -1231,6 +1231,7 @@ impl crate::State { /// it, with a button to add one; or while none is on this computer yet. pub(crate) fn no_sections(&mut self, library: Arc) { let downloading = library.downloading(); + let renaming = self.folder_renaming(&library.location); let new = crate::Command::Structure( library, crate::manage::Structure::NewSection { @@ -1238,7 +1239,9 @@ impl crate::State { }, ); let id = self.ui.id("no sections"); - let (title, buttons) = if downloading { + let (title, buttons) = if renaming { + ("Renaming the folder…", Vec::new()) + } else if downloading { ("Downloading from iCloud Drive…", Vec::new()) } else { ( diff --git a/crates/snowbound/src/unread.rs b/crates/snowbound/src/unread.rs index 2b1043c8e9928a745c671561dfc42604b5a96131..0f207e96e5212b02b918cdf039686681ee414995 100644 --- a/crates/snowbound/src/unread.rs +++ b/crates/snowbound/src/unread.rs @@ -91,6 +91,14 @@ impl Reads { }) } + /// Keeps what was read in the notebook at `from` for it at `to`, where it moved. + pub fn moved(&mut self, from: &str, to: &str) { + if let Some(kept) = self.notebooks.remove(from) { + self.notebooks.insert(to.to_owned(), kept); + self.save(); + } + } + /// Whether Show Unread Changes is on for the notebook at `notebook`. pub fn shown(&self, notebook: &str) -> bool { self.notebooks.get(notebook).is_none_or(|kept| !kept.hidden) -- 2.54.0