Only scan ignored or externals paths if expanded in a project panel

Max Brunsfeld created

Change summary

crates/project/src/worktree.rs            | 106 +++++++++++++++++++++---
crates/project/src/worktree_tests.rs      |  68 ++++++++++++++++
crates/project_panel/src/project_panel.rs |  34 +++++--
3 files changed, 182 insertions(+), 26 deletions(-)

Detailed changes

crates/project/src/worktree.rs 🔗

@@ -5,7 +5,7 @@ use ::ignore::gitignore::{Gitignore, GitignoreBuilder};
 use anyhow::{anyhow, Context, Result};
 use client::{proto, Client};
 use clock::ReplicaId;
-use collections::{HashMap, HashSet, VecDeque};
+use collections::{BTreeSet, HashMap, VecDeque};
 use fs::{
     repository::{GitFileStatus, GitRepository, RepoPath},
     Fs, LineEnding,
@@ -226,7 +226,7 @@ pub struct LocalSnapshot {
 
 struct BackgroundScannerState {
     snapshot: LocalSnapshot,
-    expanded_dirs: HashSet<(ProjectEntryId, ReplicaId)>,
+    expanded_dirs: BTreeSet<(ProjectEntryId, ReplicaId)>,
     /// The ids of all of the entries that were removed from the snapshot
     /// as part of the current update. These entry ids may be re-used
     /// if the same inode is discovered at a new path, or if the given
@@ -2209,6 +2209,15 @@ impl LocalSnapshot {
 }
 
 impl BackgroundScannerState {
+    fn is_entry_expanded(&self, entry: &Entry) -> bool {
+        let expanded = self
+            .expanded_dirs
+            .range((entry.id, 0)..=(entry.id, ReplicaId::MAX))
+            .next()
+            .is_some();
+        expanded
+    }
+
     fn reuse_entry_id(&mut self, entry: &mut Entry) {
         if let Some(removed_entry_id) = self.removed_entry_ids.remove(&entry.inode) {
             entry.id = removed_entry_id;
@@ -2661,11 +2670,11 @@ impl Entry {
     }
 
     pub fn is_dir(&self) -> bool {
-        matches!(self.kind, EntryKind::Dir | EntryKind::PendingDir)
+        self.kind.is_dir()
     }
 
     pub fn is_file(&self) -> bool {
-        matches!(self.kind, EntryKind::File(_))
+        self.kind.is_file()
     }
 
     pub fn git_status(&self) -> Option<GitFileStatus> {
@@ -2673,6 +2682,16 @@ impl Entry {
     }
 }
 
+impl EntryKind {
+    pub fn is_dir(&self) -> bool {
+        matches!(self, EntryKind::Dir | EntryKind::PendingDir)
+    }
+
+    pub fn is_file(&self) -> bool {
+        matches!(self, EntryKind::File(_))
+    }
+}
+
 impl sum_tree::Item for Entry {
     type Summary = EntrySummary;
 
@@ -2901,6 +2920,7 @@ impl BackgroundScanner {
             path: Arc::from(Path::new("")),
             ignore_stack,
             ancestor_inodes: TreeSet::from_ordered_entries(root_inode),
+            is_outside_root: false,
             scan_queue: scan_job_tx.clone(),
         }))
         .unwrap();
@@ -2961,15 +2981,27 @@ impl BackgroundScanner {
                 replica_id,
                 is_expanded,
             } => {
-                let mut state = self.state.lock();
-                if is_expanded {
-                    state.expanded_dirs.insert((entry_id, replica_id));
-                } else {
-                    state.expanded_dirs.remove(&(entry_id, replica_id));
+                let path = {
+                    let mut state = self.state.lock();
+                    if is_expanded {
+                        state.expanded_dirs.insert((entry_id, replica_id));
+                    } else {
+                        state.expanded_dirs.remove(&(entry_id, replica_id));
+                    }
+                    state
+                        .snapshot
+                        .entry_for_id(entry_id)
+                        .map(|e| state.snapshot.absolutize(&e.path))
+                };
+                if let Some(path) = path {
+                    let (scan_job_tx, mut scan_job_rx) = channel::unbounded();
+                    self.reload_entries_for_paths(vec![path.clone()], Some(scan_job_tx))
+                        .await;
+                    if let Some(job) = scan_job_rx.next().await {
+                        self.scan_dir(&job).await.log_err();
+                        self.send_status_update(false, None);
+                    }
                 }
-
-                // todo
-
                 true
             }
         }
@@ -3120,7 +3152,16 @@ impl BackgroundScanner {
         let mut ignore_stack = job.ignore_stack.clone();
         let mut new_ignore = None;
         let (root_abs_path, root_char_bag, next_entry_id, repository) = {
-            let snapshot = &self.state.lock().snapshot;
+            let state = self.state.lock();
+            if job.is_outside_root || job.ignore_stack.is_all() {
+                if let Some(entry) = state.snapshot.entry_for_path(&job.path) {
+                    if !state.is_entry_expanded(entry) {
+                        return Ok(());
+                    }
+                }
+            }
+
+            let snapshot = &state.snapshot;
             (
                 snapshot.abs_path().clone(),
                 snapshot.root_char_bag,
@@ -3131,6 +3172,7 @@ impl BackgroundScanner {
             )
         };
 
+        let mut root_canonical_path = None;
         let mut child_paths = self.fs.read_dir(&job.abs_path).await?;
         while let Some(child_abs_path) = child_paths.next().await {
             let child_abs_path: Arc<Path> = match child_abs_path {
@@ -3198,6 +3240,38 @@ impl BackgroundScanner {
                 root_char_bag,
             );
 
+            let mut is_outside_root = false;
+            if child_metadata.is_symlink {
+                let canonical_path = match self.fs.canonicalize(&child_abs_path).await {
+                    Ok(path) => path,
+                    Err(err) => {
+                        log::error!(
+                            "error reading target of symlink {:?}: {:?}",
+                            child_abs_path,
+                            err
+                        );
+                        continue;
+                    }
+                };
+
+                // lazily canonicalize the root path in order to determine if
+                // symlinks point outside of the worktree.
+                let root_canonical_path = match &root_canonical_path {
+                    Some(path) => path,
+                    None => match self.fs.canonicalize(&root_abs_path).await {
+                        Ok(path) => root_canonical_path.insert(path),
+                        Err(err) => {
+                            log::error!("error canonicalizing root {:?}: {:?}", root_abs_path, err);
+                            continue;
+                        }
+                    },
+                };
+
+                if !canonical_path.starts_with(root_canonical_path) {
+                    is_outside_root = true;
+                }
+            }
+
             if child_entry.is_dir() {
                 let is_ignored = ignore_stack.is_abs_path_ignored(&child_abs_path, true);
                 child_entry.is_ignored = is_ignored;
@@ -3210,6 +3284,7 @@ impl BackgroundScanner {
                     new_jobs.push(Some(ScanJob {
                         abs_path: child_abs_path,
                         path: child_path,
+                        is_outside_root,
                         ignore_stack: if is_ignored {
                             IgnoreStack::all()
                         } else {
@@ -3259,7 +3334,7 @@ impl BackgroundScanner {
 
         for new_job in new_jobs {
             if let Some(new_job) = new_job {
-                job.scan_queue.send(new_job).await.unwrap();
+                job.scan_queue.send(new_job).await.ok();
             }
         }
 
@@ -3354,12 +3429,14 @@ impl BackgroundScanner {
                     if let Some(scan_queue_tx) = &scan_queue_tx {
                         let mut ancestor_inodes = state.snapshot.ancestor_inodes_for_path(&path);
                         if metadata.is_dir && !ancestor_inodes.contains(&metadata.inode) {
+                            let is_outside_root = !abs_path.starts_with(&root_canonical_path);
                             ancestor_inodes.insert(metadata.inode);
                             smol::block_on(scan_queue_tx.send(ScanJob {
                                 abs_path,
                                 path,
                                 ignore_stack,
                                 ancestor_inodes,
+                                is_outside_root,
                                 scan_queue: scan_queue_tx.clone(),
                             }))
                             .unwrap();
@@ -3748,6 +3825,7 @@ struct ScanJob {
     ignore_stack: Arc<IgnoreStack>,
     scan_queue: Sender<ScanJob>,
     ancestor_inodes: TreeSet<u64>,
+    is_outside_root: bool,
 }
 
 struct UpdateIgnoreStatusJob {

crates/project/src/worktree_tests.rs 🔗

@@ -256,6 +256,74 @@ async fn test_circular_symlinks(executor: Arc<Deterministic>, cx: &mut TestAppCo
     });
 }
 
+#[gpui::test(iterations = 10)]
+async fn test_symlinks_pointing_outside(cx: &mut TestAppContext) {
+    let fs = FakeFs::new(cx.background());
+    fs.insert_tree(
+        "/root",
+        json!({
+            "dir1": {
+                "deps": {
+                    // symlinks here
+                },
+                "src": {
+                    "a.rs": "",
+                    "b.rs": "",
+                },
+            },
+            "dir2": {
+                "src": {
+                    "c.rs": "",
+                    "d.rs": "",
+                }
+            },
+            "dir3": {
+                "src": {
+                    "e.rs": "",
+                    "f.rs": "",
+                }
+            }
+        }),
+    )
+    .await;
+    fs.insert_symlink("/root/dir1/deps/dep-dir2", "../../dir2".into())
+        .await;
+    fs.insert_symlink("/root/dir1/deps/dep-dir3", "../../dir3".into())
+        .await;
+
+    let client = cx.read(|cx| Client::new(FakeHttpClient::with_404_response(), cx));
+    let tree = Worktree::local(
+        client,
+        Path::new("/root/dir1"),
+        true,
+        fs.clone(),
+        Default::default(),
+        &mut cx.to_async(),
+    )
+    .await
+    .unwrap();
+
+    cx.read(|cx| tree.read(cx).as_local().unwrap().scan_complete())
+        .await;
+
+    tree.read_with(cx, |tree, _| {
+        assert_eq!(
+            tree.entries(false)
+                .map(|entry| entry.path.as_ref())
+                .collect::<Vec<_>>(),
+            vec![
+                Path::new(""),
+                Path::new("deps"),
+                Path::new("deps/dep-dir2"),
+                Path::new("deps/dep-dir3"),
+                Path::new("src"),
+                Path::new("src/a.rs"),
+                Path::new("src/b.rs"),
+            ]
+        );
+    });
+}
+
 #[gpui::test]
 async fn test_rescan_with_gitignore(cx: &mut TestAppContext) {
     // .gitignores are handled explicitly by Zed and do not use the git

crates/project_panel/src/project_panel.rs 🔗

@@ -410,17 +410,23 @@ impl ProjectPanel {
     fn expand_selected_entry(&mut self, _: &ExpandSelectedEntry, cx: &mut ViewContext<Self>) {
         if let Some((worktree, entry)) = self.selected_entry(cx) {
             if entry.is_dir() {
+                let worktree_id = worktree.id();
+                let entry_id = entry.id;
                 let expanded_dir_ids =
-                    if let Some(expanded_dir_ids) = self.expanded_dir_ids.get_mut(&worktree.id()) {
+                    if let Some(expanded_dir_ids) = self.expanded_dir_ids.get_mut(&worktree_id) {
                         expanded_dir_ids
                     } else {
                         return;
                     };
 
-                match expanded_dir_ids.binary_search(&entry.id) {
+                match expanded_dir_ids.binary_search(&entry_id) {
                     Ok(_) => self.select_next(&SelectNext, cx),
                     Err(ix) => {
-                        expanded_dir_ids.insert(ix, entry.id);
+                        self.project.update(cx, |project, cx| {
+                            project.mark_entry_expanded(worktree_id, entry_id, cx);
+                        });
+
+                        expanded_dir_ids.insert(ix, entry_id);
                         self.update_visible_entries(None, cx);
                         cx.notify();
                     }
@@ -468,14 +474,18 @@ impl ProjectPanel {
     fn toggle_expanded(&mut self, entry_id: ProjectEntryId, cx: &mut ViewContext<Self>) {
         if let Some(worktree_id) = self.project.read(cx).worktree_id_for_entry(entry_id, cx) {
             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.project.update(cx, |project, cx| {
+                    match expanded_dir_ids.binary_search(&entry_id) {
+                        Ok(ix) => {
+                            project.mark_entry_collapsed(worktree_id, entry_id, cx);
+                            expanded_dir_ids.remove(ix);
+                        }
+                        Err(ix) => {
+                            project.mark_entry_expanded(worktree_id, entry_id, cx);
+                            expanded_dir_ids.insert(ix, entry_id);
+                        }
                     }
-                }
+                });
                 self.update_visible_entries(Some((worktree_id, entry_id)), cx);
                 cx.focus_self();
                 cx.notify();
@@ -1206,7 +1216,7 @@ impl ProjectPanel {
 
         Flex::row()
             .with_child(
-                if kind == EntryKind::Dir {
+                if kind.is_dir() {
                     if details.is_expanded {
                         Svg::new("icons/chevron_down_8.svg").with_color(style.icon_color)
                     } else {
@@ -1303,7 +1313,7 @@ impl ProjectPanel {
         })
         .on_click(MouseButton::Left, move |event, this, cx| {
             if !show_editor {
-                if kind == EntryKind::Dir {
+                if kind.is_dir() {
                     this.toggle_expanded(entry_id, cx);
                 } else {
                     this.open_entry(entry_id, event.click_count > 1, cx);