From d182182ae2545ce66825582e6e071d407831d808 Mon Sep 17 00:00:00 2001 From: Max Brunsfeld Date: Wed, 29 Sep 2021 22:08:28 -0700 Subject: [PATCH] Avoid ProjectPanel panic when worktree has no root entry Also, avoid bug where too many UniformList elements were rendered. --- zed/src/project.rs | 9 ++- zed/src/project_panel.rs | 127 ++++++++++++++++++++++----------------- zed/src/worktree.rs | 4 +- 3 files changed, 81 insertions(+), 59 deletions(-) diff --git a/zed/src/project.rs b/zed/src/project.rs index 9ec429a62a586abe6f5c6bd8a26a92d5652eec6b..3f7ef27a455a3527332747b2766ceefe2082dfce 100644 --- a/zed/src/project.rs +++ b/zed/src/project.rs @@ -20,6 +20,7 @@ pub struct Project { pub enum Event { ActiveEntryChanged(Option<(usize, usize)>), + WorktreeRemoved(usize), } impl Project { @@ -166,7 +167,7 @@ impl Project { pub fn close_remote_worktree(&mut self, id: u64, cx: &mut ModelContext) { self.worktrees.retain(|worktree| { - worktree.update(cx, |worktree, cx| { + let keep = worktree.update(cx, |worktree, cx| { if let Some(worktree) = worktree.as_remote_mut() { if worktree.remote_id() == id { worktree.close_all_buffers(cx); @@ -174,7 +175,11 @@ impl Project { } } true - }) + }); + if !keep { + cx.emit(Event::WorktreeRemoved(worktree.id())); + } + keep }); } } diff --git a/zed/src/project_panel.rs b/zed/src/project_panel.rs index 589c809bf9c193ce088dea360167dc54cf5a5d71..24bf7a08771fb60affedacd25c0b12d608efb127 100644 --- a/zed/src/project_panel.rs +++ b/zed/src/project_panel.rs @@ -10,13 +10,16 @@ use gpui::{ WeakViewHandle, }; use postage::watch; -use std::ops::Range; +use std::{ + collections::{hash_map, HashMap}, + ops::Range, +}; pub struct ProjectPanel { project: ModelHandle, list: UniformListState, visible_entries: Vec>, - expanded_dir_ids: Vec>, + expanded_dir_ids: HashMap>, settings: watch::Receiver, handle: WeakViewHandle, } @@ -56,9 +59,16 @@ impl ProjectPanel { cx.notify(); }) .detach(); - cx.subscribe(&project, |this, _, event, cx| { - if let project::Event::ActiveEntryChanged(Some((worktree_id, entry_id))) = event { - this.expand_active_entry(*worktree_id, *entry_id, cx); + cx.subscribe(&project, |this, _, event, cx| match event { + project::Event::ActiveEntryChanged(entry) => { + if let Some((worktree_id, entry_id)) = entry { + this.expand_active_entry(*worktree_id, *entry_id, cx); + this.update_visible_entries(true, cx); + cx.notify(); + } + } + project::Event::WorktreeRemoved(id) => { + this.expanded_dir_ids.remove(id); this.update_visible_entries(true, cx); cx.notify(); } @@ -82,16 +92,18 @@ impl ProjectPanel { worktree_ix, entry_id, } = action.0; - let expanded_dir_ids = &mut self.expanded_dir_ids[worktree_ix]; - match expanded_dir_ids.binary_search(&entry_id) { - Ok(ix) => { - expanded_dir_ids.remove(ix); - } - Err(ix) => { - expanded_dir_ids.insert(ix, entry_id); + let worktree_id = self.project.read(cx).worktrees()[worktree_ix].id(); + if let Some(expanded_dir_ids) = self.expanded_dir_ids.get_mut(&worktree_id) { + match expanded_dir_ids.binary_search(&entry_id) { + Ok(ix) => { + expanded_dir_ids.remove(ix); + } + Err(ix) => { + expanded_dir_ids.insert(ix, entry_id); + } } + self.update_visible_entries(false, cx); } - self.update_visible_entries(false, cx); } fn update_visible_entries(&mut self, scroll_to_active_entry: bool, cx: &mut ViewContext) { @@ -100,15 +112,23 @@ impl ProjectPanel { self.visible_entries.clear(); let mut entry_ix = 0; - for (worktree_ix, worktree) in worktrees.iter().enumerate() { + for worktree in worktrees { let snapshot = worktree.read(cx).snapshot(); + let worktree_id = worktree.id(); + + let expanded_dir_ids = match self.expanded_dir_ids.entry(worktree_id) { + hash_map::Entry::Occupied(e) => e.into_mut(), + hash_map::Entry::Vacant(e) => { + // The first time a worktree's root entry becomes available, + // mark that root entry as expanded. + if let Some(entry) = snapshot.root_entry() { + e.insert(vec![entry.id]).as_slice() + } else { + &[] + } + } + }; - if self.expanded_dir_ids.len() <= worktree_ix { - self.expanded_dir_ids - .push(vec![snapshot.root_entry().unwrap().id]) - } - - let expanded_dir_ids = &self.expanded_dir_ids[worktree_ix]; let mut visible_worktree_entries = Vec::new(); let mut entry_iter = snapshot.entries(false); while let Some(item) = entry_iter.entry() { @@ -138,13 +158,10 @@ impl ProjectPanel { cx: &mut ViewContext, ) { let project = self.project.read(cx); - if let Some(worktree) = project.worktree_for_id(worktree_id) { - let worktree_ix = project - .worktrees() - .iter() - .position(|w| w.id() == worktree_id) - .unwrap(); - let expanded_dir_ids = &mut self.expanded_dir_ids[worktree_ix]; + if let Some((worktree, expanded_dir_ids)) = project + .worktree_for_id(worktree_id) + .zip(self.expanded_dir_ids.get_mut(&worktree_id)) + { let worktree = worktree.read(cx); if let Some(mut entry) = worktree.entry_for_id(entry_id) { @@ -165,31 +182,36 @@ impl ProjectPanel { } } - fn append_visible_entries( + fn for_each_visible_entry( &self, range: Range, - items: &mut Vec, cx: &mut C, - mut render_item: impl FnMut(ProjectEntry, EntryDetails, &mut C) -> T, + mut callback: impl FnMut(ProjectEntry, EntryDetails, &mut C), ) { let project = self.project.read(cx); let active_entry = project.active_entry(); let worktrees = project.worktrees().to_vec(); - let mut total_ix = 0; + let mut ix = 0; for (worktree_ix, visible_worktree_entries) in self.visible_entries.iter().enumerate() { - if total_ix >= range.end { - break; + if ix >= range.end { + return; } - if total_ix + visible_worktree_entries.len() <= range.start { - total_ix += visible_worktree_entries.len(); + if ix + visible_worktree_entries.len() <= range.start { + ix += visible_worktree_entries.len(); continue; } - let expanded_entry_ids = &self.expanded_dir_ids[worktree_ix]; + let end_ix = range.end.min(ix + visible_worktree_entries.len()); let worktree = &worktrees[worktree_ix]; + let expanded_entry_ids = self + .expanded_dir_ids + .get(&worktree.id()) + .map(Vec::as_slice) + .unwrap_or(&[]); let snapshot = worktree.read(cx).snapshot(); let mut cursor = snapshot.entries(false); - for ix in visible_worktree_entries[(range.start - total_ix)..] + + for ix in visible_worktree_entries[(range.start - ix)..(end_ix - ix)] .iter() .copied() { @@ -209,10 +231,10 @@ impl ProjectPanel { worktree_ix, entry_id: entry.id, }; - items.push(render_item(entry, details, cx)); + callback(entry, details, cx); } - total_ix += 1; } + ix = end_ix; } } @@ -271,9 +293,11 @@ impl View for ProjectPanel { let theme = &settings.borrow().theme.project_panel; let this = handle.upgrade(cx).unwrap(); this.update(cx.app, |this, cx| { - this.append_visible_entries(range, items, cx, |entry, details, cx| { - Self::render_entry(entry, details, theme, cx) + let prev_len = items.len(); + this.for_each_visible_entry(range.clone(), cx, |entry, details, cx| { + items.push(Self::render_entry(entry, details, theme, cx)); }); + let count = items.len() - prev_len; }) }, ) @@ -470,20 +494,15 @@ mod tests { let mut result = Vec::new(); let mut project_entries = HashSet::new(); panel.update(cx, |panel, cx| { - panel.append_visible_entries( - range, - &mut result, - cx, - |project_entry, details, _| { - assert!( - project_entries.insert(project_entry), - "duplicate project entry {:?} {:?}", - project_entry, - details - ); + panel.for_each_visible_entry(range, cx, |project_entry, details, _| { + assert!( + project_entries.insert(project_entry), + "duplicate project entry {:?} {:?}", + project_entry, details - }, - ); + ); + result.push(details); + }); }); result diff --git a/zed/src/worktree.rs b/zed/src/worktree.rs index 23b70d917b37e86e0fb3b6628d8d440d0c1082ab..eaeefcd46257f293530eaee73488fd30461a9cc5 100644 --- a/zed/src/worktree.rs +++ b/zed/src/worktree.rs @@ -2826,9 +2826,7 @@ mod tests { cx.read(|cx| tree.read(cx).as_local().unwrap().scan_complete()) .await; - tree.read_with(&cx, |tree, cx| { - dbg!(tree.entries_by_path.items(&())); - + tree.read_with(&cx, |tree, _| { assert_eq!( tree.entries(false) .map(|entry| entry.path.as_ref())