Make fallback open picker more intuitive (#37564)

Kirill Bulatov , Peter Tripp , and David Kleingeld created 3 months ago

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

Before, the picker did not allow to open the current directory that was
just completed:

<img width="553" height="354" alt="image"
src="https://github.com/user-attachments/assets/e77793c8-763e-416f-9728-18d5a39b467f"
/>

pressing `enter` here would open `assets`; pressing `tab` would append
the `assets/` segment to the query.
Only backspace, removing `/` would allow to open the current directory.

After:
<img width="574" height="349" alt="image"
src="https://github.com/user-attachments/assets/bdbb3e23-7c7a-4e12-8092-51a6a0ea9f87"
/>

The first item is now a placeholder for opening the current directory
with `enter`.
Any time a fuzzy query is appended, the placeholder goes away; `tab`
selects the entry below the placeholder.

Release Notes:

- Made fallback open picker more intuitive

---------

Co-authored-by: Peter Tripp <petertripp@gmail.com>
Co-authored-by: David Kleingeld <davidsk@zed.dev>

Change summary

crates/file_finder/src/open_path_prompt.rs       |  66 +++++++++
crates/file_finder/src/open_path_prompt_tests.rs | 118 ++++++++++++-----
2 files changed, 146 insertions(+), 38 deletions(-)

Detailed changes

crates/file_finder/src/open_path_prompt.rs 🔗

@@ -1,7 +1,7 @@
 use crate::file_finder_settings::FileFinderSettings;
 use file_icons::FileIcons;
 use futures::channel::oneshot;
-use fuzzy::{StringMatch, StringMatchCandidate};
+use fuzzy::{CharBag, StringMatch, StringMatchCandidate};
 use gpui::{HighlightStyle, StyledText, Task};
 use picker::{Picker, PickerDelegate};
 use project::{DirectoryItem, DirectoryLister};
@@ -125,6 +125,13 @@ impl OpenPathDelegate {
             DirectoryState::None { .. } => Vec::new(),
         }
     }
+
+    fn current_dir(&self) -> &'static str {
+        match self.path_style {
+            PathStyle::Posix => "./",
+            PathStyle::Windows => ".\\",
+        }
+    }
 }
 
 #[derive(Debug)]
