Fix saving an untitled file outside of any existing worktree

Max Brunsfeld created

Change summary

zed/src/file_finder.rs    | 218 +++++++++++++++++-----------------------
zed/src/workspace.rs      |  25 +++
zed/src/worktree.rs       |  49 +++-----
zed/src/worktree/fuzzy.rs |  50 ++++-----
4 files changed, 159 insertions(+), 183 deletions(-)

Detailed changes

zed/src/file_finder.rs 🔗

@@ -3,7 +3,7 @@ use crate::{
     settings::Settings,
     util,
     workspace::Workspace,
-    worktree::{match_paths, PathMatch, Worktree},
+    worktree::{match_paths, PathMatch},
 };
 use gpui::{
     color::ColorF,
@@ -132,9 +132,12 @@ impl FileFinder {
                 let finder = finder.read(cx);
                 let start = range.start;
                 range.end = cmp::min(range.end, finder.matches.len());
-                items.extend(finder.matches[range].iter().enumerate().filter_map(
-                    move |(i, path_match)| finder.render_match(path_match, start + i, cx),
-                ));
+                items.extend(
+                    finder.matches[range]
+                        .iter()
+                        .enumerate()
+                        .map(move |(i, path_match)| finder.render_match(path_match, start + i)),
+                );
             },
         );
 
@@ -143,128 +146,108 @@ impl FileFinder {
             .named("matches")
     }
 
