diff --git a/src/terminal/git_data.rs b/src/terminal/git_data.rs index 07085e64..51f5e837 100644 --- a/src/terminal/git_data.rs +++ b/src/terminal/git_data.rs @@ -232,7 +232,14 @@ pub enum ScmWatcher { pub struct GitSubscriptions { refs: ByHost, sub_gen: ByHost, - held: HashMap, + /// Per window, then per watcher. Every window reconciles this one global + /// every frame, so a watcher is only "the same one" within its window: + /// keyed by the watcher alone, two windows whose file trees sit in + /// different directories took the hold from each other on every frame, + /// and each hand-over dropped the repository's watch and opened it again — + /// four round trips to a remote machine, several times a second, for as + /// long as both windows were open. + held: HashMap<(u64, ScmWatcher), (HostId, PathBuf)>, } impl GitSubscriptions { @@ -273,20 +280,22 @@ impl GitSubscriptions { /// caller can tear down what was keeping it alive. pub fn declare( &mut self, + window: u64, who: ScmWatcher, target: Option<(HostId, PathBuf)>, ) -> Option<(HostId, PathBuf)> { - let previous = self.held.get(&who).cloned(); + let key = (window, who); + let previous = self.held.get(&key).cloned(); if previous == target { return None; } match &target { Some((host, root)) => { self.acquire(*host, root); - self.held.insert(who, (*host, root.clone())); + self.held.insert(key, (*host, root.clone())); } None => { - self.held.remove(&who); + self.held.remove(&key); } } let (host, root) = previous?; @@ -294,6 +303,20 @@ impl GitSubscriptions { (!self.is_subscribed(host, &root)).then_some((host, root)) } + /// Give back everything a closed window held, reporting the repositories + /// nobody holds any more. + pub fn release_window(&mut self, window: u64) -> Vec<(HostId, PathBuf)> { + let mine: Vec = self + .held + .keys() + .filter(|(owner, _)| *owner == window) + .map(|(_, who)| *who) + .collect(); + mine.into_iter() + .filter_map(|who| self.declare(window, who, None)) + .collect() + } + fn clear_host(&mut self, host: HostId) { self.refs.clear_host(host); self.sub_gen.clear_host(host); @@ -490,6 +513,13 @@ impl ScmData { self.subs.is_subscribed(host, root) } + /// A window is gone: whatever it was holding open, it holds no longer. + pub fn release_window(&mut self, window: u64) { + for (host, root) in self.subs.release_window(window) { + self.drop_watch(host, &root); + } + } + /// Nobody is holding a repository open and nothing is watching one. pub fn is_quiet(&self) -> bool { self.subs.is_empty() && self.watches.is_empty() @@ -812,11 +842,12 @@ impl Tty7App { { return; } + let window_key = cx.entity_id().as_u64(); for (who, target) in wanted { let dropped = cx .default_global::() .subscriptions() - .declare(who, target); + .declare(window_key, who, target); if let Some((host, root)) = dropped { cx.default_global::().drop_watch(host, &root); } @@ -1504,7 +1535,7 @@ mod tests { let mut subs = GitSubscriptions::default(); let here = Some((HostId::LOCAL, root())); for _ in 0..30 { - assert_eq!(subs.declare(ScmWatcher::Panel, here.clone()), None); + assert_eq!(subs.declare(1, ScmWatcher::Panel, here.clone()), None); } assert_eq!( subs.count(HostId::LOCAL, &root()), @@ -1512,27 +1543,75 @@ mod tests { "render runs every frame; a hold is a statement, not an event" ); - subs.declare(ScmWatcher::FileTree, here.clone()); + subs.declare(1, ScmWatcher::FileTree, here.clone()); assert_eq!(subs.count(HostId::LOCAL, &root()), 2); assert_eq!( - subs.declare(ScmWatcher::Panel, None), + subs.declare(1, ScmWatcher::Panel, None), None, "the file tree is still looking" ); assert_eq!( - subs.declare(ScmWatcher::FileTree, None), + subs.declare(1, ScmWatcher::FileTree, None), Some((HostId::LOCAL, root())), "the last one out reports the repository to tear down" ); } + /// Two windows, each with its file tree in its own directory: the + /// global must hold both, not hand one hold back and forth between them + /// every frame. + #[test] + fn two_windows_watching_different_repositories_keep_both() { + let mut subs = GitSubscriptions::default(); + let other = PathBuf::from("/elsewhere"); + for _ in 0..30 { + assert_eq!( + subs.declare(1, ScmWatcher::FileTree, Some((HostId::LOCAL, root()))), + None + ); + assert_eq!( + subs.declare( + 2, + ScmWatcher::FileTree, + Some((HostId::LOCAL, other.clone())) + ), + None, + "the other window's frame must not take this one's hold" + ); + } + let generation = subs.generation(HostId::LOCAL, &root()); + assert_eq!( + subs.declare(2, ScmWatcher::FileTree, None), + Some((HostId::LOCAL, other)) + ); + assert_eq!(subs.count(HostId::LOCAL, &root()), 1); + assert_eq!( + subs.generation(HostId::LOCAL, &root()), + generation, + "nothing in flight for the first window's repository was invalidated" + ); + } + + #[test] + fn a_closed_window_gives_back_what_it_held() { + let mut subs = GitSubscriptions::default(); + subs.declare(1, ScmWatcher::FileTree, Some((HostId::LOCAL, root()))); + subs.declare(1, ScmWatcher::Panel, Some((HostId::LOCAL, root()))); + subs.declare(2, ScmWatcher::Panel, Some((HostId::LOCAL, root()))); + + assert!(subs.release_window(1).is_empty(), "window 2 still holds it"); + assert_eq!(subs.count(HostId::LOCAL, &root()), 1); + assert_eq!(subs.release_window(2), vec![(HostId::LOCAL, root())]); + assert!(subs.is_empty()); + } + #[test] fn moving_a_subscriber_to_another_repository_releases_the_first() { let mut subs = GitSubscriptions::default(); let other = PathBuf::from("/elsewhere"); - subs.declare(ScmWatcher::Panel, Some((HostId::LOCAL, root()))); + subs.declare(1, ScmWatcher::Panel, Some((HostId::LOCAL, root()))); assert_eq!( - subs.declare(ScmWatcher::Panel, Some((HostId::LOCAL, other.clone()))), + subs.declare(1, ScmWatcher::Panel, Some((HostId::LOCAL, other.clone()))), Some((HostId::LOCAL, root())) ); assert_eq!(subs.count(HostId::LOCAL, &root()), 0); diff --git a/src/ui/app.rs b/src/ui/app.rs index 09ff1903..076a1f21 100644 --- a/src/ui/app.rs +++ b/src/ui/app.rs @@ -1610,6 +1610,13 @@ impl Tty7App { let app_id = cx.entity_id(); cx.on_release(move |_, cx| crate::ui::lsp::LspStore::sync_window(app_id, Vec::new(), cx)) .detach(); + cx.on_release(move |_, cx| { + if cx.has_global::() { + cx.global_mut::() + .release_window(app_id.as_u64()); + } + }) + .detach(); cx.on_app_quit(|app, cx| { app.save_session(cx); crate::core::window_state::WindowState::from_bounds(app.window_bounds).save();