@@ -233,6 +240,7 @@ impl PickerDelegate for OpenPathDelegate {
         cx: &mut Context<Picker<Self>>,
     ) -> Task<()> {
         let lister = &self.lister;
+        let input_is_empty = query.is_empty();
         let (dir, suffix) = get_dir_and_suffix(query, self.path_style);
 
         let query = match &self.directory_state {
@@ -263,6 +271,7 @@ impl PickerDelegate for OpenPathDelegate {
         let cancel_flag = self.cancel_flag.clone();
 
         let parent_path_is_root = self.prompt_root == dir;
+        let current_dir = self.current_dir();
         cx.spawn_in(window, async move |this, cx| {
             if let Some(query) = query {
                 let paths = query.await;
@@ -353,10 +362,38 @@ impl PickerDelegate for OpenPathDelegate {
                 return;
             };
 
+            let mut max_id = 0;
             if !suffix.starts_with('.') {
-                new_entries.retain(|entry| !entry.path.string.starts_with('.'));
+                new_entries.retain(|entry| {
+                    max_id = max_id.max(entry.path.id);
+                    !entry.path.string.starts_with('.')
+                });
             }
+
             if suffix.is_empty() {
+                let should_prepend_with_current_dir = this
+                    .read_with(cx, |picker, _| {
+                        !input_is_empty
+                            && !matches!(
+                                picker.delegate.directory_state,
+                                DirectoryState::Create { .. }
+                            )
+                    })
+                    .unwrap_or(false);
+                if should_prepend_with_current_dir {
+                    new_entries.insert(
+                        0,
+                        CandidateInfo {
+                            path: StringMatchCandidate {
+                                id: max_id + 1,
+                                string: current_dir.to_string(),
+                                char_bag: CharBag::from(current_dir),
+                            },
+                            is_dir: true,
+                        },
+                    );
+                }
+
                 this.update(cx, |this, cx| {
                     this.delegate.selected_index = 0;
                     this.delegate.string_matches = new_entries
@@ -485,6 +522,10 @@ impl PickerDelegate for OpenPathDelegate {
         _: &mut Context<Picker<Self>>,
     ) -> Option<String> {
         let candidate = self.get_entry(self.selected_index)?;
+        if candidate.path.string.is_empty() || candidate.path.string == self.current_dir() {
+            return None;
+        }
+
         let path_style = self.path_style;
         Some(
             maybe!({
@@ -629,12 +670,18 @@ impl PickerDelegate for OpenPathDelegate {
             DirectoryState::None { .. } => Vec::new(),
         };
 
+        let is_current_dir_candidate = candidate.path.string == self.current_dir();
+
         let file_icon = maybe!({
             if !settings.file_icons {
                 return None;
             }
             let icon = if candidate.is_dir {
-                FileIcons::get_folder_icon(false, cx)?
+                if is_current_dir_candidate {
+                    return Some(Icon::new(IconName::ReplyArrowRight).color(Color::Muted));
+                } else {
+                    FileIcons::get_folder_icon(false, cx)?
+                }
             } else {
                 let path = path::Path::new(&candidate.path.string);
                 FileIcons::get_icon(path, cx)?
@@ -652,6 +699,8 @@ impl PickerDelegate for OpenPathDelegate {
                     .child(HighlightedLabel::new(
                         if parent_path == &self.prompt_root {
                             format!("{}{}", self.prompt_root, candidate.path.string)
+                        } else if is_current_dir_candidate {
+                            "open this directory".to_string()
                         } else {
                             candidate.path.string
                         },
@@ -747,6 +796,17 @@ impl PickerDelegate for OpenPathDelegate {
     fn placeholder_text(&self, _window: &mut Window, _cx: &mut App) -> Arc<str> {
         Arc::from(format!("[directory{MAIN_SEPARATOR_STR}]filename.ext"))
     }
+
+    fn separators_after_indices(&self) -> Vec<usize> {
+        let Some(m) = self.string_matches.first() else {
+            return Vec::new();
+        };
+        if m.string == self.current_dir() {
+            vec![0]
+        } else {
+            Vec::new()
+        }
+    }
 }
 
 fn path_candidates(

crates/file_finder/src/open_path_prompt_tests.rs 🔗

@@ -43,12 +43,17 @@ async fn test_open_path_prompt(cx: &mut TestAppContext) {
     insert_query(query, &picker, cx).await;
     assert_eq!(collect_match_candidates(&picker, cx), vec!["root"]);
 
+    #[cfg(not(windows))]
+    let expected_separator = "./";
+    #[cfg(windows)]
+    let expected_separator = ".\\";
+
     // If the query ends with a slash, the picker should show the contents of the directory.
     let query = path!("/root/");
     insert_query(query, &picker, cx).await;
     assert_eq!(
         collect_match_candidates(&picker, cx),
-        vec!["a1", "a2", "a3", "dir1", "dir2"]
+        vec![expected_separator, "a1", "a2", "a3", "dir1", "dir2"]
     );
 
     // Show candidates for the query "a".
@@ -72,7 +77,7 @@ async fn test_open_path_prompt(cx: &mut TestAppContext) {
     insert_query(query, &picker, cx).await;
     assert_eq!(
         collect_match_candidates(&picker, cx),
-        vec!["c", "d1", "d2", "d3", "dir3", "dir4"]
+        vec![expected_separator, "c", "d1", "d2", "d3", "dir3", "dir4"]
     );
 
     // Show candidates for the query "d".
@@ -116,71 +121,86 @@ async fn test_open_path_prompt_completion(cx: &mut TestAppContext) {
     // Confirm completion for the query "/root", since it's a directory, it should add a trailing slash.
     let query = path!("/root");
     insert_query(query, &picker, cx).await;
-    assert_eq!(confirm_completion(query, 0, &picker, cx), path!("/root/"));
+    assert_eq!(
+        confirm_completion(query, 0, &picker, cx).unwrap(),
+        path!("/root/")
+    );
 
     // Confirm completion for the query "/root/", selecting the first candidate "a", since it's a file, it should not add a trailing slash.
     let query = path!("/root/");
     insert_query(query, &picker, cx).await;
-    assert_eq!(confirm_completion(query, 0, &picker, cx), path!("/root/a"));
+    assert_eq!(
+        confirm_completion(query, 0, &picker, cx),
+        None,
+        "First entry is `./` and when we confirm completion, it is tabbed below"
+    );
+    assert_eq!(
+        confirm_completion(query, 1, &picker, cx).unwrap(),
+        path!("/root/a"),
+        "Second entry is the first entry of a directory that we want to be completed"
+    );
 
     // Confirm completion for the query "/root/", selecting the second candidate "dir1", since it's a directory, it should add a trailing slash.
     let query = path!("/root/");
     insert_query(query, &picker, cx).await;
     assert_eq!(
-        confirm_completion(query, 1, &picker, cx),
+        confirm_completion(query, 2, &picker, cx).unwrap(),
         path!("/root/dir1/")
     );
 
     let query = path!("/root/a");
     insert_query(query, &picker, cx).await;
-    assert_eq!(confirm_completion(query, 0, &picker, cx), path!("/root/a"));
+    assert_eq!(
+        confirm_completion(query, 0, &picker, cx).unwrap(),
+        path!("/root/a")
+    );
 
     let query = path!("/root/d");
     insert_query(query, &picker, cx).await;
     assert_eq!(
-        confirm_completion(query, 1, &picker, cx),
+        confirm_completion(query, 1, &picker, cx).unwrap(),
         path!("/root/dir2/")
     );
 
     let query = path!("/root/dir2");
     insert_query(query, &picker, cx).await;
     assert_eq!(
-        confirm_completion(query, 0, &picker, cx),
+        confirm_completion(query, 0, &picker, cx).unwrap(),
         path!("/root/dir2/")
     );
 
     let query = path!("/root/dir2/");
     insert_query(query, &picker, cx).await;
     assert_eq!(
-        confirm_completion(query, 0, &picker, cx),
+        confirm_completion(query, 1, &picker, cx).unwrap(),
         path!("/root/dir2/c")
     );
 
     let query = path!("/root/dir2/");
     insert_query(query, &picker, cx).await;
     assert_eq!(
-        confirm_completion(query, 2, &picker, cx),
+        confirm_completion(query, 3, &picker, cx).unwrap(),
         path!("/root/dir2/dir3/")
     );
 
     let query = path!("/root/dir2/d");
     insert_query(query, &picker, cx).await;
     assert_eq!(
-        confirm_completion(query, 0, &picker, cx),
+        confirm_completion(query, 0, &picker, cx).unwrap(),
         path!("/root/dir2/d")
     );
 
     let query = path!("/root/dir2/d");
     insert_query(query, &picker, cx).await;
     assert_eq!(
-        confirm_completion(query, 1, &picker, cx),
+        confirm_completion(query, 1, &picker, cx).unwrap(),
         path!("/root/dir2/dir3/")
     );
 
     let query = path!("/root/dir2/di");
     insert_query(query, &picker, cx).await;
     assert_eq!(
-        confirm_completion(query, 1, &picker, cx),
+        confirm_completion(query, 1, &picker, cx).unwrap(),
         path!("/root/dir2/dir4/")
     );
 }
@@ -211,42 +231,63 @@ async fn test_open_path_prompt_on_windows(cx: &mut TestAppContext) {
     insert_query(query, &picker, cx).await;
     assert_eq!(
         collect_match_candidates(&picker, cx),
-        vec!["a", "dir1", "dir2"]
+        vec![".\\", "a", "dir1", "dir2"]
+    );
+    assert_eq!(
+        confirm_completion(query, 0, &picker, cx),
+        None,
+        "First entry is `.\\` and when we confirm completion, it is tabbed below"
+    );
+    assert_eq!(
+        confirm_completion(query, 1, &picker, cx).unwrap(),
+        "C:/root/a",
+        "Second entry is the first entry of a directory that we want to be completed"
     );
-    assert_eq!(confirm_completion(query, 0, &picker, cx), "C:/root/a");
 
     let query = "C:\\root/";
     insert_query(query, &picker, cx).await;
     assert_eq!(
         collect_match_candidates(&picker, cx),
-        vec!["a", "dir1", "dir2"]
+        vec![".\\", "a", "dir1", "dir2"]
+    );
+    assert_eq!(
+        confirm_completion(query, 1, &picker, cx).unwrap(),
+        "C:\\root/a"
     );
-    assert_eq!(confirm_completion(query, 0, &picker, cx), "C:\\root/a");
 
     let query = "C:\\root\\";
     insert_query(query, &picker, cx).await;
     assert_eq!(
         collect_match_candidates(&picker, cx),
-        vec!["a", "dir1", "dir2"]
+        vec![".\\", "a", "dir1", "dir2"]
+    );
+    assert_eq!(
+        confirm_completion(query, 1, &picker, cx).unwrap(),
+        "C:\\root\\a"
     );
-    assert_eq!(confirm_completion(query, 0, &picker, cx), "C:\\root\\a");
 
     // Confirm completion for the query "C:/root/d", selecting the second candidate "dir2", since it's a directory, it should add a trailing slash.
     let query = "C:/root/d";
     insert_query(query, &picker, cx).await;
     assert_eq!(collect_match_candidates(&picker, cx), vec!["dir1", "dir2"]);
-    assert_eq!(confirm_completion(query, 1, &picker, cx), "C:/root/dir2\\");
+    assert_eq!(
+        confirm_completion(query, 1, &picker, cx).unwrap(),
+        "C:/root/dir2\\"
+    );
 
     let query = "C:\\root/d";
     insert_query(query, &picker, cx).await;
     assert_eq!(collect_match_candidates(&picker, cx), vec!["dir1", "dir2"]);
-    assert_eq!(confirm_completion(query, 0, &picker, cx), "C:\\root/dir1\\");
+    assert_eq!(
+        confirm_completion(query, 0, &picker, cx).unwrap(),
+        "C:\\root/dir1\\"
+    );
 
     let query = "C:\\root\\d";
     insert_query(query, &picker, cx).await;
     assert_eq!(collect_match_candidates(&picker, cx), vec!["dir1", "dir2"]);
     assert_eq!(
-        confirm_completion(query, 0, &picker, cx),
+        confirm_completion(query, 0, &picker, cx).unwrap(),
         "C:\\root\\dir1\\"
     );
 }
@@ -276,20 +317,29 @@ async fn test_open_path_prompt_on_windows_with_remote(cx: &mut TestAppContext) {
     insert_query(query, &picker, cx).await;
     assert_eq!(
         collect_match_candidates(&picker, cx),
-        vec!["a", "dir1", "dir2"]
+        vec!["./", "a", "dir1", "dir2"]
+    );
+    assert_eq!(
+        confirm_completion(query, 1, &picker, cx).unwrap(),
+        "/root/a"
     );
-    assert_eq!(confirm_completion(query, 0, &picker, cx), "/root/a");
 
     // Confirm completion for the query "/root/d", selecting the second candidate "dir2", since it's a directory, it should add a trailing slash.
     let query = "/root/d";
     insert_query(query, &picker, cx).await;
     assert_eq!(collect_match_candidates(&picker, cx), vec!["dir1", "dir2"]);
-    assert_eq!(confirm_completion(query, 1, &picker, cx), "/root/dir2/");
+    assert_eq!(
+        confirm_completion(query, 1, &picker, cx).unwrap(),
+        "/root/dir2/"
+    );
 
     let query = "/root/d";
     insert_query(query, &picker, cx).await;
     assert_eq!(collect_match_candidates(&picker, cx), vec!["dir1", "dir2"]);
-    assert_eq!(confirm_completion(query, 0, &picker, cx), "/root/dir1/");
+    assert_eq!(
+        confirm_completion(query, 0, &picker, cx).unwrap(),
+        "/root/dir1/"
+    );
 }
 
 #[gpui::test]
@@ -396,15 +446,13 @@ fn confirm_completion(
     select: usize,
     picker: &Entity<Picker<OpenPathDelegate>>,
     cx: &mut VisualTestContext,
-) -> String {
-    picker
-        .update_in(cx, |f, window, cx| {
-            if f.delegate.selected_index() != select {
-                f.delegate.set_selected_index(select, window, cx);
-            }
-            f.delegate.confirm_completion(query.to_string(), window, cx)
-        })
-        .unwrap()
+) -> Option<String> {
+    picker.update_in(cx, |f, window, cx| {
+        if f.delegate.selected_index() != select {
+            f.delegate.set_selected_index(select, window, cx);
+        }
+        f.delegate.confirm_completion(query.to_string(), window, cx)
+    })
 }
 
 fn collect_match_candidates(