From eb05103d2decbbd8589c4f506dae1996e33f3d6e Mon Sep 17 00:00:00 2001 From: Max Brunsfeld Date: Fri, 30 Jul 2021 11:39:59 -0700 Subject: [PATCH] Fix saving an untitled file outside of any existing worktree --- zed/src/file_finder.rs | 220 +++++++++++++++++--------------------- zed/src/workspace.rs | 25 ++++- zed/src/worktree.rs | 49 ++++----- zed/src/worktree/fuzzy.rs | 50 ++++----- 4 files changed, 160 insertions(+), 184 deletions(-) diff --git a/zed/src/file_finder.rs b/zed/src/file_finder.rs index d7f0f8fff1..26820b3f59 100644 --- a/zed/src/file_finder.rs +++ b/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 { + 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 - }); - - 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") - }, + .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)); + } + + 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, String, Vec)> { - self.worktree(path_match.tree_id, cx).map(|tree| { - let prefix = if path_match.include_root_name { - tree.root_name() - } else { - "" - }; + fn labels_for_match(&self, path_match: &PathMatch) -> (String, Vec, String, Vec) { + 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 path_string = path_match.path.to_string_lossy(); - let full_path = [prefix, path_string.as_ref()].join(""); - let path_positions = path_match.positions.clone(); + 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(); - 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(); - - (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) { @@ -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"); diff --git a/zed/src/workspace.rs b/zed/src/workspace.rs index 54e5f11f2c..682206f5e6 100644 --- a/zed/src/workspace.rs +++ b/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::(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::() - .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)); }); } diff --git a/zed/src/worktree.rs b/zed/src/worktree.rs index 041a94770d..03032c9116 100644 --- a/zed/src/worktree.rs +++ b/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 = 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(), diff --git a/zed/src/worktree/fuzzy.rs b/zed/src/worktree/fuzzy.rs index 0fb406fc98..8018aa829d 100644 --- a/zed/src/worktree/fuzzy.rs +++ b/zed/src/worktree/fuzzy.rs @@ -24,7 +24,7 @@ pub struct PathMatch { pub positions: Vec, pub tree_id: usize, pub path: Arc, - pub include_root_name: bool, + pub path_prefix: Arc, } 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, background: Arc, -) -> Vec -where - T: Clone + Send + Iterator + 'a, -{ +) -> Vec { 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 = + 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, path_entries: impl Iterator>, 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::>(); + let prefix = path_prefix.chars().collect::>(); 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[..],