file_finder: Prioritize file name matches over path matches (#27937)

Smit Barmase created 8 months ago

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

Adds additional comparison after the existing history-based comparison.

This comparison checks whether the first position matched via fuzzy
search is in the file name. If so, we prioritize this match over one in
the path. If both items match either the file name or the path, the
existing comparison logic is used.

- [x] Tests

Before:
<img width="580" alt="image"
src="https://github.com/user-attachments/assets/011fdd01-3dfa-4950-abb1-dfda10885664"
/>

After:
<img width="614" alt="image"
src="https://github.com/user-attachments/assets/9c4944b0-83dc-4611-94bb-aae1758fab23"
/>


Release Notes:

- Fixed an issue where fuzzy matching in file finder did not properly
prioritize matches in file names.

Change summary

crates/file_finder/src/file_finder.rs       | 61 +++++++++++++++++++++-
crates/file_finder/src/file_finder_tests.rs | 45 ++++++++++++++++
2 files changed, 101 insertions(+), 5 deletions(-)

Detailed changes

crates/file_finder/src/file_finder.rs

@@ -530,17 +530,68 @@ impl Matches {
         match (&a, &b) {
             // bubble currently opened files to the top
             (Match::History { path, .. }, _) if Some(path) == currently_opened => {
-                cmp::Ordering::Greater
+                return cmp::Ordering::Greater;
             }
             (_, Match::History { path, .. }) if Some(path) == currently_opened => {
-                cmp::Ordering::Less
+                return cmp::Ordering::Less;
             }
 
-            (Match::History { .. }, Match::Search(_)) if separate_history => cmp::Ordering::Greater,
-            (Match::Search(_), Match::History { .. }) if separate_history => cmp::Ordering::Less,
+            _ => {}
+        }
+
+        if separate_history {
+            match (a, b) {
+                (Match::History { .. }, Match::Search(_)) => return cmp::Ordering::Greater,
+                (Match::Search(_), Match::History { .. }) => return cmp::Ordering::Less,
 
-            _ => a.panel_match().cmp(&b.panel_match()),
+                _ => {}
+            }
         }
+
+        let a_panel_match = match a.panel_match() {
+            Some(pm) => pm,
+            None => {
+                return if b.panel_match().is_some() {
+                    cmp::Ordering::Less
+                } else {
+                    cmp::Ordering::Equal
+                };
+            }
+        };
+
+        let b_panel_match = match b.panel_match() {
+            Some(pm) => pm,
+            None => return cmp::Ordering::Greater,
+        };
+
+        let a_in_filename = Self::is_filename_match(a_panel_match);
+        let b_in_filename = Self::is_filename_match(b_panel_match);
+
+        match (a_in_filename, b_in_filename) {
+            (true, false) => return cmp::Ordering::Greater,
+            (false, true) => return cmp::Ordering::Less,
+            _ => {} // Both are filename matches or both are path matches
+        }
+
+        a_panel_match.cmp(b_panel_match)
+    }
+
+    /// Determines if the match occurred within the filename rather than in the path
+    fn is_filename_match(panel_match: &ProjectPanelOrdMatch) -> bool {
+        if panel_match.0.positions.is_empty() {
+            return false;
+        }
+
+        if let Some(filename) = panel_match.0.path.file_name() {
+            let path_str = panel_match.0.path.to_string_lossy();
+            let filename_str = filename.to_string_lossy();
+
+            if let Some(filename_pos) = path_str.rfind(&*filename_str) {
+                return panel_match.0.positions[0] >= filename_pos;
+            }
+        }
+
+        false
     }
 }
 

crates/file_finder/src/file_finder_tests.rs

@@ -2372,3 +2372,48 @@ fn assert_match_at_position(
     .to_string_lossy();
     assert_eq!(match_file_name, expected_file_name);
 }
+
+#[gpui::test]
+async fn test_filename_precedence(cx: &mut TestAppContext) {
+    let app_state = init_test(cx);
+
+    app_state
+        .fs
+        .as_fake()
+        .insert_tree(
+            path!("/src"),
+            json!({
+                "layout": {
+                    "app.css": "",
+                    "app.d.ts": "",
+                    "app.html": "",
+                    "+page.svelte": "",
+                },
+                "routes": {
+                    "+layout.svelte": "",
+                }
+            }),
+        )
+        .await;
+
+    let project = Project::test(app_state.fs.clone(), [path!("/src").as_ref()], cx).await;
+    let (picker, _, cx) = build_find_picker(project, cx);
+
+    cx.simulate_input("layout");
+
+    picker.update(cx, |finder, _| {
+        let search_matches = collect_search_matches(finder).search_paths_only();
+
+        assert_eq!(
+            search_matches,
+            vec![
+                PathBuf::from("routes/+layout.svelte"),
+                PathBuf::from("layout/app.css"),
+                PathBuf::from("layout/app.d.ts"),
+                PathBuf::from("layout/app.html"),
+                PathBuf::from("layout/+page.svelte"),
+            ],
+            "File with 'layout' in filename should be prioritized over files in 'layout' directory"
+        );
+    });
+}