From 364631a155f5775b951f8295a2c70a6f7cfb4887 Mon Sep 17 00:00:00 2001 From: Kirill Bulatov Date: Fri, 26 May 2023 14:03:44 +0300 Subject: [PATCH 1/7] Add absolute paths to historic elements --- crates/file_finder/src/file_finder.rs | 136 ++++++++++++++++++-------- 1 file changed, 94 insertions(+), 42 deletions(-) diff --git a/crates/file_finder/src/file_finder.rs b/crates/file_finder/src/file_finder.rs index c4fce491523134e2b7f64678cd89da121fff251f..13dc46382f4e58f14060ffae260ae68a5f1f2eec 100644 --- a/crates/file_finder/src/file_finder.rs +++ b/crates/file_finder/src/file_finder.rs @@ -6,7 +6,7 @@ use gpui::{ use picker::{Picker, PickerDelegate}; use project::{PathMatchCandidateSet, Project, ProjectPath, WorktreeId}; use std::{ - path::Path, + path::{Path, PathBuf}, sync::{ atomic::{self, AtomicBool}, Arc, @@ -25,11 +25,17 @@ pub struct FileFinderDelegate { latest_search_id: usize, latest_search_did_cancel: bool, latest_search_query: Option>, - currently_opened_path: Option, + currently_opened_path: Option, matches: Vec, selected: Option<(usize, Arc)>, cancel_flag: Arc, - history_items: Vec, + history_items: Vec, +} + +#[derive(Debug, Clone, PartialEq, Eq)] +struct FoundPath { + project: ProjectPath, + absolute: Option, } actions!(file_finder, [Toggle]); @@ -43,10 +49,36 @@ const MAX_RECENT_SELECTIONS: usize = 20; fn toggle_file_finder(workspace: &mut Workspace, _: &Toggle, cx: &mut ViewContext) { workspace.toggle_modal(cx, |workspace, cx| { - let history_items = workspace.recent_navigation_history(Some(MAX_RECENT_SELECTIONS), cx); + let project = workspace.project().read(cx); + let project_to_found_path = |project_path: ProjectPath| FoundPath { + absolute: project + .worktree_for_id(project_path.worktree_id, cx) + .map(|worktree| worktree.read(cx).abs_path().join(&project_path.path)), + project: project_path, + }; + let currently_opened_path = workspace .active_item(cx) - .and_then(|item| item.project_path(cx)); + .and_then(|item| item.project_path(cx)) + .map(project_to_found_path); + + // if exists, bubble the currently opened path to the top + let history_items = currently_opened_path + .clone() + .into_iter() + .chain( + workspace + .recent_navigation_history(Some(MAX_RECENT_SELECTIONS), cx) + .into_iter() + .filter(|history_path| { + Some(history_path) + != currently_opened_path + .as_ref() + .map(|found_path| &found_path.project) + }) + .map(project_to_found_path), + ) + .collect(); let project = workspace.project().clone(); let workspace = cx.handle().downgrade(); @@ -113,11 +145,11 @@ impl FileFinderDelegate { (file_name, file_name_positions, full_path, path_positions) } - pub fn new( + fn new( workspace: WeakViewHandle, project: ModelHandle, - currently_opened_path: Option, - history_items: Vec, + currently_opened_path: Option, + history_items: Vec, cx: &mut ViewContext, ) -> Self { cx.observe(&project, |picker, _, cx| { @@ -147,7 +179,7 @@ impl FileFinderDelegate { let relative_to = self .currently_opened_path .as_ref() - .map(|project_path| Arc::clone(&project_path.path)); + .map(|found_path| Arc::clone(&found_path.project.path)); let worktrees = self .project .read(cx) @@ -255,20 +287,33 @@ impl PickerDelegate for FileFinderDelegate { self.latest_search_id = post_inc(&mut self.search_count); self.matches.clear(); + let project = self.project.read(cx); self.matches = self - .currently_opened_path - .iter() // if exists, bubble the currently opened path to the top - .chain(self.history_items.iter().filter(|history_item| { - Some(*history_item) != self.currently_opened_path.as_ref() - })) + .history_items + .iter() .enumerate() - .map(|(i, history_item)| PathMatch { - score: i as f64, - positions: Vec::new(), - worktree_id: history_item.worktree_id.to_usize(), - path: Arc::clone(&history_item.path), - path_prefix: "".into(), - distance_to_relative_ancestor: usize::MAX, + .map(|(i, found_path)| { + let worktree_id = found_path.project.worktree_id; + // TODO kb wrong: + // * for no worktree, check the project path + // * if no project path exists — filter out(?), otherwise take it and open a workspace for it (enum for match kinds?) + let path = if project.worktree_for_id(worktree_id, cx).is_some() { + Arc::clone(&found_path.project.path) + } else { + found_path + .absolute + .as_ref() + .map(|abs_path| Arc::from(abs_path.as_path())) + .unwrap_or_else(|| Arc::clone(&found_path.project.path)) + }; + PathMatch { + score: i as f64, + positions: Vec::new(), + worktree_id: worktree_id.to_usize(), + path, + path_prefix: "".into(), + distance_to_relative_ancestor: usize::MAX, + } }) .collect(); cx.notify(); @@ -876,10 +921,10 @@ mod tests { // When workspace has an active item, sort items which are closer to that item // first when they have the same name. In this case, b.txt is closer to dir2's a.txt // so that one should be sorted earlier - let b_path = Some(ProjectPath { + let b_path = Some(dummy_found_path(ProjectPath { worktree_id, path: Arc::from(Path::new("/root/dir2/b.txt")), - }); + })); let (_, finder) = cx.add_window(|cx| { Picker::new( FileFinderDelegate::new( @@ -1012,10 +1057,10 @@ mod tests { .await; assert_eq!( history_after_first, - vec![ProjectPath { + vec![dummy_found_path(ProjectPath { worktree_id, path: Arc::from(Path::new("test/first.rs")), - }], + })], "Should show 1st opened item in the history when opening the 2nd item" ); @@ -1032,14 +1077,14 @@ mod tests { assert_eq!( history_after_second, vec![ - ProjectPath { + dummy_found_path(ProjectPath { worktree_id, path: Arc::from(Path::new("test/second.rs")), - }, - ProjectPath { + }), + dummy_found_path(ProjectPath { worktree_id, path: Arc::from(Path::new("test/first.rs")), - }, + }), ], "Should show 1st and 2nd opened items in the history when opening the 3rd item. \ 2nd item should be the first in the history, as the last opened." @@ -1058,18 +1103,18 @@ mod tests { assert_eq!( history_after_third, vec![ - ProjectPath { + dummy_found_path(ProjectPath { worktree_id, path: Arc::from(Path::new("test/third.rs")), - }, - ProjectPath { + }), + dummy_found_path(ProjectPath { worktree_id, path: Arc::from(Path::new("test/second.rs")), - }, - ProjectPath { + }), + dummy_found_path(ProjectPath { worktree_id, path: Arc::from(Path::new("test/first.rs")), - }, + }), ], "Should show 1st, 2nd and 3rd opened items in the history when opening the 2nd item again. \ 3rd item should be the first in the history, as the last opened." @@ -1088,18 +1133,18 @@ mod tests { assert_eq!( history_after_second_again, vec![ - ProjectPath { + dummy_found_path(ProjectPath { worktree_id, path: Arc::from(Path::new("test/second.rs")), - }, - ProjectPath { + }), + dummy_found_path(ProjectPath { worktree_id, path: Arc::from(Path::new("test/third.rs")), - }, - ProjectPath { + }), + dummy_found_path(ProjectPath { worktree_id, path: Arc::from(Path::new("test/first.rs")), - }, + }), ], "Should show 1st, 2nd and 3rd opened items in the history when opening the 3rd item again. \ 2nd item, as the last opened, 3rd item should go next as it was opened right before." @@ -1114,7 +1159,7 @@ mod tests { workspace: &ViewHandle, deterministic: &gpui::executor::Deterministic, cx: &mut gpui::TestAppContext, - ) -> Vec { + ) -> Vec { cx.dispatch_action(window_id, Toggle); let finder = cx.read(|cx| workspace.read(cx).modal::().unwrap()); finder @@ -1216,4 +1261,11 @@ mod tests { }) .unwrap() } + + fn dummy_found_path(project_path: ProjectPath) -> FoundPath { + FoundPath { + project: project_path, + absolute: None, + } + } } From b75c27da6fc93311698e809803f9c65afcecd919 Mon Sep 17 00:00:00 2001 From: Kirill Bulatov Date: Fri, 26 May 2023 14:15:58 +0300 Subject: [PATCH 2/7] Simplify selected index handling --- crates/file_finder/src/file_finder.rs | 22 ++++++---------------- 1 file changed, 6 insertions(+), 16 deletions(-) diff --git a/crates/file_finder/src/file_finder.rs b/crates/file_finder/src/file_finder.rs index 13dc46382f4e58f14060ffae260ae68a5f1f2eec..5a9d5017462713f9a24a1734b37e031ebb059cb8 100644 --- a/crates/file_finder/src/file_finder.rs +++ b/crates/file_finder/src/file_finder.rs @@ -6,7 +6,7 @@ use gpui::{ use picker::{Picker, PickerDelegate}; use project::{PathMatchCandidateSet, Project, ProjectPath, WorktreeId}; use std::{ - path::{Path, PathBuf}, + path::PathBuf, sync::{ atomic::{self, AtomicBool}, Arc, @@ -27,7 +27,7 @@ pub struct FileFinderDelegate { latest_search_query: Option>, currently_opened_path: Option, matches: Vec, - selected: Option<(usize, Arc)>, + selected_index: Option, cancel_flag: Arc, history_items: Vec, } @@ -165,7 +165,7 @@ impl FileFinderDelegate { latest_search_query: None, currently_opened_path, matches: Vec::new(), - selected: None, + selected_index: None, cancel_flag: Arc::new(AtomicBool::new(false)), history_items, } @@ -264,21 +264,11 @@ impl PickerDelegate for FileFinderDelegate { } fn selected_index(&self) -> usize { - if let Some(selected) = self.selected.as_ref() { - for (ix, path_match) in self.matches.iter().enumerate() { - if (path_match.worktree_id, path_match.path.as_ref()) - == (selected.0, selected.1.as_ref()) - { - return ix; - } - } - } - 0 + self.selected_index.unwrap_or(0) } fn set_selected_index(&mut self, ix: usize, cx: &mut ViewContext) { - let mat = &self.matches[ix]; - self.selected = Some((mat.worktree_id, mat.path.clone())); + self.selected_index = Some(ix); cx.notify(); } @@ -418,7 +408,7 @@ impl PickerDelegate for FileFinderDelegate { #[cfg(test)] mod tests { - use std::{assert_eq, collections::HashMap, time::Duration}; + use std::{assert_eq, collections::HashMap, path::Path, time::Duration}; use super::*; use editor::Editor; From 2fdc960704c18f6cfe02b5a0952df6ce98059aff Mon Sep 17 00:00:00 2001 From: Kirill Bulatov Date: Fri, 26 May 2023 15:44:44 +0300 Subject: [PATCH 3/7] Properly display labels for currently opened external files --- crates/ai/src/ai.rs | 2 +- crates/file_finder/src/file_finder.rs | 306 +++++++++++++++++++------- crates/workspace/src/workspace.rs | 30 +++ 3 files changed, 255 insertions(+), 83 deletions(-) diff --git a/crates/ai/src/ai.rs b/crates/ai/src/ai.rs index c68f41c6bf1cc39d20cc2a9f94dec899a5854516..2a0110510f8481f5f9a84788ab6d51929b4f2363 100644 --- a/crates/ai/src/ai.rs +++ b/crates/ai/src/ai.rs @@ -240,7 +240,7 @@ impl Assistant { .assist_stacks .entry(cx.view_id()) .or_default() - .push((dbg!(assist_id), assist_task)); + .push((assist_id, assist_task)); Ok(()) } diff --git a/crates/file_finder/src/file_finder.rs b/crates/file_finder/src/file_finder.rs index 5a9d5017462713f9a24a1734b37e031ebb059cb8..111e410f6f9a15341c17c3a2bb2421045370872c 100644 --- a/crates/file_finder/src/file_finder.rs +++ b/crates/file_finder/src/file_finder.rs @@ -6,7 +6,7 @@ use gpui::{ use picker::{Picker, PickerDelegate}; use project::{PathMatchCandidateSet, Project, ProjectPath, WorktreeId}; use std::{ - path::PathBuf, + path::{Path, PathBuf}, sync::{ atomic::{self, AtomicBool}, Arc, @@ -26,12 +26,45 @@ pub struct FileFinderDelegate { latest_search_did_cancel: bool, latest_search_query: Option>, currently_opened_path: Option, - matches: Vec, + matches: Matches, selected_index: Option, cancel_flag: Arc, history_items: Vec, } +enum Matches { + History(Vec), + Search(Vec), +} + +#[derive(Debug)] +enum Match<'a> { + History(&'a FoundPath), + Search(&'a PathMatch), +} + +impl Matches { + fn len(&self) -> usize { + match self { + Self::History(items) => items.len(), + Self::Search(items) => items.len(), + } + } + + fn get(&self, index: usize) -> Option> { + match self { + Self::History(items) => items.get(index).map(Match::History), + Self::Search(items) => items.get(index).map(Match::Search), + } + } +} + +impl Default for Matches { + fn default() -> Self { + Self::History(Vec::new()) + } +} + #[derive(Debug, Clone, PartialEq, Eq)] struct FoundPath { project: ProjectPath, @@ -52,6 +85,8 @@ fn toggle_file_finder(workspace: &mut Workspace, _: &Toggle, cx: &mut ViewContex let project = workspace.project().read(cx); let project_to_found_path = |project_path: ProjectPath| FoundPath { absolute: project + // TODO kb this will be called on every panel reopen, will the workspaec exist if all files are closed? + // might need to store these in the history instead .worktree_for_id(project_path.worktree_id, cx) .map(|worktree| worktree.read(cx).abs_path().join(&project_path.path)), project: project_path, @@ -119,32 +154,6 @@ impl FileSearchQuery { } impl FileFinderDelegate { - fn labels_for_match(&self, path_match: &PathMatch) -> (String, Vec, String, Vec) { - let path = &path_match.path; - let path_string = 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.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) - } - fn new( workspace: WeakViewHandle, project: ModelHandle, @@ -164,7 +173,7 @@ impl FileFinderDelegate { latest_search_did_cancel: false, latest_search_query: None, currently_opened_path, - matches: Vec::new(), + matches: Matches::default(), selected_index: None, cancel_flag: Arc::new(AtomicBool::new(false)), history_items, @@ -220,13 +229,13 @@ impl FileFinderDelegate { .update(&mut cx, |picker, cx| { picker .delegate_mut() - .set_matches(search_id, did_cancel, query, matches, cx) + .set_search_matches(search_id, did_cancel, query, matches, cx) }) .log_err(); }) } - fn set_matches( + fn set_search_matches( &mut self, search_id: usize, did_cancel: bool, @@ -243,15 +252,104 @@ impl FileFinderDelegate { .as_ref() .map(|query| query.path_like.path_query()) { - util::extend_sorted(&mut self.matches, matches.into_iter(), 100, |a, b| b.cmp(a)); + match &mut self.matches { + Matches::History(_) => self.matches = Matches::Search(matches), + Matches::Search(search_matches) => { + util::extend_sorted(search_matches, matches.into_iter(), 100, |a, b| { + b.cmp(a) + }) + } + } } else { - self.matches = matches; + self.matches = Matches::Search(matches); } self.latest_search_query = Some(query); self.latest_search_did_cancel = did_cancel; cx.notify(); } } + + fn labels_for_match( + &self, + path_match: Match, + cx: &AppContext, + ix: usize, + ) -> (String, Vec, String, Vec) { + match path_match { + Match::History(found_path) => { + let worktree_id = found_path.project.worktree_id; + let project_relative_path = &found_path.project.path; + let has_worktree = self + .project + .read(cx) + .worktree_for_id(worktree_id, cx) + .is_some(); + + if !has_worktree { + if let Some(absolute_path) = &found_path.absolute { + return ( + absolute_path + .file_name() + .map_or_else( + || project_relative_path.to_string_lossy(), + |file_name| file_name.to_string_lossy(), + ) + .to_string(), + Vec::new(), + absolute_path.to_string_lossy().to_string(), + Vec::new(), + ); + } + } + + let mut path = Arc::clone(project_relative_path); + if project_relative_path.as_ref() == Path::new("") { + if let Some(absolute_path) = &found_path.absolute { + path = Arc::from(absolute_path.as_path()); + } + } + self.labels_for_path_match(&PathMatch { + score: ix as f64, + positions: Vec::new(), + worktree_id: worktree_id.to_usize(), + path, + path_prefix: "".into(), + distance_to_relative_ancestor: usize::MAX, + }) + } + Match::Search(path_match) => self.labels_for_path_match(path_match), + } + } + + fn labels_for_path_match( + &self, + path_match: &PathMatch, + ) -> (String, Vec, String, Vec) { + let path = &path_match.path; + let path_string = path.to_string_lossy(); + // TODO kb full path could be very long, trim it to fit the panel width + 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.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) + } } impl PickerDelegate for FileFinderDelegate { @@ -274,38 +372,25 @@ impl PickerDelegate for FileFinderDelegate { fn update_matches(&mut self, raw_query: String, cx: &mut ViewContext) -> Task<()> { if raw_query.is_empty() { - self.latest_search_id = post_inc(&mut self.search_count); - self.matches.clear(); - let project = self.project.read(cx); - self.matches = self - .history_items - .iter() - .enumerate() - .map(|(i, found_path)| { - let worktree_id = found_path.project.worktree_id; - // TODO kb wrong: - // * for no worktree, check the project path - // * if no project path exists — filter out(?), otherwise take it and open a workspace for it (enum for match kinds?) - let path = if project.worktree_for_id(worktree_id, cx).is_some() { - Arc::clone(&found_path.project.path) - } else { - found_path - .absolute - .as_ref() - .map(|abs_path| Arc::from(abs_path.as_path())) - .unwrap_or_else(|| Arc::clone(&found_path.project.path)) - }; - PathMatch { - score: i as f64, - positions: Vec::new(), - worktree_id: worktree_id.to_usize(), - path, - path_prefix: "".into(), - distance_to_relative_ancestor: usize::MAX, - } - }) - .collect(); + self.latest_search_id = post_inc(&mut self.search_count); + self.matches = Matches::History( + self.history_items + .iter() + .filter(|history_item| { + project + .worktree_for_id(history_item.project.worktree_id, cx) + .is_some() + || (project.is_local() + && history_item + .absolute + .as_ref() + .filter(|abs_path| abs_path.exists()) + .is_some()) + }) + .cloned() + .collect(), + ); cx.notify(); Task::ready(()) } else { @@ -328,16 +413,52 @@ impl PickerDelegate for FileFinderDelegate { fn confirm(&mut self, cx: &mut ViewContext) { if let Some(m) = self.matches.get(self.selected_index()) { if let Some(workspace) = self.workspace.upgrade(cx) { - let project_path = ProjectPath { - worktree_id: WorktreeId::from_usize(m.worktree_id), - path: m.path.clone(), - }; - let open_task = workspace.update(cx, |workspace, cx| { - workspace.open_path(project_path.clone(), None, true, cx) + let open_task = workspace.update(cx, |workspace, cx| match m { + Match::History(history_match) => { + let worktree_id = history_match.project.worktree_id; + if workspace + .project() + .read(cx) + .worktree_for_id(worktree_id, cx) + .is_some() + { + workspace.open_path( + ProjectPath { + worktree_id, + path: Arc::clone(&history_match.project.path), + }, + None, + true, + cx, + ) + } else { + match history_match.absolute.as_ref() { + Some(abs_path) => { + workspace.open_abs_path(abs_path.to_path_buf(), true, cx) + } + None => workspace.open_path( + ProjectPath { + worktree_id, + path: Arc::clone(&history_match.project.path), + }, + None, + true, + cx, + ), + } + } + } + Match::Search(m) => workspace.open_path( + ProjectPath { + worktree_id: WorktreeId::from_usize(m.worktree_id), + path: m.path.clone(), + }, + None, + true, + cx, + ), }); - let workspace = workspace.downgrade(); - let row = self .latest_search_query .as_ref() @@ -368,6 +489,7 @@ impl PickerDelegate for FileFinderDelegate { } } workspace + .downgrade() .update(&mut cx, |workspace, cx| workspace.dismiss_modal(cx)) .log_err(); @@ -387,11 +509,14 @@ impl PickerDelegate for FileFinderDelegate { selected: bool, cx: &AppContext, ) -> AnyElement> { - let path_match = &self.matches[ix]; + let path_match = self + .matches + .get(ix) + .expect("Invalid matches state: no element for index {ix}"); let theme = theme::current(cx); let style = theme.picker.item.style_for(mouse_state, selected); let (file_name, file_name_positions, full_path, full_path_positions) = - self.labels_for_match(path_match); + self.labels_for_match(path_match, cx, ix); Flex::column() .with_child( Label::new(file_name, style.label.clone()).with_highlights(file_name_positions), @@ -684,12 +809,16 @@ mod tests { finder.update(cx, |finder, cx| { let delegate = finder.delegate_mut(); - let matches = delegate.matches.clone(); + let matches = match &delegate.matches { + Matches::Search(path_matches) => path_matches, + _ => panic!("Search matches expected"), + } + .clone(); // Simulate a search being cancelled after the time limit, // returning only a subset of the matches that would have been found. drop(delegate.spawn_search(query.clone(), cx)); - delegate.set_matches( + delegate.set_search_matches( delegate.latest_search_id, true, // did-cancel query.clone(), @@ -699,7 +828,7 @@ mod tests { // Simulate another cancellation. drop(delegate.spawn_search(query.clone(), cx)); - delegate.set_matches( + delegate.set_search_matches( delegate.latest_search_id, true, // did-cancel query.clone(), @@ -707,7 +836,12 @@ mod tests { cx, ); - assert_eq!(delegate.matches, matches[0..4]) + match &delegate.matches { + Matches::Search(new_matches) => { + assert_eq!(new_matches.as_slice(), &matches[0..4]) + } + _ => panic!("Search matches expected"), + }; }); } @@ -807,10 +941,14 @@ mod tests { cx.read(|cx| { let finder = finder.read(cx); let delegate = finder.delegate(); - assert_eq!(delegate.matches.len(), 1); + let matches = match &delegate.matches { + Matches::Search(path_matches) => path_matches, + _ => panic!("Search matches expected"), + }; + assert_eq!(matches.len(), 1); let (file_name, file_name_positions, full_path, full_path_positions) = - delegate.labels_for_match(&delegate.matches[0]); + delegate.labels_for_path_match(&matches[0]); assert_eq!(file_name, "the-file"); assert_eq!(file_name_positions, &[0, 1, 4]); assert_eq!(full_path, "the-file"); @@ -936,8 +1074,12 @@ mod tests { finder.read_with(cx, |f, _| { let delegate = f.delegate(); - assert_eq!(delegate.matches[0].path.as_ref(), Path::new("dir2/a.txt")); - assert_eq!(delegate.matches[1].path.as_ref(), Path::new("dir1/a.txt")); + let matches = match &delegate.matches { + Matches::Search(path_matches) => path_matches, + _ => panic!("Search matches expected"), + }; + assert_eq!(matches[0].path.as_ref(), Path::new("dir2/a.txt")); + assert_eq!(matches[1].path.as_ref(), Path::new("dir1/a.txt")); }); } diff --git a/crates/workspace/src/workspace.rs b/crates/workspace/src/workspace.rs index 346fa7637d7659c62d40a39b6c7b3c5b6c339bfa..64a68e39059bc71cb42158215242d4023b9227e3 100644 --- a/crates/workspace/src/workspace.rs +++ b/crates/workspace/src/workspace.rs @@ -1582,6 +1582,36 @@ impl Workspace { Pane::add_item(self, &active_pane, item, true, true, None, cx); } + pub fn open_abs_path( + &mut self, + abs_path: PathBuf, + visible: bool, + cx: &mut ViewContext, + ) -> Task>> { + cx.spawn(|workspace, mut cx| async move { + let open_paths_task_result = workspace + .update(&mut cx, |workspace, cx| { + workspace.open_paths(vec![abs_path.clone()], visible, cx) + }) + .with_context(|| format!("open abs path {abs_path:?} task spawn"))? + .await; + anyhow::ensure!( + open_paths_task_result.len() == 1, + "open abs path {abs_path:?} task returned incorrect number of results" + ); + match open_paths_task_result + .into_iter() + .next() + .expect("ensured single task result") + { + Some(open_result) => { + open_result.with_context(|| format!("open abs path {abs_path:?} task join")) + } + None => anyhow::bail!("open abs path {abs_path:?} task returned None"), + } + }) + } + pub fn open_path( &mut self, path: impl Into, From 508533ebb732efd8dd0ac532159b2637b5bdb61d Mon Sep 17 00:00:00 2001 From: Kirill Bulatov Date: Fri, 26 May 2023 17:31:35 +0300 Subject: [PATCH 4/7] Track abs paths in history --- crates/file_finder/src/file_finder.rs | 31 ++++++++------ crates/workspace/src/pane.rs | 39 ++++++++++++----- crates/workspace/src/workspace.rs | 62 ++++++++++++++++++++++----- 3 files changed, 97 insertions(+), 35 deletions(-) diff --git a/crates/file_finder/src/file_finder.rs b/crates/file_finder/src/file_finder.rs index 111e410f6f9a15341c17c3a2bb2421045370872c..d9916342beacda311b59dff73f5b9863793cc626 100644 --- a/crates/file_finder/src/file_finder.rs +++ b/crates/file_finder/src/file_finder.rs @@ -71,6 +71,12 @@ struct FoundPath { absolute: Option, } +impl FoundPath { + fn new(project: ProjectPath, absolute: Option) -> Self { + Self { project, absolute } + } +} + actions!(file_finder, [Toggle]); pub fn init(cx: &mut AppContext) { @@ -83,37 +89,34 @@ const MAX_RECENT_SELECTIONS: usize = 20; fn toggle_file_finder(workspace: &mut Workspace, _: &Toggle, cx: &mut ViewContext) { workspace.toggle_modal(cx, |workspace, cx| { let project = workspace.project().read(cx); - let project_to_found_path = |project_path: ProjectPath| FoundPath { - absolute: project - // TODO kb this will be called on every panel reopen, will the workspaec exist if all files are closed? - // might need to store these in the history instead - .worktree_for_id(project_path.worktree_id, cx) - .map(|worktree| worktree.read(cx).abs_path().join(&project_path.path)), - project: project_path, - }; let currently_opened_path = workspace .active_item(cx) .and_then(|item| item.project_path(cx)) - .map(project_to_found_path); + .map(|project_path| { + let abs_path = project + .worktree_for_id(project_path.worktree_id, cx) + .map(|worktree| worktree.read(cx).abs_path().join(&project_path.path)); + FoundPath::new(project_path, abs_path) + }); // if exists, bubble the currently opened path to the top - let history_items = currently_opened_path - .clone() + let history_items = dbg!(dbg!(currently_opened_path.clone()) .into_iter() .chain( workspace + // TODO kb history contains empty paths .recent_navigation_history(Some(MAX_RECENT_SELECTIONS), cx) .into_iter() - .filter(|history_path| { + .filter(|(history_path, _)| { Some(history_path) != currently_opened_path .as_ref() .map(|found_path| &found_path.project) }) - .map(project_to_found_path), + .map(|(history_path, abs_path)| FoundPath::new(history_path, abs_path)), ) - .collect(); + .collect()); let project = workspace.project().clone(); let workspace = cx.handle().downgrade(); diff --git a/crates/workspace/src/pane.rs b/crates/workspace/src/pane.rs index 4bb3bf61c444b2a4180b1a33a0e3edcee83a38a0..e2f1767e92bbc7788d952e7f82743b650bb96c47 100644 --- a/crates/workspace/src/pane.rs +++ b/crates/workspace/src/pane.rs @@ -31,7 +31,7 @@ use std::{ any::Any, cell::RefCell, cmp, mem, - path::Path, + path::{Path, PathBuf}, rc::Rc, sync::{ atomic::{AtomicUsize, Ordering}, @@ -180,7 +180,7 @@ struct NavHistory { backward_stack: VecDeque, forward_stack: VecDeque, closed_stack: VecDeque, - paths_by_item: HashMap, + paths_by_item: HashMap)>, pane: WeakViewHandle, next_timestamp: Arc, } @@ -482,7 +482,7 @@ impl Pane { .paths_by_item .get(&entry.item.id()) .cloned() - .map(|project_path| (project_path, entry)); + .map(|(project_path, _)| (project_path, entry)); } } }) @@ -492,7 +492,7 @@ impl Pane { if let Some((project_path, entry)) = to_load { // If the item was no longer present, then load it again from its previous path. - let task = workspace.load_path(project_path, cx); + let task = workspace.load_path(project_path.clone(), cx); cx.spawn(|workspace, mut cx| async move { let task = task.await; let mut navigated = false; @@ -510,6 +510,7 @@ impl Pane { workspace, pane.clone(), project_entry_id, + &project_path, true, cx, build_item, @@ -546,6 +547,7 @@ impl Pane { workspace: &mut Workspace, pane: ViewHandle, project_entry_id: ProjectEntryId, + project_path: &ProjectPath, focus_item: bool, cx: &mut ViewContext, build_item: impl FnOnce(&mut ViewContext) -> Box, @@ -578,6 +580,15 @@ impl Pane { None, cx, ); + { + let abs_path = workspace.absolute_path(project_path, cx); + pane.read(cx) + .nav_history + .borrow_mut() + .paths_by_item + .insert(new_item.id(), (project_path.clone(), abs_path)); + } + new_item } } @@ -1003,10 +1014,14 @@ impl Pane { .set_mode(NavigationMode::Normal); if let Some(path) = item.project_path(cx) { + let abs_path = self + .workspace() + .upgrade(cx) + .and_then(|workspace| workspace.read(cx).absolute_path(&path, cx)); self.nav_history .borrow_mut() .paths_by_item - .insert(item.id(), path); + .insert(item.id(), (path, abs_path)); } else { self.nav_history .borrow_mut() @@ -1954,7 +1969,7 @@ impl PaneNavHistory { pub fn for_each_entry( &self, cx: &AppContext, - mut f: impl FnMut(&NavigationEntry, ProjectPath), + mut f: impl FnMut(&NavigationEntry, (ProjectPath, Option)), ) { let borrowed_history = self.0.borrow(); borrowed_history @@ -1963,12 +1978,14 @@ impl PaneNavHistory { .chain(borrowed_history.backward_stack.iter()) .chain(borrowed_history.closed_stack.iter()) .for_each(|entry| { - if let Some(path) = borrowed_history.paths_by_item.get(&entry.item.id()) { - f(entry, path.clone()); + if let Some(project_and_abs_path) = + borrowed_history.paths_by_item.get(&entry.item.id()) + { + f(entry, project_and_abs_path.clone()); } else if let Some(item) = entry.item.upgrade(cx) { - let path = item.project_path(cx); - if let Some(path) = path { - f(entry, path); + if let Some(path) = item.project_path(cx) { + // TODO kb ??? this should be the full path + f(entry, (path, None)); } } }) diff --git a/crates/workspace/src/workspace.rs b/crates/workspace/src/workspace.rs index 64a68e39059bc71cb42158215242d4023b9227e3..817560fa6173258f61e3317dfcb85d72c28dc68c 100644 --- a/crates/workspace/src/workspace.rs +++ b/crates/workspace/src/workspace.rs @@ -946,31 +946,53 @@ impl Workspace { &self, limit: Option, cx: &AppContext, - ) -> Vec { - let mut history: HashMap = HashMap::default(); + ) -> Vec<(ProjectPath, Option)> { + let mut abs_paths_opened: HashMap> = HashMap::default(); + let mut history: HashMap, usize)> = HashMap::default(); for pane in &self.panes { let pane = pane.read(cx); pane.nav_history() - .for_each_entry(cx, |entry, project_path| { + .for_each_entry(cx, |entry, (project_path, fs_path)| { + if let Some(fs_path) = &fs_path { + abs_paths_opened + .entry(fs_path.clone()) + .or_default() + .insert(project_path.clone()); + } let timestamp = entry.timestamp; match history.entry(project_path) { hash_map::Entry::Occupied(mut entry) => { - if ×tamp > entry.get() { - entry.insert(timestamp); + let (old_fs_path, old_timestamp) = entry.get(); + if ×tamp > old_timestamp { + assert_eq!(&fs_path, old_fs_path, "Inconsistent nav history"); + entry.insert((fs_path, timestamp)); } } hash_map::Entry::Vacant(entry) => { - entry.insert(timestamp); + entry.insert((fs_path, timestamp)); } } }); } + let project = self.project.read(cx); history .into_iter() - .sorted_by_key(|(_, timestamp)| *timestamp) - .map(|(project_path, _)| project_path) + .sorted_by_key(|(_, (_, timestamp))| *timestamp) + .map(|(project_path, (fs_path, _))| (project_path, fs_path)) .rev() + .filter(|(history_path, abs_path)| { + project + .worktree_for_id(history_path.worktree_id, cx) + .is_some() + || abs_path + .as_ref() + .and_then(|abs_path| { + let buffers_opened = abs_paths_opened.get(abs_path)?; + Some(buffers_opened.len() < 2) + }) + .unwrap_or(false) + }) .take(limit.unwrap_or(usize::MAX)) .collect() } @@ -1283,6 +1305,17 @@ impl Workspace { }) } + pub fn absolute_path(&self, project_path: &ProjectPath, cx: &AppContext) -> Option { + Some( + self.project() + .read(cx) + .worktree_for_id(project_path.worktree_id, cx)? + .read(cx) + .abs_path() + .to_path_buf(), + ) + } + fn add_folder_to_project(&mut self, _: &AddFolderToProject, cx: &mut ViewContext) { let mut paths = cx.prompt_for_paths(PathPromptOptions { files: false, @@ -1628,14 +1661,23 @@ impl Workspace { }) }); - let task = self.load_path(path.into(), cx); + let project_path = path.into(); + let task = self.load_path(project_path.clone(), cx); cx.spawn(|this, mut cx| async move { let (project_entry_id, build_item) = task.await?; let pane = pane .upgrade(&cx) .ok_or_else(|| anyhow!("pane was closed"))?; this.update(&mut cx, |this, cx| { - Pane::open_item(this, pane, project_entry_id, focus_item, cx, build_item) + Pane::open_item( + this, + pane, + project_entry_id, + &project_path, + focus_item, + cx, + build_item, + ) }) }) } From cf2bbfc85ac4af8a20e1cf13a5f3ea79c0adb815 Mon Sep 17 00:00:00 2001 From: Kirill Bulatov Date: Fri, 26 May 2023 18:13:09 +0300 Subject: [PATCH 5/7] Better display labels for external files --- crates/file_finder/src/file_finder.rs | 6 ++--- crates/workspace/src/pane.rs | 28 ++++++++++++---------- crates/workspace/src/workspace.rs | 34 ++++++++++++--------------- 3 files changed, 33 insertions(+), 35 deletions(-) diff --git a/crates/file_finder/src/file_finder.rs b/crates/file_finder/src/file_finder.rs index d9916342beacda311b59dff73f5b9863793cc626..6617bd28883f3ff944e36e4e95e2d04437d6c078 100644 --- a/crates/file_finder/src/file_finder.rs +++ b/crates/file_finder/src/file_finder.rs @@ -101,11 +101,11 @@ fn toggle_file_finder(workspace: &mut Workspace, _: &Toggle, cx: &mut ViewContex }); // if exists, bubble the currently opened path to the top - let history_items = dbg!(dbg!(currently_opened_path.clone()) + let history_items = currently_opened_path + .clone() .into_iter() .chain( workspace - // TODO kb history contains empty paths .recent_navigation_history(Some(MAX_RECENT_SELECTIONS), cx) .into_iter() .filter(|(history_path, _)| { @@ -116,7 +116,7 @@ fn toggle_file_finder(workspace: &mut Workspace, _: &Toggle, cx: &mut ViewContex }) .map(|(history_path, abs_path)| FoundPath::new(history_path, abs_path)), ) - .collect()); + .collect(); let project = workspace.project().clone(); let workspace = cx.handle().downgrade(); diff --git a/crates/workspace/src/pane.rs b/crates/workspace/src/pane.rs index e2f1767e92bbc7788d952e7f82743b650bb96c47..b8d08ee2d44ef14a6a3dadc83b99e581e1120bf2 100644 --- a/crates/workspace/src/pane.rs +++ b/crates/workspace/src/pane.rs @@ -492,7 +492,7 @@ impl Pane { if let Some((project_path, entry)) = to_load { // If the item was no longer present, then load it again from its previous path. - let task = workspace.load_path(project_path.clone(), cx); + let task = workspace.load_path(project_path, cx); cx.spawn(|workspace, mut cx| async move { let task = task.await; let mut navigated = false; @@ -510,7 +510,6 @@ impl Pane { workspace, pane.clone(), project_entry_id, - &project_path, true, cx, build_item, @@ -547,7 +546,6 @@ impl Pane { workspace: &mut Workspace, pane: ViewHandle, project_entry_id: ProjectEntryId, - project_path: &ProjectPath, focus_item: bool, cx: &mut ViewContext, build_item: impl FnOnce(&mut ViewContext) -> Box, @@ -580,15 +578,6 @@ impl Pane { None, cx, ); - { - let abs_path = workspace.absolute_path(project_path, cx); - pane.read(cx) - .nav_history - .borrow_mut() - .paths_by_item - .insert(new_item.id(), (project_path.clone(), abs_path)); - } - new_item } } @@ -602,6 +591,20 @@ impl Pane { destination_index: Option, cx: &mut ViewContext, ) { + if item.is_singleton(cx) { + if let Some(&entry_id) = item.project_entry_ids(cx).get(0) { + if let Some(project_path) = + workspace.project().read(cx).path_for_entry(entry_id, cx) + { + let abs_path = workspace.absolute_path(&project_path, cx); + pane.read(cx) + .nav_history + .borrow_mut() + .paths_by_item + .insert(item.id(), (project_path, abs_path)); + } + } + } // If no destination index is specified, add or move the item after the active item. let mut insertion_index = { let pane = pane.read(cx); @@ -1984,7 +1987,6 @@ impl PaneNavHistory { f(entry, project_and_abs_path.clone()); } else if let Some(item) = entry.item.upgrade(cx) { if let Some(path) = item.project_path(cx) { - // TODO kb ??? this should be the full path f(entry, (path, None)); } } diff --git a/crates/workspace/src/workspace.rs b/crates/workspace/src/workspace.rs index 817560fa6173258f61e3317dfcb85d72c28dc68c..1fe1c3ac933ab99a3f1de3f257eaacd2518fbf95 100644 --- a/crates/workspace/src/workspace.rs +++ b/crates/workspace/src/workspace.rs @@ -1306,14 +1306,19 @@ impl Workspace { } pub fn absolute_path(&self, project_path: &ProjectPath, cx: &AppContext) -> Option { - Some( - self.project() - .read(cx) - .worktree_for_id(project_path.worktree_id, cx)? - .read(cx) - .abs_path() - .to_path_buf(), - ) + let workspace_root = self + .project() + .read(cx) + .worktree_for_id(project_path.worktree_id, cx)? + .read(cx) + .abs_path(); + let project_path = project_path.path.as_ref(); + + Some(if project_path == Path::new("") { + workspace_root.to_path_buf() + } else { + workspace_root.join(project_path) + }) } fn add_folder_to_project(&mut self, _: &AddFolderToProject, cx: &mut ViewContext) { @@ -1661,23 +1666,14 @@ impl Workspace { }) }); - let project_path = path.into(); - let task = self.load_path(project_path.clone(), cx); + let task = self.load_path(path.into(), cx); cx.spawn(|this, mut cx| async move { let (project_entry_id, build_item) = task.await?; let pane = pane .upgrade(&cx) .ok_or_else(|| anyhow!("pane was closed"))?; this.update(&mut cx, |this, cx| { - Pane::open_item( - this, - pane, - project_entry_id, - &project_path, - focus_item, - cx, - build_item, - ) + Pane::open_item(this, pane, project_entry_id, focus_item, cx, build_item) }) }) } From 3a3c1c5a5bc1ffbb3761f441d9ef8bd47b62d0db Mon Sep 17 00:00:00 2001 From: Kirill Bulatov Date: Sat, 27 May 2023 00:06:09 +0300 Subject: [PATCH 6/7] Add a test co-authored-by: Mikayla --- crates/file_finder/src/file_finder.rs | 256 ++++++++++++++++++++++---- crates/project/src/project.rs | 2 +- crates/workspace/src/pane.rs | 8 +- crates/workspace/src/workspace.rs | 24 +-- 4 files changed, 235 insertions(+), 55 deletions(-) diff --git a/crates/file_finder/src/file_finder.rs b/crates/file_finder/src/file_finder.rs index 6617bd28883f3ff944e36e4e95e2d04437d6c078..d00704d6b9a3013b7122ff39fcc2439504f3a990 100644 --- a/crates/file_finder/src/file_finder.rs +++ b/crates/file_finder/src/file_finder.rs @@ -32,6 +32,7 @@ pub struct FileFinderDelegate { history_items: Vec, } +#[derive(Debug)] enum Matches { History(Vec), Search(Vec), @@ -114,6 +115,12 @@ fn toggle_file_finder(workspace: &mut Workspace, _: &Toggle, cx: &mut ViewContex .as_ref() .map(|found_path| &found_path.project) }) + .filter(|(_, history_abs_path)| { + history_abs_path.as_ref() + != currently_opened_path + .as_ref() + .and_then(|found_path| found_path.absolute.as_ref()) + }) .map(|(history_path, abs_path)| FoundPath::new(history_path, abs_path)), ) .collect(); @@ -330,7 +337,6 @@ impl FileFinderDelegate { ) -> (String, Vec, String, Vec) { let path = &path_match.path; let path_string = path.to_string_lossy(); - // TODO kb full path could be very long, trim it to fit the panel width let full_path = [path_match.path_prefix.as_ref(), path_string.as_ref()].join(""); let path_positions = path_match.positions.clone(); @@ -437,7 +443,7 @@ impl PickerDelegate for FileFinderDelegate { } else { match history_match.absolute.as_ref() { Some(abs_path) => { - workspace.open_abs_path(abs_path.to_path_buf(), true, cx) + workspace.open_abs_path(abs_path.to_path_buf(), false, cx) } None => workspace.open_path( ProjectPath { @@ -1192,10 +1198,13 @@ mod tests { .await; assert_eq!( history_after_first, - vec![dummy_found_path(ProjectPath { - worktree_id, - path: Arc::from(Path::new("test/first.rs")), - })], + vec![FoundPath::new( + ProjectPath { + worktree_id, + path: Arc::from(Path::new("test/first.rs")), + }, + Some(PathBuf::from("/src/test/first.rs")) + )], "Should show 1st opened item in the history when opening the 2nd item" ); @@ -1212,14 +1221,20 @@ mod tests { assert_eq!( history_after_second, vec![ - dummy_found_path(ProjectPath { - worktree_id, - path: Arc::from(Path::new("test/second.rs")), - }), - dummy_found_path(ProjectPath { - worktree_id, - path: Arc::from(Path::new("test/first.rs")), - }), + FoundPath::new( + ProjectPath { + worktree_id, + path: Arc::from(Path::new("test/second.rs")), + }, + Some(PathBuf::from("/src/test/second.rs")) + ), + FoundPath::new( + ProjectPath { + worktree_id, + path: Arc::from(Path::new("test/first.rs")), + }, + Some(PathBuf::from("/src/test/first.rs")) + ), ], "Should show 1st and 2nd opened items in the history when opening the 3rd item. \ 2nd item should be the first in the history, as the last opened." @@ -1238,18 +1253,27 @@ mod tests { assert_eq!( history_after_third, vec![ - dummy_found_path(ProjectPath { - worktree_id, - path: Arc::from(Path::new("test/third.rs")), - }), - dummy_found_path(ProjectPath { - worktree_id, - path: Arc::from(Path::new("test/second.rs")), - }), - dummy_found_path(ProjectPath { - worktree_id, - path: Arc::from(Path::new("test/first.rs")), - }), + FoundPath::new( + ProjectPath { + worktree_id, + path: Arc::from(Path::new("test/third.rs")), + }, + Some(PathBuf::from("/src/test/third.rs")) + ), + FoundPath::new( + ProjectPath { + worktree_id, + path: Arc::from(Path::new("test/second.rs")), + }, + Some(PathBuf::from("/src/test/second.rs")) + ), + FoundPath::new( + ProjectPath { + worktree_id, + path: Arc::from(Path::new("test/first.rs")), + }, + Some(PathBuf::from("/src/test/first.rs")) + ), ], "Should show 1st, 2nd and 3rd opened items in the history when opening the 2nd item again. \ 3rd item should be the first in the history, as the last opened." @@ -1268,24 +1292,162 @@ mod tests { assert_eq!( history_after_second_again, vec![ - dummy_found_path(ProjectPath { - worktree_id, - path: Arc::from(Path::new("test/second.rs")), - }), - dummy_found_path(ProjectPath { - worktree_id, - path: Arc::from(Path::new("test/third.rs")), - }), - dummy_found_path(ProjectPath { - worktree_id, - path: Arc::from(Path::new("test/first.rs")), - }), + FoundPath::new( + ProjectPath { + worktree_id, + path: Arc::from(Path::new("test/second.rs")), + }, + Some(PathBuf::from("/src/test/second.rs")) + ), + FoundPath::new( + ProjectPath { + worktree_id, + path: Arc::from(Path::new("test/third.rs")), + }, + Some(PathBuf::from("/src/test/third.rs")) + ), + FoundPath::new( + ProjectPath { + worktree_id, + path: Arc::from(Path::new("test/first.rs")), + }, + Some(PathBuf::from("/src/test/first.rs")) + ), ], "Should show 1st, 2nd and 3rd opened items in the history when opening the 3rd item again. \ 2nd item, as the last opened, 3rd item should go next as it was opened right before." ); } + #[gpui::test] + async fn test_external_files_history( + deterministic: Arc, + cx: &mut gpui::TestAppContext, + ) { + let app_state = init_test(cx); + + app_state + .fs + .as_fake() + .insert_tree( + "/src", + json!({ + "test": { + "first.rs": "// First Rust file", + "second.rs": "// Second Rust file", + } + }), + ) + .await; + + app_state + .fs + .as_fake() + .insert_tree( + "/external-src", + json!({ + "test": { + "third.rs": "// Third Rust file", + "fourth.rs": "// Fourth Rust file", + } + }), + ) + .await; + + let project = Project::test(app_state.fs.clone(), ["/src".as_ref()], cx).await; + cx.update(|cx| { + project.update(cx, |project, cx| { + project.find_or_create_local_worktree("/external-src", false, cx) + }) + }) + .detach(); + deterministic.run_until_parked(); + + let (window_id, workspace) = cx.add_window(|cx| Workspace::test_new(project, cx)); + let worktree_id = cx.read(|cx| { + let worktrees = workspace.read(cx).worktrees(cx).collect::>(); + assert_eq!(worktrees.len(), 1,); + + WorktreeId::from_usize(worktrees[0].id()) + }); + workspace + .update(cx, |workspace, cx| { + workspace.open_abs_path(PathBuf::from("/external-src/test/third.rs"), false, cx) + }) + .detach(); + deterministic.run_until_parked(); + let external_worktree_id = cx.read(|cx| { + let worktrees = workspace.read(cx).worktrees(cx).collect::>(); + assert_eq!( + worktrees.len(), + 2, + "External file should get opened in a new worktree" + ); + + WorktreeId::from_usize( + worktrees + .into_iter() + .find(|worktree| worktree.id() != worktree_id.to_usize()) + .expect("New worktree should have a different id") + .id(), + ) + }); + close_active_item(&workspace, &deterministic, cx).await; + + let initial_history_items = open_close_queried_buffer( + "sec", + 1, + "second.rs", + window_id, + &workspace, + &deterministic, + cx, + ) + .await; + assert_eq!( + initial_history_items, + vec![FoundPath::new( + ProjectPath { + worktree_id: external_worktree_id, + path: Arc::from(Path::new("")), + }, + Some(PathBuf::from("/external-src/test/third.rs")) + )], + "Should show external file with its full path in the history after it was open" + ); + + let updated_history_items = open_close_queried_buffer( + "fir", + 1, + "first.rs", + window_id, + &workspace, + &deterministic, + cx, + ) + .await; + assert_eq!( + updated_history_items, + vec![ + FoundPath::new( + ProjectPath { + worktree_id, + path: Arc::from(Path::new("test/second.rs")), + }, + Some(PathBuf::from("/src/test/second.rs")) + ), + FoundPath::new( + ProjectPath { + worktree_id: external_worktree_id, + path: Arc::from(Path::new("")), + }, + Some(PathBuf::from("/external-src/test/third.rs")) + ), + ], + "Should keep external file with history updates", + ); + } + async fn open_close_queried_buffer( input: &str, expected_matches: usize, @@ -1332,6 +1494,16 @@ mod tests { ); }); + close_active_item(workspace, deterministic, cx).await; + + history_items + } + + async fn close_active_item( + workspace: &ViewHandle, + deterministic: &gpui::executor::Deterministic, + cx: &mut TestAppContext, + ) { let mut original_items = HashMap::new(); cx.read(|cx| { for pane in workspace.read(cx).panes() { @@ -1341,6 +1513,8 @@ mod tests { assert!(insertion_result.is_none(), "Pane id {pane_id} collision"); } }); + + let active_pane = cx.read(|cx| workspace.read(cx).active_pane().clone()); active_pane .update(cx, |pane, cx| { pane.close_active_item(&workspace::CloseActiveItem, cx) @@ -1365,8 +1539,10 @@ mod tests { } } }); - - history_items + assert!( + original_items.len() <= 1, + "At most one panel should got closed" + ); } fn init_test(cx: &mut TestAppContext) -> Arc { diff --git a/crates/project/src/project.rs b/crates/project/src/project.rs index 6a6f87897860ebe09b81326553c1bf779012cc88..c5442221eb295866a439e549d4c343d3ad81f85d 100644 --- a/crates/project/src/project.rs +++ b/crates/project/src/project.rs @@ -3218,7 +3218,7 @@ impl Project { ) -> Result<(), anyhow::Error> { let (worktree, relative_path) = self .find_local_worktree(&abs_path, cx) - .ok_or_else(|| anyhow!("no worktree found for diagnostics"))?; + .ok_or_else(|| anyhow!("no worktree found for diagnostics path {abs_path:?}"))?; let project_path = ProjectPath { worktree_id: worktree.read(cx).id(), diff --git a/crates/workspace/src/pane.rs b/crates/workspace/src/pane.rs index b8d08ee2d44ef14a6a3dadc83b99e581e1120bf2..cf42013e8dadf95407de1e9291c241ef448a9b89 100644 --- a/crates/workspace/src/pane.rs +++ b/crates/workspace/src/pane.rs @@ -1018,9 +1018,11 @@ impl Pane { if let Some(path) = item.project_path(cx) { let abs_path = self - .workspace() - .upgrade(cx) - .and_then(|workspace| workspace.read(cx).absolute_path(&path, cx)); + .nav_history + .borrow() + .paths_by_item + .get(&item.id()) + .and_then(|(_, abs_path)| abs_path.clone()); self.nav_history .borrow_mut() .paths_by_item diff --git a/crates/workspace/src/workspace.rs b/crates/workspace/src/workspace.rs index 1fe1c3ac933ab99a3f1de3f257eaacd2518fbf95..38e1dbc9ac62795696e9bfac62e490886d74dd6b 100644 --- a/crates/workspace/src/workspace.rs +++ b/crates/workspace/src/workspace.rs @@ -975,23 +975,25 @@ impl Workspace { }); } - let project = self.project.read(cx); history .into_iter() .sorted_by_key(|(_, (_, timestamp))| *timestamp) .map(|(project_path, (fs_path, _))| (project_path, fs_path)) .rev() .filter(|(history_path, abs_path)| { - project - .worktree_for_id(history_path.worktree_id, cx) - .is_some() - || abs_path - .as_ref() - .and_then(|abs_path| { - let buffers_opened = abs_paths_opened.get(abs_path)?; - Some(buffers_opened.len() < 2) - }) - .unwrap_or(false) + let latest_project_path_opened = abs_path + .as_ref() + .and_then(|abs_path| abs_paths_opened.get(abs_path)) + .and_then(|project_paths| { + project_paths + .iter() + .max_by(|b1, b2| b1.worktree_id.cmp(&b2.worktree_id)) + }); + + match latest_project_path_opened { + Some(latest_project_path_opened) => latest_project_path_opened == history_path, + None => true, + } }) .take(limit.unwrap_or(usize::MAX)) .collect() From 5fda9e934d556b876a8de20702f474cf36f75b0a Mon Sep 17 00:00:00 2001 From: Kirill Bulatov Date: Sat, 27 May 2023 01:21:38 +0300 Subject: [PATCH 7/7] Shorten full paths with ~ --- crates/file_finder/src/file_finder.rs | 25 ++++++++++++++++++++++++- 1 file changed, 24 insertions(+), 1 deletion(-) diff --git a/crates/file_finder/src/file_finder.rs b/crates/file_finder/src/file_finder.rs index d00704d6b9a3013b7122ff39fcc2439504f3a990..73e7ca6eaa1cf56f98115a6607632f10da422fd4 100644 --- a/crates/file_finder/src/file_finder.rs +++ b/crates/file_finder/src/file_finder.rs @@ -285,7 +285,7 @@ impl FileFinderDelegate { cx: &AppContext, ix: usize, ) -> (String, Vec, String, Vec) { - match path_match { + let (file_name, file_name_positions, full_path, full_path_positions) = match path_match { Match::History(found_path) => { let worktree_id = found_path.project.worktree_id; let project_relative_path = &found_path.project.path; @@ -328,7 +328,30 @@ impl FileFinderDelegate { }) } Match::Search(path_match) => self.labels_for_path_match(path_match), + }; + + if file_name_positions.is_empty() { + if let Some(user_home_path) = std::env::var("HOME").ok() { + let user_home_path = user_home_path.trim(); + if !user_home_path.is_empty() { + if (&full_path).starts_with(user_home_path) { + return ( + file_name, + file_name_positions, + full_path.replace(user_home_path, "~"), + full_path_positions, + ); + } + } + } } + + ( + file_name, + file_name_positions, + full_path, + full_path_positions, + ) } fn labels_for_path_match(