Order history items by open recency (#7248)

Kirill Bulatov and Andrew Lygin created 2 years ago

Closes https://github.com/zed-industries/zed/issues/7244

Follow-up of https://github.com/zed-industries/zed/pull/7210 that
returns back ordering of history items by open recency (last opened
history item should be on top)

Release Notes:

- N/A

---------

Co-authored-by: Andrew Lygin <[e-mail]>

Change summary

crates/file_finder/src/file_finder.rs       | 39 ++++++---------
crates/file_finder/src/file_finder_tests.rs | 56 +++++++++++++++++++++++
2 files changed, 72 insertions(+), 23 deletions(-)

Detailed changes

crates/file_finder/src/file_finder.rs

@@ -1,7 +1,7 @@
 #[cfg(test)]
 mod file_finder_tests;
 
-use collections::HashMap;
+use collections::{HashMap, HashSet};
 use editor::{scroll::Autoscroll, Bias, Editor};
 use fuzzy::{CharBag, PathMatch, PathMatchCandidate};
 use gpui::{
@@ -222,9 +222,11 @@ impl Matches {
         query_matches: Option<&'a HashMap<Arc<Path>, ProjectPanelOrdMatch>>,
         history_items: impl IntoIterator<Item = &'a FoundPath> + 'a,
     ) {
-        let history_items_to_show = history_items
+        let mut processed_paths = HashSet::default();
+        self.history = history_items
             .into_iter()
             .chain(currently_opened)
+            .filter(|&path| processed_paths.insert(path))
             .filter_map(|history_item| match &query_matches {
                 Some(query_matches) => Some((
                     history_item.clone(),
@@ -232,31 +234,22 @@ impl Matches {
                 )),
                 None => Some((history_item.clone(), None)),
             })
-            .sorted_by(|(path_a, match_a), (path_b, match_b)| {
-                match (
+            .enumerate()
+            .sorted_by(
+                |(index_a, (path_a, match_a)), (index_b, (path_b, match_b))| match (
                     Some(path_a) == currently_opened,
                     Some(path_b) == currently_opened,
                 ) {
+                    // bubble currently opened files to the top
                     (true, false) => cmp::Ordering::Less,
                     (false, true) => cmp::Ordering::Greater,
-                    _ => match_b.cmp(match_a),
-                }
-            });
-
-        self.history.clear();
-        util::extend_sorted(
-            &mut self.history,
-            history_items_to_show.collect::<Vec<_>>(),
-            100,
-            |(path_a, match_a), (path_b, match_b)| match (
-                Some(path_a) == currently_opened,
-                Some(path_b) == currently_opened,
-            ) {
-                (true, false) => cmp::Ordering::Less,
-                (false, true) => cmp::Ordering::Greater,
-                _ => match_b.cmp(match_a).then(path_b.cmp(path_a)),
-            },
-        );
+                    // arrange the files by their score (best score on top) and by their occurrence in the history
+                    // (history items visited later are on the top)
+                    _ => match_b.cmp(match_a).then(index_a.cmp(index_b)),
+                },
+            )
+            .map(|(_, paths)| paths)
+            .collect();
     }
 }
 
@@ -319,7 +312,7 @@ fn matching_history_item_paths(
     matching_history_paths
 }
 
-#[derive(Debug, Clone, PartialEq, Eq, PartialOrd, Ord)]
+#[derive(Debug, Clone, PartialEq, Eq, PartialOrd, Ord, Hash)]
 struct FoundPath {
     project: ProjectPath,
     absolute: Option<PathBuf>,

crates/file_finder/src/file_finder_tests.rs

@@ -1177,6 +1177,62 @@ async fn test_keep_opened_file_on_top_of_search_results_and_select_next_one(
     });
 }
 
+#[gpui::test]
+async fn test_history_items_shown_in_order_of_open(cx: &mut TestAppContext) {
+    let app_state = init_test(cx);
+
+    app_state
+        .fs
+        .as_fake()
+        .insert_tree(
+            "/test",
+            json!({
+                "test": {
+                    "1.txt": "// One",
+                    "2.txt": "// Two",
+                    "3.txt": "// Three",
+                }
+            }),
+        )
+        .await;
+
+    let project = Project::test(app_state.fs.clone(), ["/test".as_ref()], cx).await;
+    let (workspace, cx) = cx.add_window_view(|cx| Workspace::test_new(project, cx));
+
+    open_queried_buffer("1", 1, "1.txt", &workspace, cx).await;
+    open_queried_buffer("2", 1, "2.txt", &workspace, cx).await;
+    open_queried_buffer("3", 1, "3.txt", &workspace, cx).await;
+
+    let picker = open_file_picker(&workspace, cx);
+    picker.update(cx, |finder, _| {
+        assert_eq!(finder.delegate.matches.len(), 3);
+        assert_match_at_position(finder, 0, "3.txt");
+        assert_match_selection(finder, 1, "2.txt");
+        assert_match_at_position(finder, 2, "1.txt");
+    });
+
+    cx.dispatch_action(Confirm); // Open 2.txt
+
+    let picker = open_file_picker(&workspace, cx);
+    picker.update(cx, |finder, _| {
+        assert_eq!(finder.delegate.matches.len(), 3);
+        assert_match_at_position(finder, 0, "2.txt");
+        assert_match_selection(finder, 1, "3.txt");
+        assert_match_at_position(finder, 2, "1.txt");
+    });
+
+    cx.dispatch_action(SelectNext);
+    cx.dispatch_action(Confirm); // Open 1.txt
+
+    let picker = open_file_picker(&workspace, cx);
+    picker.update(cx, |finder, _| {
+        assert_eq!(finder.delegate.matches.len(), 3);
+        assert_match_at_position(finder, 0, "1.txt");
+        assert_match_selection(finder, 1, "2.txt");
+        assert_match_at_position(finder, 2, "3.txt");
+    });
+}
+
 #[gpui::test]
 async fn test_history_items_vs_very_good_external_match(cx: &mut gpui::TestAppContext) {
     let app_state = init_test(cx);