authorclover caruso <git@paperclover.net> 2026-10-10 19:04:20-07:00
committerclover caruso <git@paperclover.net> 2026-10-10 22:56:05-07:00
log8c01efc1126e6d58946a2209125df65e46d10145
tree279207eaa5bc7c921924c33360da520d1d7abd67
parent57278aa264f7bbda73a30923db9b52a5541ae034
signature Signed by SSH key SHA256:52mNGHRsVFBDED9IAX5pe+LRWUefqTbxEReunq21QvU (~clover)

fix: keep section mutations responsive across navigation

Show submitted section and group names until their own storage operation finishes. Serialize notebook structure changes, resolve repeated renames by file identity, and reconcile successful catalogs without superseding newer navigation. Retarget open section workers in place after moves, preserving queued edits and the editor while a cancelled remote attempt finishes off-thread. Cover repeated and failed renames, stale labels across catalog refreshes, and a queued edit whose old remote stalls or fails after its section moves. Fixes #109 Refs #108 Assisted-by: gpt-6.1-sol

9 files changed, 610 insertions(+), 51 deletions(-)

Cargo.lock+1
......@@ -3627,6 +3627,7 @@ dependencies = [
36273627 "accesskit_consumer",
36283628 "accesskit_winit",
36293629 "arboard",
3630 "async-lock",
36303631 "base64",
36313632 "block2 0.5.1",
36323633 "canvas",
crates/notebook/src/session.rs+51-15
......@@ -2139,10 +2139,30 @@ impl Section {
21392139 }
21402140 });
21412141 let observed: Arc<Mutex<Observed>> = Arc::new(Mutex::new(None));
2142 let worker = {
2143 let (sender, notify) = (sender.clone(), Arc::clone(&notify));
2144 let observed = Arc::clone(&observed);
2145 replica.start_sync(Duration::from_secs(2), connect, move |result| {
2142 let mut section = Self {
2143 file,
2144 replica,
2145 worker: None,
2146 events: Mutex::new(events),
2147 sender,
2148 notify,
2149 observed,
2150 };
2151 section.worker = Some(section.start_worker(connect, None)?);
2152 Ok(section)
2153 }
2154
2155 fn start_worker<R: Remote + 'static>(
2156 &self,
2157 connect: impl FnMut() -> io::Result<R> + Send + 'static,
2158 replacing: Option<&SyncWorker>,
2159 ) -> Result<SyncWorker> {
2160 let (sender, notify) = (self.sender.clone(), Arc::clone(&self.notify));
2161 let observed = Arc::clone(&self.observed);
2162 Ok(self.replica.start_sync_replacing(
2163 Duration::from_secs(2),
2164 connect,
2165 move |result| {
21462166 let error = result.as_ref().err().map(reached);
21472167 // Reaching the file as the last attempt did is no news to the host.
21482168 let mut changed = false;
......@@ -2182,17 +2202,33 @@ impl Section {
21822202 {
21832203 notify();
21842204 }
2185 })?
2186 };
2187 Ok(Self {
2188 file,
2189 replica,
2190 worker: Some(worker),
2191 events: Mutex::new(events),
2192 sender,
2193 notify,
2194 observed,
2195 })
2205 },
2206 replacing,
2207 )?)
2208 }
2209
2210 /// Follows a section file moved within its notebook, keeping its replica and queued edits.
2211 pub fn relocate(&mut self, file: impl AsRef<Path>) -> Result<()> {
2212 self.relocate_with(fs::absolute(file)?, |file| Ok(FileRemote(file.to_owned())))
2213 }
2214
2215 /// `relocate`, with a host's publication target. Cancellation leaves any in-flight step
2216 /// to finish off-thread; synchronization ownership serializes the replacement worker.
2217 pub fn relocate_with<R: Remote + 'static>(
2218 &mut self,
2219 file: impl AsRef<Path>,
2220 mut connect: impl FnMut(&Path) -> io::Result<R> + Send + 'static,
2221 ) -> Result<()> {
2222 let file = file.as_ref().to_owned();
2223 let target = file.clone();
2224 let previous = self.worker.take();
2225 if let Some(worker) = &previous {
2226 worker.cancel();
2227 }
2228 self.observed = Arc::new(Mutex::new(None));
2229 self.worker = Some(self.start_worker(move || connect(&target), previous.as_ref())?);
2230 self.file = file;
2231 Ok(())
21962232 }
21972233
21982234 /// The absolute local path, or share-relative path for an SMB session.
crates/notebook/src/worker.rs+37-5
......@@ -86,6 +86,11 @@ pub struct SyncWorker {
8686}
8787
8888impl SyncWorker {
89 pub(crate) fn cancel(&self) {
90 self.signal.stopped.store(true, Ordering::Release);
91 self.signal.wake();
92 }
93
8994 /// Requests a retry, for example after a network reachability change. Working
9095 /// offline, it synchronizes once, as OneNote's Sync Now does.
9196 /// A pending contention backoff finishes before processing the notification.
......@@ -120,8 +125,7 @@ impl SyncWorker {
120125 /// A stopped worker leaves pending edits and uncertain attempts in the cache.
121126 /// Call outside the worker's own callback, which cannot join its calling thread.
122127 pub fn stop(mut self) -> Result<()> {
123 self.signal.stopped.store(true, Ordering::Release);
124 self.signal.wake();
128 self.cancel();
125129 let thread = self.thread.take().expect("Worker owns its thread");
126130 #[cfg(target_arch = "wasm32")]
127131 return thread.finished().unwrap_or(Ok(()));
......@@ -150,8 +154,7 @@ impl SyncWorker {
150154
151155impl Drop for SyncWorker {
152156 fn drop(&mut self) {
153 self.signal.stopped.store(true, Ordering::Release);
154 self.signal.wake();
157 self.cancel();
155158 }
156159}
157160
......@@ -166,10 +169,25 @@ impl Replica {
166169 /// Cache/document errors stop the worker; inspect them through `observe` or `stop`.
167170 /// Remote calls and callbacks must be bounded for `stop` to have bounded latency.
168171 pub fn start_sync<R, F, O>(
172 self: &Arc<Self>,
173 interval: Duration,
174 connect: F,
175 observe: O,
176 ) -> io::Result<SyncWorker>
177 where
178 R: Remote + 'static,
179 F: FnMut() -> io::Result<R> + Send + 'static,
180 O: FnMut(&Result<Synced>) + Send + 'static,
181 {
182 self.start_sync_replacing(interval, connect, observe, None)
183 }
184
185 pub(crate) fn start_sync_replacing<R, F, O>(
169186 self: &Arc<Self>,
170187 interval: Duration,
171188 mut connect: F,
172189 mut observe: O,
190 replacing: Option<&SyncWorker>,
173191 ) -> io::Result<SyncWorker>
174192 where
175193 R: Remote + 'static,
......@@ -187,10 +205,24 @@ impl Replica {
187205 .worker
188206 .lock()
189207 .map_err(|_| io::Error::other("Synchronization worker registration panicked"))?;
190 if owner.upgrade().is_some() {
208 let previous = owner.upgrade();
209 if let Some(previous) = &previous
210 && !replacing.is_some_and(|worker| {
211 Arc::ptr_eq(previous, &worker.signal) && previous.stopped.load(Ordering::Acquire)
212 })
213 {
191214 return Err(io::ErrorKind::WouldBlock.into());
192215 }
193216 let (signal, receiver) = Signal::new();
217 if let Some(previous) = previous {
218 signal
219 .offline
220 .store(previous.offline.load(Ordering::Acquire), Ordering::Release);
221 if let (Ok(previous), Ok(mut next)) = (previous.pause.lock(), signal.pause.lock()) {
222 next.length = previous.length;
223 next.waiting = previous.waiting;
224 }
225 }
194226 let replica = Arc::clone(self);
195227 let worker_signal = Arc::clone(&signal);
196228 let thread = task::spawn("onestore-sync", move || async move {
crates/notebook/tests/session.rs+134
......@@ -1218,6 +1218,140 @@ fn every_section_gets_an_offline_copy_that_closing_the_notebook_discards() {
12181218/// A section file whose stamp reads are counted.
12191219struct Counted(std::path::PathBuf, Arc<AtomicUsize>);
12201220
1221#[test]
1222fn a_moved_section_keeps_its_replica_while_the_old_remote_is_stalled() {
1223 use std::sync::{Condvar, Mutex, mpsc};
1224
1225 struct Stalled {
1226 file: std::path::PathBuf,
1227 source: Vec<u8>,
1228 entered: Arc<Mutex<Option<mpsc::Sender<()>>>>,
1229 release: Arc<(Mutex<bool>, Condvar)>,
1230 finished: Option<mpsc::Sender<()>>,
1231 publications: Arc<AtomicUsize>,
1232 late_error: bool,
1233 }
1234
1235 impl Drop for Stalled {
1236 fn drop(&mut self) {
1237 if let Some(finished) = self.finished.take() {
1238 let _ = finished.send(());
1239 }
1240 }
1241 }
1242
1243 impl notebook::Remote for Stalled {
1244 fn read(&mut self) -> std::io::Result<Vec<u8>> {
1245 Ok(self.source.clone())
1246 }
1247
1248 fn stamp(&mut self) -> std::io::Result<onestore::Stamp> {
1249 if let Some(entered) = self.entered.lock().unwrap().take() {
1250 entered.send(()).unwrap();
1251 let (lock, changed) = &*self.release;
1252 let _ = changed
1253 .wait_timeout_while(lock.lock().unwrap(), Duration::from_secs(5), |done| !*done)
1254 .unwrap();
1255 if self.late_error {
1256 return Err(std::io::ErrorKind::TimedOut.into());
1257 }
1258 }
1259 onestore::Stamp::of(&self.source).map_err(std::io::Error::other)
1260 }
1261
1262 fn publish(
1263 &mut self,
1264 transaction: &onestore::Transaction,
1265 ) -> Result<(), onestore::CommitError> {
1266 self.publications.fetch_add(1, Ordering::SeqCst);
1267 transaction.commit_file(&self.file)
1268 }
1269
1270 fn confirm(&mut self, _: &onestore::Stamp) -> Result<(), onestore::CommitError> {
1271 Ok(())
1272 }
1273 }
1274
1275 for late_error in [false, true] {
1276 let directory = tempfile::tempdir().unwrap();
1277 let file = directory.path().join("Before.one");
1278 let target = directory.path().join("After.one");
1279 let source = onestore::create_section("Before.one", "Original", "Author").unwrap();
1280 std::fs::write(&file, &source).unwrap();
1281 let (entered, waiting) = mpsc::channel();
1282 let (connected, connecting) = mpsc::channel();
1283 let (finished, stopped) = mpsc::channel();
1284 let begin = Arc::new((Mutex::new(false), Condvar::new()));
1285 let starting = Arc::clone(&begin);
1286 let release = Arc::new((Mutex::new(false), Condvar::new()));
1287 let gate = Arc::clone(&release);
1288 let entered = Arc::new(Mutex::new(Some(entered)));
1289 let publications = Arc::new(AtomicUsize::new(0));
1290 let attempts = Arc::clone(&publications);
1291 let mut opening = true;
1292 let mut section = Section::open_with(
1293 &file,
1294 directory.path().join("cache"),
1295 move |file| {
1296 let finishing = (!std::mem::replace(&mut opening, false)).then(|| finished.clone());
1297 if finishing.is_some() {
1298 connected.send(()).unwrap();
1299 let (lock, changed) = &*starting;
1300 let _ = changed
1301 .wait_timeout_while(lock.lock().unwrap(), Duration::from_secs(5), |done| {
1302 !*done
1303 })
1304 .unwrap();
1305 }
1306 Ok(Stalled {
1307 file: file.to_owned(),
1308 source: source.clone(),
1309 entered: Arc::clone(&entered),
1310 release: Arc::clone(&gate),
1311 finished: finishing,
1312 publications: Arc::clone(&attempts),
1313 late_error,
1314 })
1315 },
1316 || {},
1317 )
1318 .unwrap();
1319 connecting.recv_timeout(Duration::from_secs(3)).unwrap();
1320 let replica = Arc::clone(section.replica());
1321 let (space, ..) = section.pages().unwrap()[0];
1322 let before = section.page(space).unwrap();
1323 let edit = typed(&section, space, &before, 0..0, "Moved ");
1324 *begin.0.lock().unwrap() = true;
1325 begin.1.notify_all();
1326 waiting.recv_timeout(Duration::from_secs(3)).unwrap();
1327 section.set_offline(true);
1328 std::fs::rename(&file, &target).unwrap();
1329 let start = Instant::now();
1330 section.relocate(&target).unwrap();
1331 assert!(start.elapsed() < Duration::from_secs(1));
1332 assert!(Arc::ptr_eq(&replica, section.replica()));
1333 assert_eq!(section.file(), target);
1334 assert_same(section.page(space).unwrap(), &edited(&before, "Moved "));
1335 *release.0.lock().unwrap() = true;
1336 release.1.notify_all();
1337 stopped.recv_timeout(Duration::from_secs(3)).unwrap();
1338 assert_eq!(
1339 publications.load(Ordering::SeqCst),
1340 usize::from(!late_error)
1341 );
1342 assert!(section.sync_status().unwrap().error.is_none());
1343 assert!(!matches!(
1344 section.status(edit).unwrap(),
1345 Some(EditStatus::Published { .. })
1346 ));
1347 section.set_offline(false);
1348 published(&section, edit);
1349 assert_same(stored_page(&target, space), &edited(&before, "Moved "));
1350 assert!(!file.exists());
1351 section.close().unwrap();
1352 }
1353}
1354
12211355impl notebook::Remote for Counted {
12221356 fn read(&mut self) -> std::io::Result<Vec<u8>> {
12231357 onestore::read_file(&self.0)
crates/snowbound/Cargo.toml+1
......@@ -23,6 +23,7 @@ notebook = { path = "../notebook" }
2323serde = { version = "1", features = ["derive"] }
2424serde_json = "1"
2525pollster = "0.4"
26async-lock = "3.4.2"
2627wgpu = { workspace = true, optional = true }
2728parley.workspace = true
2829winit = "0.30.13"
crates/snowbound/src/library.rs+91
......@@ -8,6 +8,8 @@ use notebook::sidecar::themes::Themes;
88use notebook::smb::{Client, Credentials};
99use onestore::protected::Key;
1010use std::{
11 borrow::Cow,
12 collections::BTreeMap,
1113 error::Error,
1214 io,
1315 path::{Path, PathBuf},
......@@ -324,6 +326,8 @@ pub struct Library {
324326 /// Sections opened ahead of being shown, or left open after, most recent first, which
325327 /// `open` hands out before opening any.
326328 kept: Mutex<crate::prefetch::Recent<String, Section>>,
329 pub(crate) renames: Arc<Mutex<BTreeMap<[u8; 16], (String, Arc<()>)>>>,
330 pub(crate) structure: Arc<async_lock::Mutex<()>>,
327331 themes: Mutex<ThemeCache>,
328332 /// The keys of its password-protected sections unlocked this run, kept as it is read again.
329333 keys: Arc<Keys>,
......@@ -359,6 +363,8 @@ impl Library {
359363 watch: None,
360364 tag_art: Default::default(),
361365 kept: Mutex::new(crate::prefetch::Recent::new(KEPT)),
366 renames: Arc::default(),
367 structure: Arc::default(),
362368 themes: Default::default(),
363369 keys: Default::default(),
364370 #[cfg(feature = "live")]
......@@ -796,6 +802,8 @@ impl Library {
796802 watch,
797803 tag_art: Mutex::new(self.tag_art()),
798804 keys: Arc::clone(&self.keys),
805 structure: Arc::clone(&self.structure),
806 renames: Arc::clone(&self.renames),
799807 #[cfg(feature = "live")]
800808 joined: self.joined.clone(),
801809 ..Self::new(&self.location, name, &self.cache)
......@@ -882,6 +890,60 @@ impl Library {
882890 }
883891 }
884892
893 /// Follows a moved section without closing its readable replica or waiting for the share.
894 pub(crate) fn relocate(&self, path: &str, section: &mut Section) -> Result<(), Box<dyn Error>> {
895 #[cfg(feature = "live")]
896 if let Some(joined) = &self.joined {
897 let guest = Arc::clone(&joined.guest);
898 section.relocate_with(path, move |file| {
899 Ok(notebook::live::share::HostedRemote::new(
900 &guest,
901 &file.to_string_lossy(),
902 ))
903 })?;
904 if let Some(background) = &self.background {
905 background.hold(path, section);
906 }
907 return Ok(());
908 }
909 match &self.server {
910 #[cfg(not(target_arch = "wasm32"))]
911 Some(server) => {
912 let file = match server.mount.root.as_str() {
913 "" => path.to_owned(),
914 root => format!("{root}/{path}"),
915 };
916 let server = Arc::clone(server);
917 section.relocate_with(file, move |file| {
918 Ok(notebook::SmbRemote::new(
919 server.connect()?,
920 file.to_string_lossy(),
921 LIMIT,
922 ))
923 })?;
924 }
925 #[cfg(target_arch = "wasm32")]
926 Some(_) => unreachable!("The browser opens no notebook on a server"),
927 None => {
928 let file = Path::new(&self.location).join(path);
929 #[cfg(target_os = "macos")]
930 if self.in_icloud() {
931 section.relocate_with(file, |file| {
932 Ok(crate::icloud::Coordinated(file.to_owned()))
933 })?;
934 } else {
935 section.relocate(file)?;
936 }
937 #[cfg(not(target_os = "macos"))]
938 section.relocate(file)?;
939 }
940 }
941 if let Some(background) = &self.background {
942 background.hold(path, section);
943 }
944 Ok(())
945 }
946
885947 /// Opens a section, waiting natively until `deadline` for its current owner.
886948 fn open_by(
887949 &self,
......@@ -1147,6 +1209,35 @@ impl Library {
11471209 key(&self.location, path)
11481210 }
11491211
1212 /// A committed entry label, replaced by its pending rename until storage answers.
1213 pub(crate) fn label<'a>(&self, path: &str, committed: &'a str) -> Cow<'a, str> {
1214 self.renames
1215 .lock()
1216 .ok()
1217 .and_then(|renames| {
1218 renames
1219 .get(&self.entry_identity(path)?)
1220 .map(|(name, _)| name.clone())
1221 })
1222 .map_or(Cow::Borrowed(committed), Cow::Owned)
1223 }
1224
1225 pub(crate) fn entry_identity(&self, path: &str) -> Option<[u8; 16]> {
1226 let catalog = self.catalog()?;
1227 catalog
1228 .sections()
1229 .find(|section| section.path == path)
1230 .map(|section| section.file_id)
1231 .or_else(|| {
1232 folders(catalog, |_| true)
1233 .into_iter()
1234 .find(|folder| folder.path == path)?
1235 .toc
1236 .as_ref()
1237 .map(|toc| toc.file_id)
1238 })
1239 }
1240
11501241 /// The readable sections of the folder at catalog path `folder`, as tabs in its order.
11511242 pub fn tabs(&self, folder: &str) -> Vec<Tab> {
11521243 match &self.notebook {
crates/snowbound/src/main.rs+61-25
......@@ -568,6 +568,22 @@ enum Bar {
568568}
569569
570570impl Session {
571 fn follow(&mut self, library: Arc<Library>, path: &str) -> Result<(), Box<dyn Error>> {
572 let folder = path.rsplit_once('/').map_or("", |(folder, _)| folder);
573 let tabs = library.tabs(folder);
574 let tab = tabs
575 .iter()
576 .position(|tab| tab.path == path)
577 .ok_or("The section is not in its notebook")?;
578 if self.tabs[self.tab].path != path {
579 library.relocate(path, &mut self.section)?;
580 }
581 self.tabs = tabs;
582 self.tab = tab;
583 self.library = library;
584 Ok(())
585 }
586
571587 fn title(&self) -> &str {
572588 match self.conflict(self.space) {
573589 Some((_, version)) => &version.title,
......@@ -1797,6 +1813,11 @@ impl State {
17971813 let typed = renaming
17981814 .as_ref()
17991815 .map(|(tab, name)| (*tab, name.to_string()));
1816 let labels: Vec<_> = session
1817 .tabs
1818 .iter()
1819 .map(|tab| session.library.label(&tab.path, &tab.name))
1820 .collect();
18001821 let tabs: Vec<_> = session
18011822 .tabs
18021823 .iter()
......@@ -1804,7 +1825,7 @@ impl State {
18041825 .map(|(index, tab)| {
18051826 let name = match &typed {
18061827 Some((renamed, name)) if *renamed == index => name.as_str(),
1807 _ => tab.name.as_str(),
1828 _ => labels[index].as_ref(),
18081829 };
18091830 let unread = unread.contains(&session.library.key(&tab.path));
18101831 (name, section_color(tab.color), unread)
......@@ -3848,7 +3869,35 @@ impl State {
38483869 }
38493870 let created = matches!(opening.loaded, Shown::Created(_));
38503871 match opening.loaded {
3851 Shown::Section(session) | Shown::Created(session) => {
3872 Shown::Section(mut session) | Shown::Created(mut session) => {
3873 if let Some(library) = self
3874 .notebooks
3875 .iter()
3876 .find(|listed| {
3877 listed.location == session.library.location
3878 && Arc::ptr_eq(&listed.structure, &session.library.structure)
3879 && !Arc::ptr_eq(listed, &session.library)
3880 })
3881 .cloned()
3882 {
3883 let identity = session
3884 .library
3885 .section_identity(&session.tabs[session.tab].path);
3886 let path = library.catalog().and_then(|catalog| {
3887 catalog
3888 .sections()
3889 .find(|section| Some(section.file_id) == identity)
3890 .map(|section| section.path.clone())
3891 });
3892 if let Some(path) = path {
3893 session.follow(library, &path)?;
3894 } else if let Some(path) = library.first_section() {
3895 return self.apply(Command::OpenSection(library, path));
3896 } else {
3897 self.switching = None;
3898 return self.adopt(library, None);
3899 }
3900 }
38523901 // The settings keep the notebook shown, not the section.
38533902 let other_notebook = self
38543903 .session
......@@ -3954,7 +4003,7 @@ impl State {
39544003 }
39554004
39564005 /// Takes `library`, a notebook read again after a change, in place of the one it was;
3957 /// the open section, now at catalog `path` in it, stays open, reopened where it moved.
4006 /// the open section, now at catalog `path` in it, keeps its replica where it moved.
39584007 /// Without `path` the notebook has no sections left, and one of its shows none.
39594008 fn adopt(&mut self, library: Arc<Library>, path: Option<&str>) -> Result<(), Box<dyn Error>> {
39604009 self.list(&library);
......@@ -3968,7 +4017,11 @@ impl State {
39684017 {
39694018 self.persist()?;
39704019 if let Some(session) = self.session.take() {
3971 session.section.close()?;
4020 crate::spawn_async(move || async move {
4021 if let Err(error) = session.section.close_async().await {
4022 eprintln!("Synchronization stopped: {error}");
4023 }
4024 });
39724025 }
39734026 self.sectionless = Some(Arc::clone(&library));
39744027 self.title();
......@@ -3979,28 +4032,11 @@ impl State {
39794032 let Some(session) = &mut self.session else {
39804033 return Ok(());
39814034 };
3982 let folder = path.rsplit_once('/').map_or("", |(folder, _)| folder);
3983 let space = session.space;
3984 if session.tabs[session.tab].path == path {
3985 session.tabs = library.tabs(folder);
3986 session.tab = session
3987 .tabs
3988 .iter()
3989 .position(|tab| tab.path == path)
3990 .ok_or("The section is not in its notebook")?;
3991 session.library = library;
3992 } else {
3993 // Its file moved: the replica it holds reopens on the file's new path.
3994 self.persist()?;
3995 if let Some(session) = self.session.take() {
3996 session.section.close()?;
3997 }
3998 let section = library.open(path, notify(self.proxy.clone()))?;
3999 let (session, page) = read_session(section, library, path.to_owned(), Some(space))?;
4000 self.session = Some(session);
4001 let response = self.view.refresh(page)?;
4002 self.respond(response);
4035 if !shown(&session.library) {
4036 return Ok(());
40034037 }
4038 session.follow(library, path)?;
4039 self.title();
40044040 self.save_settings();
40054041 Ok(())
40064042 }
crates/snowbound/src/manage.rs+228-4
......@@ -111,6 +111,63 @@ fn unused(taken: &[String], base: &str, numbered: bool) -> String {
111111 .expect("Some number is free")
112112}
113113
114fn rename(
115 notebook: &mut Notebook,
116 library: &Library,
117 path: &str,
118 name: &str,
119 request: Option<&([u8; 16], Arc<()>)>,
120) -> Result<String, Box<dyn Error>> {
121 let current = request
122 .and_then(|(identity, _)| {
123 notebook
124 .catalog()
125 .sections()
126 .find(|section| &section.file_id == identity)
127 .map(|section| section.path.clone())
128 .or_else(|| {
129 crate::library::folders(notebook.catalog(), |_| true)
130 .into_iter()
131 .find(|folder| {
132 folder
133 .toc
134 .as_ref()
135 .is_some_and(|toc| &toc.file_id == identity)
136 })
137 .map(|folder| folder.path.clone())
138 })
139 })
140 .unwrap_or_else(|| path.to_owned());
141 let latest = request.is_none_or(|(identity, request)| {
142 library
143 .renames
144 .lock()
145 .unwrap_or_else(|poisoned| poisoned.into_inner())
146 .get(identity)
147 .is_some_and(|(_, pending)| Arc::ptr_eq(pending, request))
148 });
149 if latest && crate::library::entry_name(&current) != name {
150 Ok(notebook.rename(&current, name)?)
151 } else {
152 Ok(current)
153 }
154}
155
156fn finish_rename(library: &Library, request: Option<([u8; 16], Arc<()>)>) {
157 if let Some((identity, request)) = request {
158 let mut renames = library
159 .renames
160 .lock()
161 .unwrap_or_else(|poisoned| poisoned.into_inner());
162 if renames
163 .get(&identity)
164 .is_some_and(|(_, pending)| Arc::ptr_eq(pending, &request))
165 {
166 renames.remove(&identity);
167 }
168 }
169}
170
114171impl State {
115172 /// A page OneNote 2010 would create now, titled `title` in the Default font's face, with
116173 /// its date and time.
......@@ -781,7 +838,24 @@ impl State {
781838 .filter(|session| Arc::ptr_eq(&session.library, &library))
782839 .map(|session| session.tabs[session.tab].path.clone());
783840 let notify = notify(self.proxy.clone());
784 self.load_async(move || async move {
841 let (proxy, requested) = (self.proxy.clone(), self.loading);
842 let showing = self.switching.is_none() && self.opening.is_none();
843 let structure = Arc::clone(&library.structure);
844 let pending = match &change {
845 Structure::Rename { path, name } => library.entry_identity(path).map(|identity| {
846 let request = Arc::new(());
847 library
848 .renames
849 .lock()
850 .unwrap_or_else(|poisoned| poisoned.into_inner())
851 .insert(identity, (name.clone(), Arc::clone(&request)));
852 (identity, request)
853 }),
854 _ => None,
855 };
856 let renaming = Arc::clone(&library);
857 let request = pending.clone();
858 let read = move || async move {
785859 // A section kept open would hold a file the change moves.
786860 library.close_kept_async().await?;
787861 let mut notebook = library.reopen()?;
......@@ -817,7 +891,7 @@ impl State {
817891 (current, None)
818892 }
819893 Structure::Rename { path, name } => {
820 let renamed = notebook.rename(&path, &name)?;
894 let renamed = rename(&mut notebook, &library, &path, &name, request.as_ref())?;
821895 (
822896 current.map(|current| follow(&current, &path, &renamed)),
823897 None,
......@@ -892,8 +966,7 @@ impl State {
892966 let Some(path) = shown(&library, open) else {
893967 return Ok(Loaded::Library(library, None));
894968 };
895 // The open section, perhaps renamed or moved, stays open: its replica is this
896 // thread's to reopen only where no section holds it.
969 // The shown section keeps its replica when its file moves.
897970 if followed.as_deref() == Some(path.as_str()) {
898971 return Ok(Loaded::Library(library, Some(path)));
899972 }
......@@ -904,6 +977,75 @@ impl State {
904977 } else {
905978 Loaded::Section(Box::new(session), page)
906979 })
980 };
981 crate::spawn_async(move || async move {
982 let _changing = structure.lock().await;
983 let loaded: Result<Loaded, Box<dyn Error>> = read().await;
984 let loaded = loaded.map_err(|error| crate::plain(&*error, "page"));
985 let _ = proxy.send_event(crate::UserEvent::Then(Box::new(move |state| {
986 finish_rename(&renaming, pending);
987 let loaded = match loaded {
988 Ok(loaded) => loaded,
989 Err(error) => {
990 platform::alert("Couldn't open", &error);
991 return Ok(());
992 }
993 };
994 let library = match &loaded {
995 Loaded::Library(library, _) => Arc::clone(library),
996 Loaded::Section(session, _) | Loaded::Created(session, _) => {
997 Arc::clone(&session.library)
998 }
999 _ => unreachable!("A structure change returns its notebook"),
1000 };
1001 if !state
1002 .notebooks
1003 .iter()
1004 .any(|listed| listed.location == library.location)
1005 {
1006 return Ok(());
1007 }
1008 state.list(&library);
1009 let current = state
1010 .session
1011 .as_ref()
1012 .filter(|session| session.library.location == library.location)
1013 .and_then(|session| {
1014 session
1015 .library
1016 .section_identity(&session.tabs[session.tab].path)
1017 });
1018 if let Some(identity) = current {
1019 let path = library.catalog().and_then(|catalog| {
1020 catalog
1021 .sections()
1022 .find(|section| section.file_id == identity)
1023 .map(|section| section.path.clone())
1024 });
1025 if let Some(path) = path {
1026 state.adopt(library, Some(&path))?;
1027 } else if requested != state.loading || !showing {
1028 if let Some(path) = library.first_section() {
1029 state.commands.push(Command::OpenSection(library, path));
1030 } else {
1031 state.adopt(library, None)?;
1032 }
1033 }
1034 } else if state
1035 .sectionless
1036 .as_ref()
1037 .is_some_and(|shown| shown.location == library.location)
1038 {
1039 state.sectionless = Some(library);
1040 }
1041 if showing && requested == state.loading {
1042 match loaded {
1043 Loaded::Library(library, path) => state.adopt(library, path.as_deref())?,
1044 loaded => state.load(move || Ok(loaded)),
1045 }
1046 }
1047 Ok(())
1048 })));
9071049 });
9081050 }
9091051
......@@ -1018,6 +1160,88 @@ mod tests {
10181160 creation.dated(&date, &time).unwrap()
10191161 }
10201162
1163 #[test]
1164 fn repeated_section_renames_follow_identity_and_keep_the_latest_label() {
1165 let temporary =
1166 std::env::temp_dir().join(format!("snowbound-rename-order-{}", std::process::id()));
1167 let _ = notebook::fs::remove_dir_all(&temporary);
1168 notebook::fs::create_dir_all(&temporary).unwrap();
1169 let root = temporary.join("Notebook");
1170 let cache = temporary.join("cache");
1171 let mut notebook = Notebook::create(&root, &cache, Notebook::NEW_COLOR, &dated()).unwrap();
1172 notebook.create_section("", "Taken", &dated()).unwrap();
1173 let library = Library::created(root.to_str().unwrap(), notebook, &cache);
1174 let original = "New Section 1.one";
1175 let identity = library.entry_identity(original).unwrap();
1176 let pending = |library: &Library, name: &str| {
1177 let request = Arc::new(());
1178 library
1179 .renames
1180 .lock()
1181 .unwrap()
1182 .insert(identity, (name.to_owned(), Arc::clone(&request)));
1183 (identity, request)
1184 };
1185 let first = pending(&library, "B");
1186 let mut notebook = library.reopen().unwrap();
1187 assert_eq!(
1188 rename(&mut notebook, &library, original, "B", Some(&first)).unwrap(),
1189 "B.one"
1190 );
1191 let refreshed = library.with(notebook);
1192 let failed = pending(&refreshed, "Taken");
1193 let mut notebook = refreshed.reopen().unwrap();
1194 assert!(rename(&mut notebook, &library, original, "Taken", Some(&failed)).is_err());
1195 let latest = pending(&library, "C");
1196 finish_rename(&library, Some(first));
1197 finish_rename(&refreshed, Some(failed));
1198 assert_eq!(library.label(original, "New Section 1"), "C");
1199 assert_eq!(refreshed.label("B.one", "B"), "C");
1200 assert_eq!(
1201 rename(&mut notebook, &library, original, "C", Some(&latest)).unwrap(),
1202 "C.one"
1203 );
1204 let refreshed = refreshed.with(notebook);
1205 assert_eq!(refreshed.entry_identity("C.one"), Some(identity));
1206 assert!(!root.join(original).exists());
1207 assert!(!root.join("B.one").exists());
1208 assert!(root.join("C.one").exists());
1209 finish_rename(&library, Some(latest));
1210 assert!(matches!(
1211 refreshed.label("C.one", "C"),
1212 std::borrow::Cow::Borrowed("C")
1213 ));
1214
1215 let mut notebook = refreshed.reopen().unwrap();
1216 for stale_name in ["Old", "C"] {
1217 let stale = pending(&library, stale_name);
1218 let same = pending(&refreshed, "C");
1219 assert_eq!(
1220 rename(&mut notebook, &library, original, stale_name, Some(&stale)).unwrap(),
1221 "C.one"
1222 );
1223 finish_rename(&library, Some(stale));
1224 assert!(
1225 library
1226 .renames
1227 .lock()
1228 .unwrap()
1229 .get(&identity)
1230 .is_some_and(|(_, request)| Arc::ptr_eq(request, &same.1))
1231 );
1232 assert_eq!(
1233 rename(&mut notebook, &library, original, "C", Some(&same)).unwrap(),
1234 "C.one"
1235 );
1236 finish_rename(&refreshed, Some(same));
1237 }
1238 assert!(library.renames.lock().unwrap().is_empty());
1239 drop(notebook);
1240 drop(refreshed);
1241 drop(library);
1242 notebook::fs::remove_dir_all(temporary).unwrap();
1243 }
1244
10211245 /// Applies `ops` to the section file at `file` as one edit, as the section thread does,
10221246 /// and publishes it.
10231247 fn edit(file: &Path, ops: Vec<Op>) {
crates/snowbound/src/sidebar.rs+6-2
......@@ -417,6 +417,7 @@ fn folder(
417417 false,
418418 ),
419419 };
420 let name = library.label(&section.path, &name);
420421 gap(ui, tree, &folder.path, Some(&section.path));
421422 let renaming = renamed(tree, &section.path);
422423 let id = ui.id(("section", notebook, &section.path));
......@@ -463,12 +464,15 @@ fn folder(
463464 }
464465 let key = library.key(&group.path);
465466 let unfolded = !tree.folded.contains(&key);
466 let name = group.path.rsplit('/').next().unwrap_or_default();
467 let name = library.label(
468 &group.path,
469 group.path.rsplit('/').next().unwrap_or_default(),
470 );
467471 gap(ui, tree, &folder.path, Some(&group.path));
468472 let renaming = renamed(tree, &group.path);
469473 let id = ui.id(("group", notebook, &group.path));
470474 let row = Row {
471 label: name,
475 label: &name,
472476 icon: Leading::Icon(art::SECTION_GROUP),
473477 depth,
474478 dim: false,