diff --git a/Cargo.lock b/Cargo.lock index 087c81c4505800a98784df46b085e5c2f2ef4e46..89343ba90781022fc673ee0c101e64f5c4ad9c5f 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -3627,6 +3627,7 @@ dependencies = [ "accesskit_consumer", "accesskit_winit", "arboard", + "async-lock", "base64", "block2 0.5.1", "canvas", diff --git a/crates/notebook/src/session.rs b/crates/notebook/src/session.rs index 5d0600ca57ccc2c1dadf774ff4fe43b6c8a4371d..9bfc62d2bf3d0cb9dbf18626745f85fd08c2b7e5 100644 --- a/crates/notebook/src/session.rs +++ b/crates/notebook/src/session.rs @@ -2139,10 +2139,30 @@ impl Section { } }); let observed: Arc> = Arc::new(Mutex::new(None)); - let worker = { - let (sender, notify) = (sender.clone(), Arc::clone(¬ify)); - let observed = Arc::clone(&observed); - replica.start_sync(Duration::from_secs(2), connect, move |result| { + let mut section = Self { + file, + replica, + worker: None, + events: Mutex::new(events), + sender, + notify, + observed, + }; + section.worker = Some(section.start_worker(connect, None)?); + Ok(section) + } + + fn start_worker( + &self, + connect: impl FnMut() -> io::Result + Send + 'static, + replacing: Option<&SyncWorker>, + ) -> Result { + let (sender, notify) = (self.sender.clone(), Arc::clone(&self.notify)); + let observed = Arc::clone(&self.observed); + Ok(self.replica.start_sync_replacing( + Duration::from_secs(2), + connect, + move |result| { let error = result.as_ref().err().map(reached); // Reaching the file as the last attempt did is no news to the host. let mut changed = false; @@ -2182,17 +2202,33 @@ impl Section { { notify(); } - })? - }; - Ok(Self { - file, - replica, - worker: Some(worker), - events: Mutex::new(events), - sender, - notify, - observed, - }) + }, + replacing, + )?) + } + + /// Follows a section file moved within its notebook, keeping its replica and queued edits. + pub fn relocate(&mut self, file: impl AsRef) -> Result<()> { + self.relocate_with(fs::absolute(file)?, |file| Ok(FileRemote(file.to_owned()))) + } + + /// `relocate`, with a host's publication target. Cancellation leaves any in-flight step + /// to finish off-thread; synchronization ownership serializes the replacement worker. + pub fn relocate_with( + &mut self, + file: impl AsRef, + mut connect: impl FnMut(&Path) -> io::Result + Send + 'static, + ) -> Result<()> { + let file = file.as_ref().to_owned(); + let target = file.clone(); + let previous = self.worker.take(); + if let Some(worker) = &previous { + worker.cancel(); + } + self.observed = Arc::new(Mutex::new(None)); + self.worker = Some(self.start_worker(move || connect(&target), previous.as_ref())?); + self.file = file; + Ok(()) } /// The absolute local path, or share-relative path for an SMB session. diff --git a/crates/notebook/src/worker.rs b/crates/notebook/src/worker.rs index df6ae0fd73a40e4c0cb805890ee505de559bb7b6..f978c9eb1b6021f2932456d1aa504fd5f34dc2c0 100644 --- a/crates/notebook/src/worker.rs +++ b/crates/notebook/src/worker.rs @@ -86,6 +86,11 @@ pub struct SyncWorker { } impl SyncWorker { + pub(crate) fn cancel(&self) { + self.signal.stopped.store(true, Ordering::Release); + self.signal.wake(); + } + /// Requests a retry, for example after a network reachability change. Working /// offline, it synchronizes once, as OneNote's Sync Now does. /// A pending contention backoff finishes before processing the notification. @@ -120,8 +125,7 @@ impl SyncWorker { /// A stopped worker leaves pending edits and uncertain attempts in the cache. /// Call outside the worker's own callback, which cannot join its calling thread. pub fn stop(mut self) -> Result<()> { - self.signal.stopped.store(true, Ordering::Release); - self.signal.wake(); + self.cancel(); let thread = self.thread.take().expect("Worker owns its thread"); #[cfg(target_arch = "wasm32")] return thread.finished().unwrap_or(Ok(())); @@ -150,8 +154,7 @@ impl SyncWorker { impl Drop for SyncWorker { fn drop(&mut self) { - self.signal.stopped.store(true, Ordering::Release); - self.signal.wake(); + self.cancel(); } } @@ -166,10 +169,25 @@ impl Replica { /// Cache/document errors stop the worker; inspect them through `observe` or `stop`. /// Remote calls and callbacks must be bounded for `stop` to have bounded latency. pub fn start_sync( + self: &Arc, + interval: Duration, + connect: F, + observe: O, + ) -> io::Result + where + R: Remote + 'static, + F: FnMut() -> io::Result + Send + 'static, + O: FnMut(&Result) + Send + 'static, + { + self.start_sync_replacing(interval, connect, observe, None) + } + + pub(crate) fn start_sync_replacing( self: &Arc, interval: Duration, mut connect: F, mut observe: O, + replacing: Option<&SyncWorker>, ) -> io::Result where R: Remote + 'static, @@ -187,10 +205,24 @@ impl Replica { .worker .lock() .map_err(|_| io::Error::other("Synchronization worker registration panicked"))?; - if owner.upgrade().is_some() { + let previous = owner.upgrade(); + if let Some(previous) = &previous + && !replacing.is_some_and(|worker| { + Arc::ptr_eq(previous, &worker.signal) && previous.stopped.load(Ordering::Acquire) + }) + { return Err(io::ErrorKind::WouldBlock.into()); } let (signal, receiver) = Signal::new(); + if let Some(previous) = previous { + signal + .offline + .store(previous.offline.load(Ordering::Acquire), Ordering::Release); + if let (Ok(previous), Ok(mut next)) = (previous.pause.lock(), signal.pause.lock()) { + next.length = previous.length; + next.waiting = previous.waiting; + } + } let replica = Arc::clone(self); let worker_signal = Arc::clone(&signal); let thread = task::spawn("onestore-sync", move || async move { diff --git a/crates/notebook/tests/session.rs b/crates/notebook/tests/session.rs index 8a20e57391d92fc9d7e631f92ab057c8a08075f5..2ab09726b442a1a266b5d85b27a48e8e1ece5450 100644 --- a/crates/notebook/tests/session.rs +++ b/crates/notebook/tests/session.rs @@ -1218,6 +1218,140 @@ fn every_section_gets_an_offline_copy_that_closing_the_notebook_discards() { /// A section file whose stamp reads are counted. struct Counted(std::path::PathBuf, Arc); +#[test] +fn a_moved_section_keeps_its_replica_while_the_old_remote_is_stalled() { + use std::sync::{Condvar, Mutex, mpsc}; + + struct Stalled { + file: std::path::PathBuf, + source: Vec, + entered: Arc>>>, + release: Arc<(Mutex, Condvar)>, + finished: Option>, + publications: Arc, + late_error: bool, + } + + impl Drop for Stalled { + fn drop(&mut self) { + if let Some(finished) = self.finished.take() { + let _ = finished.send(()); + } + } + } + + impl notebook::Remote for Stalled { + fn read(&mut self) -> std::io::Result> { + Ok(self.source.clone()) + } + + fn stamp(&mut self) -> std::io::Result { + if let Some(entered) = self.entered.lock().unwrap().take() { + entered.send(()).unwrap(); + let (lock, changed) = &*self.release; + let _ = changed + .wait_timeout_while(lock.lock().unwrap(), Duration::from_secs(5), |done| !*done) + .unwrap(); + if self.late_error { + return Err(std::io::ErrorKind::TimedOut.into()); + } + } + onestore::Stamp::of(&self.source).map_err(std::io::Error::other) + } + + fn publish( + &mut self, + transaction: &onestore::Transaction, + ) -> Result<(), onestore::CommitError> { + self.publications.fetch_add(1, Ordering::SeqCst); + transaction.commit_file(&self.file) + } + + fn confirm(&mut self, _: &onestore::Stamp) -> Result<(), onestore::CommitError> { + Ok(()) + } + } + + for late_error in [false, true] { + let directory = tempfile::tempdir().unwrap(); + let file = directory.path().join("Before.one"); + let target = directory.path().join("After.one"); + let source = onestore::create_section("Before.one", "Original", "Author").unwrap(); + std::fs::write(&file, &source).unwrap(); + let (entered, waiting) = mpsc::channel(); + let (connected, connecting) = mpsc::channel(); + let (finished, stopped) = mpsc::channel(); + let begin = Arc::new((Mutex::new(false), Condvar::new())); + let starting = Arc::clone(&begin); + let release = Arc::new((Mutex::new(false), Condvar::new())); + let gate = Arc::clone(&release); + let entered = Arc::new(Mutex::new(Some(entered))); + let publications = Arc::new(AtomicUsize::new(0)); + let attempts = Arc::clone(&publications); + let mut opening = true; + let mut section = Section::open_with( + &file, + directory.path().join("cache"), + move |file| { + let finishing = (!std::mem::replace(&mut opening, false)).then(|| finished.clone()); + if finishing.is_some() { + connected.send(()).unwrap(); + let (lock, changed) = &*starting; + let _ = changed + .wait_timeout_while(lock.lock().unwrap(), Duration::from_secs(5), |done| { + !*done + }) + .unwrap(); + } + Ok(Stalled { + file: file.to_owned(), + source: source.clone(), + entered: Arc::clone(&entered), + release: Arc::clone(&gate), + finished: finishing, + publications: Arc::clone(&attempts), + late_error, + }) + }, + || {}, + ) + .unwrap(); + connecting.recv_timeout(Duration::from_secs(3)).unwrap(); + let replica = Arc::clone(section.replica()); + let (space, ..) = section.pages().unwrap()[0]; + let before = section.page(space).unwrap(); + let edit = typed(§ion, space, &before, 0..0, "Moved "); + *begin.0.lock().unwrap() = true; + begin.1.notify_all(); + waiting.recv_timeout(Duration::from_secs(3)).unwrap(); + section.set_offline(true); + std::fs::rename(&file, &target).unwrap(); + let start = Instant::now(); + section.relocate(&target).unwrap(); + assert!(start.elapsed() < Duration::from_secs(1)); + assert!(Arc::ptr_eq(&replica, section.replica())); + assert_eq!(section.file(), target); + assert_same(section.page(space).unwrap(), &edited(&before, "Moved ")); + *release.0.lock().unwrap() = true; + release.1.notify_all(); + stopped.recv_timeout(Duration::from_secs(3)).unwrap(); + assert_eq!( + publications.load(Ordering::SeqCst), + usize::from(!late_error) + ); + assert!(section.sync_status().unwrap().error.is_none()); + assert!(!matches!( + section.status(edit).unwrap(), + Some(EditStatus::Published { .. }) + )); + section.set_offline(false); + published(§ion, edit); + assert_same(stored_page(&target, space), &edited(&before, "Moved ")); + assert!(!file.exists()); + section.close().unwrap(); + } +} + impl notebook::Remote for Counted { fn read(&mut self) -> std::io::Result> { onestore::read_file(&self.0) diff --git a/crates/snowbound/Cargo.toml b/crates/snowbound/Cargo.toml index 654d63eef495a7d9dea99fc772fa0f8343b57991..4efb2bd455b51cc9456bebba44fb890d56828f5d 100644 --- a/crates/snowbound/Cargo.toml +++ b/crates/snowbound/Cargo.toml @@ -23,6 +23,7 @@ notebook = { path = "../notebook" } serde = { version = "1", features = ["derive"] } serde_json = "1" pollster = "0.4" +async-lock = "3.4.2" wgpu = { workspace = true, optional = true } parley.workspace = true winit = "0.30.13" diff --git a/crates/snowbound/src/library.rs b/crates/snowbound/src/library.rs index 7678482809052655e2d117e301393237f3c6837a..23e0f0cc01d7055fa377de0781591eac0aed2ab3 100644 --- a/crates/snowbound/src/library.rs +++ b/crates/snowbound/src/library.rs @@ -8,6 +8,8 @@ use notebook::sidecar::themes::Themes; use notebook::smb::{Client, Credentials}; use onestore::protected::Key; use std::{ + borrow::Cow, + collections::BTreeMap, error::Error, io, path::{Path, PathBuf}, @@ -324,6 +326,8 @@ pub struct Library { /// Sections opened ahead of being shown, or left open after, most recent first, which /// `open` hands out before opening any. kept: Mutex>, + pub(crate) renames: Arc)>>>, + pub(crate) structure: Arc>, themes: Mutex, /// The keys of its password-protected sections unlocked this run, kept as it is read again. keys: Arc, @@ -359,6 +363,8 @@ impl Library { watch: None, tag_art: Default::default(), kept: Mutex::new(crate::prefetch::Recent::new(KEPT)), + renames: Arc::default(), + structure: Arc::default(), themes: Default::default(), keys: Default::default(), #[cfg(feature = "live")] @@ -796,6 +802,8 @@ impl Library { watch, tag_art: Mutex::new(self.tag_art()), keys: Arc::clone(&self.keys), + structure: Arc::clone(&self.structure), + renames: Arc::clone(&self.renames), #[cfg(feature = "live")] joined: self.joined.clone(), ..Self::new(&self.location, name, &self.cache) @@ -882,6 +890,60 @@ impl Library { } } + /// Follows a moved section without closing its readable replica or waiting for the share. + pub(crate) fn relocate(&self, path: &str, section: &mut Section) -> Result<(), Box> { + #[cfg(feature = "live")] + if let Some(joined) = &self.joined { + let guest = Arc::clone(&joined.guest); + section.relocate_with(path, move |file| { + Ok(notebook::live::share::HostedRemote::new( + &guest, + &file.to_string_lossy(), + )) + })?; + if let Some(background) = &self.background { + background.hold(path, section); + } + return Ok(()); + } + match &self.server { + #[cfg(not(target_arch = "wasm32"))] + Some(server) => { + let file = match server.mount.root.as_str() { + "" => path.to_owned(), + root => format!("{root}/{path}"), + }; + let server = Arc::clone(server); + section.relocate_with(file, move |file| { + Ok(notebook::SmbRemote::new( + server.connect()?, + file.to_string_lossy(), + LIMIT, + )) + })?; + } + #[cfg(target_arch = "wasm32")] + Some(_) => unreachable!("The browser opens no notebook on a server"), + None => { + let file = Path::new(&self.location).join(path); + #[cfg(target_os = "macos")] + if self.in_icloud() { + section.relocate_with(file, |file| { + Ok(crate::icloud::Coordinated(file.to_owned())) + })?; + } else { + section.relocate(file)?; + } + #[cfg(not(target_os = "macos"))] + section.relocate(file)?; + } + } + if let Some(background) = &self.background { + background.hold(path, section); + } + Ok(()) + } + /// Opens a section, waiting natively until `deadline` for its current owner. fn open_by( &self, @@ -1147,6 +1209,35 @@ impl Library { key(&self.location, path) } + /// A committed entry label, replaced by its pending rename until storage answers. + pub(crate) fn label<'a>(&self, path: &str, committed: &'a str) -> Cow<'a, str> { + self.renames + .lock() + .ok() + .and_then(|renames| { + renames + .get(&self.entry_identity(path)?) + .map(|(name, _)| name.clone()) + }) + .map_or(Cow::Borrowed(committed), Cow::Owned) + } + + pub(crate) fn entry_identity(&self, path: &str) -> Option<[u8; 16]> { + let catalog = self.catalog()?; + catalog + .sections() + .find(|section| section.path == path) + .map(|section| section.file_id) + .or_else(|| { + folders(catalog, |_| true) + .into_iter() + .find(|folder| folder.path == path)? + .toc + .as_ref() + .map(|toc| toc.file_id) + }) + } + /// The readable sections of the folder at catalog path `folder`, as tabs in its order. pub fn tabs(&self, folder: &str) -> Vec { match &self.notebook { diff --git a/crates/snowbound/src/main.rs b/crates/snowbound/src/main.rs index f897c2f75d3845e2056de950716d031cec6fb2b9..9cf8ea007e5d1bec7a421596003f0085fff95eaa 100644 --- a/crates/snowbound/src/main.rs +++ b/crates/snowbound/src/main.rs @@ -568,6 +568,22 @@ enum Bar { } impl Session { + fn follow(&mut self, library: Arc, path: &str) -> Result<(), Box> { + let folder = path.rsplit_once('/').map_or("", |(folder, _)| folder); + let tabs = library.tabs(folder); + let tab = tabs + .iter() + .position(|tab| tab.path == path) + .ok_or("The section is not in its notebook")?; + if self.tabs[self.tab].path != path { + library.relocate(path, &mut self.section)?; + } + self.tabs = tabs; + self.tab = tab; + self.library = library; + Ok(()) + } + fn title(&self) -> &str { match self.conflict(self.space) { Some((_, version)) => &version.title, @@ -1797,6 +1813,11 @@ impl State { let typed = renaming .as_ref() .map(|(tab, name)| (*tab, name.to_string())); + let labels: Vec<_> = session + .tabs + .iter() + .map(|tab| session.library.label(&tab.path, &tab.name)) + .collect(); let tabs: Vec<_> = session .tabs .iter() @@ -1804,7 +1825,7 @@ impl State { .map(|(index, tab)| { let name = match &typed { Some((renamed, name)) if *renamed == index => name.as_str(), - _ => tab.name.as_str(), + _ => labels[index].as_ref(), }; let unread = unread.contains(&session.library.key(&tab.path)); (name, section_color(tab.color), unread) @@ -3848,7 +3869,35 @@ impl State { } let created = matches!(opening.loaded, Shown::Created(_)); match opening.loaded { - Shown::Section(session) | Shown::Created(session) => { + Shown::Section(mut session) | Shown::Created(mut session) => { + if let Some(library) = self + .notebooks + .iter() + .find(|listed| { + listed.location == session.library.location + && Arc::ptr_eq(&listed.structure, &session.library.structure) + && !Arc::ptr_eq(listed, &session.library) + }) + .cloned() + { + let identity = session + .library + .section_identity(&session.tabs[session.tab].path); + let path = library.catalog().and_then(|catalog| { + catalog + .sections() + .find(|section| Some(section.file_id) == identity) + .map(|section| section.path.clone()) + }); + if let Some(path) = path { + session.follow(library, &path)?; + } else if let Some(path) = library.first_section() { + return self.apply(Command::OpenSection(library, path)); + } else { + self.switching = None; + return self.adopt(library, None); + } + } // The settings keep the notebook shown, not the section. let other_notebook = self .session @@ -3954,7 +4003,7 @@ impl State { } /// Takes `library`, a notebook read again after a change, in place of the one it was; - /// the open section, now at catalog `path` in it, stays open, reopened where it moved. + /// the open section, now at catalog `path` in it, keeps its replica where it moved. /// Without `path` the notebook has no sections left, and one of its shows none. fn adopt(&mut self, library: Arc, path: Option<&str>) -> Result<(), Box> { self.list(&library); @@ -3968,7 +4017,11 @@ impl State { { self.persist()?; if let Some(session) = self.session.take() { - session.section.close()?; + crate::spawn_async(move || async move { + if let Err(error) = session.section.close_async().await { + eprintln!("Synchronization stopped: {error}"); + } + }); } self.sectionless = Some(Arc::clone(&library)); self.title(); @@ -3979,28 +4032,11 @@ impl State { let Some(session) = &mut self.session else { return Ok(()); }; - let folder = path.rsplit_once('/').map_or("", |(folder, _)| folder); - let space = session.space; - if session.tabs[session.tab].path == path { - session.tabs = library.tabs(folder); - session.tab = session - .tabs - .iter() - .position(|tab| tab.path == path) - .ok_or("The section is not in its notebook")?; - session.library = library; - } else { - // Its file moved: the replica it holds reopens on the file's new path. - self.persist()?; - if let Some(session) = self.session.take() { - session.section.close()?; - } - let section = library.open(path, notify(self.proxy.clone()))?; - let (session, page) = read_session(section, library, path.to_owned(), Some(space))?; - self.session = Some(session); - let response = self.view.refresh(page)?; - self.respond(response); + if !shown(&session.library) { + return Ok(()); } + session.follow(library, path)?; + self.title(); self.save_settings(); Ok(()) } diff --git a/crates/snowbound/src/manage.rs b/crates/snowbound/src/manage.rs index 7fd0494925836d11cc8a218120ff9c7fe934531f..d283635230e43072334fa9d6e60a2cdae22046cf 100644 --- a/crates/snowbound/src/manage.rs +++ b/crates/snowbound/src/manage.rs @@ -111,6 +111,63 @@ fn unused(taken: &[String], base: &str, numbered: bool) -> String { .expect("Some number is free") } +fn rename( + notebook: &mut Notebook, + library: &Library, + path: &str, + name: &str, + request: Option<&([u8; 16], Arc<()>)>, +) -> Result> { + let current = request + .and_then(|(identity, _)| { + notebook + .catalog() + .sections() + .find(|section| §ion.file_id == identity) + .map(|section| section.path.clone()) + .or_else(|| { + crate::library::folders(notebook.catalog(), |_| true) + .into_iter() + .find(|folder| { + folder + .toc + .as_ref() + .is_some_and(|toc| &toc.file_id == identity) + }) + .map(|folder| folder.path.clone()) + }) + }) + .unwrap_or_else(|| path.to_owned()); + let latest = request.is_none_or(|(identity, request)| { + library + .renames + .lock() + .unwrap_or_else(|poisoned| poisoned.into_inner()) + .get(identity) + .is_some_and(|(_, pending)| Arc::ptr_eq(pending, request)) + }); + if latest && crate::library::entry_name(¤t) != name { + Ok(notebook.rename(¤t, name)?) + } else { + Ok(current) + } +} + +fn finish_rename(library: &Library, request: Option<([u8; 16], Arc<()>)>) { + if let Some((identity, request)) = request { + let mut renames = library + .renames + .lock() + .unwrap_or_else(|poisoned| poisoned.into_inner()); + if renames + .get(&identity) + .is_some_and(|(_, pending)| Arc::ptr_eq(pending, &request)) + { + renames.remove(&identity); + } + } +} + impl State { /// A page OneNote 2010 would create now, titled `title` in the Default font's face, with /// its date and time. @@ -781,7 +838,24 @@ impl State { .filter(|session| Arc::ptr_eq(&session.library, &library)) .map(|session| session.tabs[session.tab].path.clone()); let notify = notify(self.proxy.clone()); - self.load_async(move || async move { + let (proxy, requested) = (self.proxy.clone(), self.loading); + let showing = self.switching.is_none() && self.opening.is_none(); + let structure = Arc::clone(&library.structure); + let pending = match &change { + Structure::Rename { path, name } => library.entry_identity(path).map(|identity| { + let request = Arc::new(()); + library + .renames + .lock() + .unwrap_or_else(|poisoned| poisoned.into_inner()) + .insert(identity, (name.clone(), Arc::clone(&request))); + (identity, request) + }), + _ => None, + }; + let renaming = Arc::clone(&library); + let request = pending.clone(); + let read = move || async move { // A section kept open would hold a file the change moves. library.close_kept_async().await?; let mut notebook = library.reopen()?; @@ -817,7 +891,7 @@ impl State { (current, None) } Structure::Rename { path, name } => { - let renamed = notebook.rename(&path, &name)?; + let renamed = rename(&mut notebook, &library, &path, &name, request.as_ref())?; ( current.map(|current| follow(¤t, &path, &renamed)), None, @@ -892,8 +966,7 @@ impl State { let Some(path) = shown(&library, open) else { return Ok(Loaded::Library(library, None)); }; - // The open section, perhaps renamed or moved, stays open: its replica is this - // thread's to reopen only where no section holds it. + // The shown section keeps its replica when its file moves. if followed.as_deref() == Some(path.as_str()) { return Ok(Loaded::Library(library, Some(path))); } @@ -904,6 +977,75 @@ impl State { } else { Loaded::Section(Box::new(session), page) }) + }; + crate::spawn_async(move || async move { + let _changing = structure.lock().await; + let loaded: Result> = read().await; + let loaded = loaded.map_err(|error| crate::plain(&*error, "page")); + let _ = proxy.send_event(crate::UserEvent::Then(Box::new(move |state| { + finish_rename(&renaming, pending); + let loaded = match loaded { + Ok(loaded) => loaded, + Err(error) => { + platform::alert("Couldn't open", &error); + return Ok(()); + } + }; + let library = match &loaded { + Loaded::Library(library, _) => Arc::clone(library), + Loaded::Section(session, _) | Loaded::Created(session, _) => { + Arc::clone(&session.library) + } + _ => unreachable!("A structure change returns its notebook"), + }; + if !state + .notebooks + .iter() + .any(|listed| listed.location == library.location) + { + return Ok(()); + } + state.list(&library); + let current = state + .session + .as_ref() + .filter(|session| session.library.location == library.location) + .and_then(|session| { + session + .library + .section_identity(&session.tabs[session.tab].path) + }); + if let Some(identity) = current { + let path = library.catalog().and_then(|catalog| { + catalog + .sections() + .find(|section| section.file_id == identity) + .map(|section| section.path.clone()) + }); + if let Some(path) = path { + state.adopt(library, Some(&path))?; + } else if requested != state.loading || !showing { + if let Some(path) = library.first_section() { + state.commands.push(Command::OpenSection(library, path)); + } else { + state.adopt(library, None)?; + } + } + } else if state + .sectionless + .as_ref() + .is_some_and(|shown| shown.location == library.location) + { + state.sectionless = Some(library); + } + if showing && requested == state.loading { + match loaded { + Loaded::Library(library, path) => state.adopt(library, path.as_deref())?, + loaded => state.load(move || Ok(loaded)), + } + } + Ok(()) + }))); }); } @@ -1018,6 +1160,88 @@ mod tests { creation.dated(&date, &time).unwrap() } + #[test] + fn repeated_section_renames_follow_identity_and_keep_the_latest_label() { + let temporary = + std::env::temp_dir().join(format!("snowbound-rename-order-{}", std::process::id())); + let _ = notebook::fs::remove_dir_all(&temporary); + notebook::fs::create_dir_all(&temporary).unwrap(); + let root = temporary.join("Notebook"); + let cache = temporary.join("cache"); + let mut notebook = Notebook::create(&root, &cache, Notebook::NEW_COLOR, &dated()).unwrap(); + notebook.create_section("", "Taken", &dated()).unwrap(); + let library = Library::created(root.to_str().unwrap(), notebook, &cache); + let original = "New Section 1.one"; + let identity = library.entry_identity(original).unwrap(); + let pending = |library: &Library, name: &str| { + let request = Arc::new(()); + library + .renames + .lock() + .unwrap() + .insert(identity, (name.to_owned(), Arc::clone(&request))); + (identity, request) + }; + let first = pending(&library, "B"); + let mut notebook = library.reopen().unwrap(); + assert_eq!( + rename(&mut notebook, &library, original, "B", Some(&first)).unwrap(), + "B.one" + ); + let refreshed = library.with(notebook); + let failed = pending(&refreshed, "Taken"); + let mut notebook = refreshed.reopen().unwrap(); + assert!(rename(&mut notebook, &library, original, "Taken", Some(&failed)).is_err()); + let latest = pending(&library, "C"); + finish_rename(&library, Some(first)); + finish_rename(&refreshed, Some(failed)); + assert_eq!(library.label(original, "New Section 1"), "C"); + assert_eq!(refreshed.label("B.one", "B"), "C"); + assert_eq!( + rename(&mut notebook, &library, original, "C", Some(&latest)).unwrap(), + "C.one" + ); + let refreshed = refreshed.with(notebook); + assert_eq!(refreshed.entry_identity("C.one"), Some(identity)); + assert!(!root.join(original).exists()); + assert!(!root.join("B.one").exists()); + assert!(root.join("C.one").exists()); + finish_rename(&library, Some(latest)); + assert!(matches!( + refreshed.label("C.one", "C"), + std::borrow::Cow::Borrowed("C") + )); + + let mut notebook = refreshed.reopen().unwrap(); + for stale_name in ["Old", "C"] { + let stale = pending(&library, stale_name); + let same = pending(&refreshed, "C"); + assert_eq!( + rename(&mut notebook, &library, original, stale_name, Some(&stale)).unwrap(), + "C.one" + ); + finish_rename(&library, Some(stale)); + assert!( + library + .renames + .lock() + .unwrap() + .get(&identity) + .is_some_and(|(_, request)| Arc::ptr_eq(request, &same.1)) + ); + assert_eq!( + rename(&mut notebook, &library, original, "C", Some(&same)).unwrap(), + "C.one" + ); + finish_rename(&refreshed, Some(same)); + } + assert!(library.renames.lock().unwrap().is_empty()); + drop(notebook); + drop(refreshed); + drop(library); + notebook::fs::remove_dir_all(temporary).unwrap(); + } + /// Applies `ops` to the section file at `file` as one edit, as the section thread does, /// and publishes it. fn edit(file: &Path, ops: Vec) { diff --git a/crates/snowbound/src/sidebar.rs b/crates/snowbound/src/sidebar.rs index a5b80aed888fe789ca199c845648aaa8782d35d6..c69bd7c4b3a128bad3be860c0fb0ab7d42af5eb7 100644 --- a/crates/snowbound/src/sidebar.rs +++ b/crates/snowbound/src/sidebar.rs @@ -417,6 +417,7 @@ fn folder( false, ), }; + let name = library.label(§ion.path, &name); gap(ui, tree, &folder.path, Some(§ion.path)); let renaming = renamed(tree, §ion.path); let id = ui.id(("section", notebook, §ion.path)); @@ -463,12 +464,15 @@ fn folder( } let key = library.key(&group.path); let unfolded = !tree.folded.contains(&key); - let name = group.path.rsplit('/').next().unwrap_or_default(); + let name = library.label( + &group.path, + group.path.rsplit('/').next().unwrap_or_default(), + ); gap(ui, tree, &folder.path, Some(&group.path)); let renaming = renamed(tree, &group.path); let id = ui.id(("group", notebook, &group.path)); let row = Row { - label: name, + label: &name, icon: Leading::Icon(art::SECTION_GROUP), depth, dim: false,