-    fn render_match(
-        &self,
-        path_match: &PathMatch,
-        index: usize,
-        cx: &AppContext,
-    ) -> Option<ElementBox> {
+    fn render_match(&self, path_match: &PathMatch, index: usize) -> ElementBox {
         let settings = self.settings.borrow();
         let theme = &settings.theme.ui;
-        self.labels_for_match(path_match, cx).map(
-            |(file_name, file_name_positions, full_path, full_path_positions)| {
-                let bold = *Properties::new().weight(Weight::BOLD);
-                let selected_index = self.selected_index();
-                let mut container = Container::new(
-                    Flex::row()
-                        .with_child(
-                            Container::new(
-                                LineBox::new(
+        let (file_name, file_name_positions, full_path, full_path_positions) =
+            self.labels_for_match(path_match);
+        let bold = *Properties::new().weight(Weight::BOLD);
+        let selected_index = self.selected_index();
+        let mut container = Container::new(
+            Flex::row()
+                .with_child(
+                    Container::new(
+                        LineBox::new(
+                            settings.ui_font_family,
+                            settings.ui_font_size,
+                            Svg::new("icons/file-16.svg").boxed(),
+                        )
+                        .boxed(),
+                    )
+                    .with_padding_right(6.0)
+                    .boxed(),
+                )
+                .with_child(
+                    Expanded::new(
+                        1.0,
+                        Flex::column()
+                            .with_child(
+                                Label::new(
+                                    file_name.to_string(),
                                     settings.ui_font_family,
                                     settings.ui_font_size,
-                                    Svg::new("icons/file-16.svg").boxed(),
+                                )
+                                .with_default_color(theme.modal_match_text.0)
+                                .with_highlights(
+                                    theme.modal_match_text_highlight.0,
+                                    bold,
+                                    file_name_positions,
                                 )
                                 .boxed(),
                             )
-                            .with_padding_right(6.0)
-                            .boxed(),
-                        )
-                        .with_child(
-                            Expanded::new(
-                                1.0,
-                                Flex::column()
-                                    .with_child(
-                                        Label::new(
-                                            file_name.to_string(),
-                                            settings.ui_font_family,
-                                            settings.ui_font_size,
-                                        )
-                                        .with_default_color(theme.modal_match_text.0)
-                                        .with_highlights(
-                                            theme.modal_match_text_highlight.0,
-                                            bold,
-                                            file_name_positions,
-                                        )
-                                        .boxed(),
-                                    )
-                                    .with_child(
-                                        Label::new(
-                                            full_path,
-                                            settings.ui_font_family,
-                                            settings.ui_font_size,
-                                        )
-                                        .with_default_color(theme.modal_match_text.0)
-                                        .with_highlights(
-                                            theme.modal_match_text_highlight.0,
-                                            bold,
-                                            full_path_positions,
-                                        )
-                                        .boxed(),
-                                    )
-                                    .boxed(),
+                            .with_child(
+                                Label::new(
+                                    full_path,
+                                    settings.ui_font_family,
+                                    settings.ui_font_size,
+                                )
+                                .with_default_color(theme.modal_match_text.0)
+                                .with_highlights(
+                                    theme.modal_match_text_highlight.0,
+                                    bold,
+                                    full_path_positions,
+                                )
+                                .boxed(),
                             )
                             .boxed(),
-                        )
-                        .boxed(),
+                    )
+                    .boxed(),
                 )
-                .with_uniform_padding(6.0)
-                .with_background_color(if index == selected_index {
-                    theme.modal_match_background_active.0
-                } else {
-                    theme.modal_match_background.0
-                });
+                .boxed(),
+        )
+        .with_uniform_padding(6.0)
+        .with_background_color(if index == selected_index {
+            theme.modal_match_background_active.0
+        } else {
+            theme.modal_match_background.0
+        });
 
-                if index == selected_index || index < self.matches.len() - 1 {
-                    container =
-                        container.with_border(Border::bottom(1.0, theme.modal_match_border));
-                }
+        if index == selected_index || index < self.matches.len() - 1 {
+            container = container.with_border(Border::bottom(1.0, theme.modal_match_border));
+        }
 
-                let entry = (path_match.tree_id, path_match.path.clone());
-                EventHandler::new(container.boxed())
-                    .on_mouse_down(move |cx| {
-                        cx.dispatch_action("file_finder:select", entry.clone());
-                        true
-                    })
-                    .named("match")
-            },
-        )
+        let entry = (path_match.tree_id, path_match.path.clone());
+        EventHandler::new(container.boxed())
+            .on_mouse_down(move |cx| {
+                cx.dispatch_action("file_finder:select", entry.clone());
+                true
+            })
+            .named("match")
     }
 
-    fn labels_for_match(
-        &self,
-        path_match: &PathMatch,
-        cx: &AppContext,
-    ) -> Option<(String, Vec<usize>, String, Vec<usize>)> {
-        self.worktree(path_match.tree_id, cx).map(|tree| {
-            let prefix = if path_match.include_root_name {
-                tree.root_name()
-            } else {
-                ""
-            };
-
-            let path_string = path_match.path.to_string_lossy();
-            let full_path = [prefix, path_string.as_ref()].join("");
-            let path_positions = path_match.positions.clone();
+    fn labels_for_match(&self, path_match: &PathMatch) -> (String, Vec<usize>, String, Vec<usize>) {
+        let path_string = path_match.path.to_string_lossy();
+        let full_path = [path_match.path_prefix.as_ref(), path_string.as_ref()].join("");
+        let path_positions = path_match.positions.clone();
 
-            let file_name = path_match.path.file_name().map_or_else(
-                || prefix.to_string(),
-                |file_name| file_name.to_string_lossy().to_string(),
-            );
-            let file_name_start =
-                prefix.chars().count() + path_string.chars().count() - file_name.chars().count();
-            let file_name_positions = path_positions
-                .iter()
-                .filter_map(|pos| {
-                    if pos >= &file_name_start {
-                        Some(pos - file_name_start)
-                    } else {
-                        None
-                    }
-                })
-                .collect();
+        let file_name = path_match.path.file_name().map_or_else(
+            || path_match.path_prefix.to_string(),
+            |file_name| file_name.to_string_lossy().to_string(),
+        );
+        let file_name_start = path_match.path_prefix.chars().count() + path_string.chars().count()
+            - file_name.chars().count();
+        let file_name_positions = path_positions
+            .iter()
+            .filter_map(|pos| {
+                if pos >= &file_name_start {
+                    Some(pos - file_name_start)
+                } else {
+                    None
+                }
+            })
+            .collect();
 
-            (file_name, file_name_positions, full_path, path_positions)
-        })
+        (file_name, file_name_positions, full_path, path_positions)
     }
 
     fn toggle(workspace_view: &mut Workspace, _: &(), cx: &mut ViewContext<Workspace>) {
@@ -418,11 +401,9 @@ impl FileFinder {
         self.cancel_flag = Arc::new(AtomicBool::new(false));
         let cancel_flag = self.cancel_flag.clone();
         Some(cx.spawn(|this, mut cx| async move {
-            let include_root_name = snapshots.len() > 1;
             let matches = match_paths(
-                snapshots.iter(),
+                &snapshots,
                 &query,
-                include_root_name,
                 false,
                 false,
                 100,
@@ -455,15 +436,6 @@ impl FileFinder {
             cx.notify();
         }
     }
-
-    fn worktree<'a>(&'a self, tree_id: usize, cx: &'a AppContext) -> Option<&'a Worktree> {
-        self.workspace
-            .upgrade(cx)?
-            .read(cx)
-            .worktrees()
-            .get(&tree_id)
-            .map(|worktree| worktree.read(cx))
-    }
 }
 
 #[cfg(test)]
@@ -651,7 +623,7 @@ mod tests {
             assert_eq!(finder.matches.len(), 1);
 
             let (file_name, file_name_positions, full_path, full_path_positions) =
-                finder.labels_for_match(&finder.matches[0], cx).unwrap();
+                finder.labels_for_match(&finder.matches[0]);
             assert_eq!(file_name, "the-file");
             assert_eq!(file_name_positions, &[0, 1, 4]);
             assert_eq!(full_path, "the-file");

zed/src/workspace.rs 🔗

@@ -1322,14 +1322,31 @@ mod tests {
         cx.dispatch_global_action("workspace:new_file", app_state);
         let window_id = *cx.window_ids().first().unwrap();
         let workspace = cx.root_view::<Workspace>(window_id).unwrap();
-        workspace.update(&mut cx, |workspace, cx| {
-            let editor = workspace
+        let editor = workspace.update(&mut cx, |workspace, cx| {
+            workspace
                 .active_item(cx)
                 .unwrap()
                 .to_any()
                 .downcast::<Editor>()
-                .unwrap();
-            assert!(editor.update(cx, |editor, cx| editor.text(cx).is_empty()));
+                .unwrap()
+        });
+
+        editor.update(&mut cx, |editor, cx| {
+            assert!(editor.text(cx).is_empty());
+        });
+
+        workspace.update(&mut cx, |workspace, cx| workspace.save_active_item(&(), cx));
+
+        let dir = TempDir::new("test-new-empty-workspace").unwrap();
+        cx.simulate_new_path_selection(|_| {
+            Some(dir.path().canonicalize().unwrap().join("the-new-name"))
+        });
+
+        editor
+            .condition(&cx, |editor, cx| editor.title(cx) == "the-new-name")
+            .await;
+        editor.update(&mut cx, |editor, cx| {
+            assert!(!editor.is_dirty(cx));
         });
     }
 

zed/src/worktree.rs 🔗

@@ -586,17 +586,11 @@ impl LocalWorktree {
 
         // After determining whether the root entry is a file or a directory, populate the
         // snapshot's "root name", which will be used for the purpose of fuzzy matching.
-        let mut root_name = abs_path
+        let root_name = abs_path
             .file_name()
             .map_or(String::new(), |f| f.to_string_lossy().to_string());
         let root_char_bag = root_name.chars().map(|c| c.to_ascii_lowercase()).collect();
-        let metadata = fs
-            .metadata(&abs_path)
-            .await?
-            .ok_or_else(|| anyhow!("root entry does not exist"))?;
-        if metadata.is_dir {
-            root_name.push('/');
-        }
+        let metadata = fs.metadata(&abs_path).await?;
 
         let (scan_states_tx, scan_states_rx) = smol::channel::unbounded();
         let (mut last_scan_state_tx, last_scan_state_rx) = watch::channel_with(ScanState::Scanning);
@@ -613,12 +607,14 @@ impl LocalWorktree {
                 removed_entry_ids: Default::default(),
                 next_entry_id: Arc::new(next_entry_id),
             };
-            snapshot.insert_entry(Entry::new(
-                path.into(),
-                &metadata,
-                &snapshot.next_entry_id,
-                snapshot.root_char_bag,
-            ));
+            if let Some(metadata) = metadata {
+                snapshot.insert_entry(Entry::new(
+                    path.into(),
+                    &metadata,
+                    &snapshot.next_entry_id,
+                    snapshot.root_char_bag,
+                ));
+            }
 
             let tree = Self {
                 snapshot: snapshot.clone(),
@@ -1229,12 +1225,10 @@ impl Snapshot {
         ChildEntriesIter::new(path, self)
     }
 
-    pub fn root_entry(&self) -> &Entry {
-        self.entry_for_path("").unwrap()
+    pub fn root_entry(&self) -> Option<&Entry> {
+        self.entry_for_path("")
     }
 
-    /// Returns the filename of the snapshot's root, plus a trailing slash if the snapshot's root is
-    /// a directory.
     pub fn root_name(&self) -> &str {
         &self.root_name
     }
@@ -1856,8 +1850,8 @@ impl BackgroundScanner {
             let snapshot = self.snapshot.lock();
             root_char_bag = snapshot.root_char_bag;
             next_entry_id = snapshot.next_entry_id.clone();
-            is_dir = snapshot.root_entry().is_dir();
-        }
+            is_dir = snapshot.root_entry().map_or(false, |e| e.is_dir())
+        };
 
         if is_dir {
             let path: Arc<Path> = Arc::from(Path::new(""));
@@ -2605,24 +2599,22 @@ mod tests {
 
         cx.read(|cx| tree.read(cx).as_local().unwrap().scan_complete())
             .await;
-        let snapshot = cx.read(|cx| {
+        let snapshots = [cx.read(|cx| {
             let tree = tree.read(cx);
             assert_eq!(tree.file_count(), 5);
             assert_eq!(
                 tree.inode_for_path("fennel/grape"),
                 tree.inode_for_path("finnochio/grape")
             );
-
             tree.snapshot()
-        });
+        })];
         let results = cx
             .read(|cx| {
                 match_paths(
-                    Some(&snapshot).into_iter(),
+                    &snapshots,
                     "bna",
                     false,
                     false,
-                    false,
                     10,
                     Default::default(),
                     cx.background().clone(),
@@ -2662,19 +2654,18 @@ mod tests {
 
         cx.read(|cx| tree.read(cx).as_local().unwrap().scan_complete())
             .await;
-        let snapshot = cx.read(|cx| {
+        let snapshots = [cx.read(|cx| {
             let tree = tree.read(cx);
             assert_eq!(tree.file_count(), 0);
             tree.snapshot()
-        });
+        })];
         let results = cx
             .read(|cx| {
                 match_paths(
-                    Some(&snapshot).into_iter(),
+                    &snapshots,
                     "dir",
                     false,
                     false,
-                    false,
                     10,
                     Default::default(),
                     cx.background().clone(),

zed/src/worktree/fuzzy.rs 🔗

@@ -24,7 +24,7 @@ pub struct PathMatch {
     pub positions: Vec<usize>,
     pub tree_id: usize,
     pub path: Arc<Path>,
-    pub include_root_name: bool,
+    pub path_prefix: Arc<str>,
 }
 
 impl PartialEq for PathMatch {
@@ -51,23 +51,19 @@ impl Ord for PathMatch {
     }
 }
 
-pub async fn match_paths<'a, T>(
-    snapshots: T,
+pub async fn match_paths(
+    snapshots: &[Snapshot],
     query: &str,
-    include_root_name: bool,
     include_ignored: bool,
     smart_case: bool,
     max_results: usize,
     cancel_flag: Arc<AtomicBool>,
     background: Arc<executor::Background>,
-) -> Vec<PathMatch>
-where
-    T: Clone + Send + Iterator<Item = &'a Snapshot> + 'a,
-{
+) -> Vec<PathMatch> {
     let path_count: usize = if include_ignored {
-        snapshots.clone().map(Snapshot::file_count).sum()
+        snapshots.iter().map(Snapshot::file_count).sum()
     } else {
-        snapshots.clone().map(Snapshot::visible_file_count).sum()
+        snapshots.iter().map(Snapshot::visible_file_count).sum()
     };
     if path_count == 0 {
         return Vec::new();
@@ -89,7 +85,6 @@ where
     background
         .scoped(|scope| {
             for (segment_idx, results) in segment_results.iter_mut().enumerate() {
-                let snapshots = snapshots.clone();
                 let cancel_flag = &cancel_flag;
                 scope.spawn(async move {
                     let segment_start = segment_idx * segment_size;
@@ -111,9 +106,16 @@ where
                             tree_start + snapshot.visible_file_count()
                         };
 
-                        let include_root_name =
-                            include_root_name || snapshot.root_entry().is_file();
                         if tree_start < segment_end && segment_start < tree_end {
+                            let path_prefix: Arc<str> =
+                                if snapshot.root_entry().map_or(false, |e| e.is_file()) {
+                                    snapshot.root_name().into()
+                                } else if snapshots.len() > 1 {
+                                    format!("{}/", snapshot.root_name()).into()
+                                } else {
+                                    "".into()
+                                };
+
                             let start = max(tree_start, segment_start) - tree_start;
                             let end = min(tree_end, segment_end) - tree_start;
                             let entries = if include_ignored {
@@ -134,7 +136,7 @@ where
 
                             match_single_tree_paths(
                                 snapshot,
-                                include_root_name,
+                                path_prefix,
                                 paths,
                                 query,
                                 lowercase_query,
@@ -173,7 +175,7 @@ where
 
 fn match_single_tree_paths<'a>(
     snapshot: &Snapshot,
-    include_root_name: bool,
+    path_prefix: Arc<str>,
     path_entries: impl Iterator<Item = MatchCandidate<'a>>,
     query: &[char],
     lowercase_query: &[char],
@@ -191,13 +193,7 @@ fn match_single_tree_paths<'a>(
     let mut path_chars = Vec::new();
     let mut lowercase_path_chars = Vec::new();
 
-    let prefix = if include_root_name {
-        snapshot.root_name()
-    } else {
-        ""
-    }
-    .chars()
-    .collect::<Vec<_>>();
+    let prefix = path_prefix.chars().collect::<Vec<_>>();
     let lowercase_prefix = prefix
         .iter()
         .map(|c| c.to_ascii_lowercase())
@@ -223,7 +219,7 @@ fn match_single_tree_paths<'a>(
             last_positions,
             &lowercase_prefix,
             &lowercase_path_chars,
-            &lowercase_query[..],
+            lowercase_query,
         ) {
             continue;
         }
@@ -235,8 +231,8 @@ fn match_single_tree_paths<'a>(
         best_position_matrix.resize(matrix_len, 0);
 
         let score = score_match(
-            &query[..],
-            &lowercase_query[..],
+            query,
+            lowercase_query,
             &path_chars,
             &lowercase_path_chars,
             &prefix,
@@ -253,9 +249,9 @@ fn match_single_tree_paths<'a>(
             let mat = PathMatch {
                 tree_id: snapshot.id,
                 path: candidate.path.clone(),
+                path_prefix: path_prefix.clone(),
                 score,
                 positions: match_positions.clone(),
-                include_root_name,
             };
             if let Err(i) = results.binary_search_by(|m| mat.cmp(&m)) {
                 if results.len() < max_results {
@@ -630,7 +626,7 @@ mod tests {
                 root_char_bag: Default::default(),
                 next_entry_id: Default::default(),
             },
-            false,
+            "".into(),
             path_entries.into_iter(),
             &query[..],
             &lowercase_query[..],