Danilo Leal
f88bc7e18a
The recent "pickers with previews" overhaul left the picker
sizing/presentation API spread across several overlapping knobs that
every call site had to set correctly — and many didn't, causing pickers
across the app to render at the wrong width, lose their container, or
stop dismissing. This PR consolidates that surface into a small,
hard-to-misuse API and makes correct sizing the default.
Net effect: a plain `Picker::uniform_list(delegate, …)` now renders
correctly out of the box (standard width, standard max-height, shrinks
to fit, dismisses properly), and the ~35 call sites only specify what
genuinely differs.
## API changes
**Presentation** — three overlapping booleans (`is_modal`, `is_popover`,
`is_resizable`) collapsed into one enum, with resizability living inside
the only variant where it's meaningful:
```rust
enum Presentation {
Modal { resizable: bool }, // own chrome, dismisses on blur, optionally resizable
Popover, // own chrome, dismisses on blur, never resizable
Embedded, // host container owns chrome + dismissal
}
```
- `modal(bool)` is **removed** in favor of explicit, self-documenting
builders:
- *(default)* → `Modal` (resizable iff it has a preview)
- `.popover()` → `Popover` (menu-attached surfaces)
- `.embedded()` → `Embedded` (pickers nested in a larger modal/view)
- Dynamic callers use `.when(cond, Picker::embedded)` (added `impl
FluentBuilder for Picker`).
**Sizing** — preview-vs-not now drives everything; the manual padding
knob is gone:
| Before | After |
|---|---|
| `vertical_padding` field + `no_vertical_padding()` | removed — derived
from whether a preview is visible |
| `height(...)` (ambiguous: fixed vs max) | `max_height(...)` (plain
pickers shrink-to-fit, capped here) |
| `minimum_results_width(...)` | removed — a plain picker's min width
tracks its opening width; preview pickers use standard internal pane
mins |
| default size = 60% viewport | default = `DEFAULT_MODAL_WIDTH` (34rem)
× `DEFAULT_MODAL_MAX_HEIGHT` (24rem, max) |
| resize handles gated on `is_modal` | gated on `is_resizable` (new
`resizable(bool)` builder; auto-`true` for preview pickers) |
Call sites now only override the exceptions: narrow popover selectors
(`initial_width`), the taller outline view (`max_height`), and preview
pickers (constructed via `*_with_preview`).
## Behavior fixes
- **Wrong widths everywhere**: pickers were falling back to
60%-of-viewport because the original migration set
`minimum_results_width` but never `initial_width`. Fixed at the source
via the new defaults.
- **Popovers had no container and wouldn't dismiss**: `is_modal=false`
was suppressing both the elevated background *and* blur-dismiss. Split
out so popovers keep their chrome and dismiss on click-away/escape. This
fixed the agent-panel model/profile selectors, sidebar recent projects,
and the settings theme/font/icon/ollama pickers (which were incorrectly
using `modal(false)`).
- **Sidebar recent projects stretched to full height**: was missing the
shrink-to-fit behavior; now capped and content-sized like other
popovers.
- **Preview crash**: removed an over-strict `debug_assert!` that
panicked when previewing an empty file (`message == None && editor
empty` is valid).
- **Preview-aware default size**: pickers open at standard width with
the preview hidden, and expand to the larger "telescope" size when a
preview is shown. Fixes the text finder rendering super-wide by default,
and makes the file finder expand (rather than cram its results) when you
toggle the preview.
---
Release Notes:
- N/A
553 lines
17 KiB
Rust
553 lines
17 KiB
Rust
use std::sync::{Arc, atomic};
|
|
|
|
use fuzzy::{StringMatch, StringMatchCandidate};
|
|
use gpui::{AppContext, Entity, TestAppContext, VisualTestContext};
|
|
use picker::{Picker, PickerDelegate};
|
|
use project::Project;
|
|
use serde_json::json;
|
|
use util::path;
|
|
use workspace::{AppState, MultiWorkspace};
|
|
|
|
use crate::{CandidateInfo, DirectoryState, OpenPathDelegate};
|
|
|
|
#[gpui::test]
|
|
async fn test_open_path_prompt(cx: &mut TestAppContext) {
|
|
let app_state = init_test(cx);
|
|
app_state
|
|
.fs
|
|
.as_fake()
|
|
.insert_tree(
|
|
path!("/root"),
|
|
json!({
|
|
".a1": ".A1",
|
|
".b1": ".B1",
|
|
"a1": "A1",
|
|
"a2": "A2",
|
|
"a3": "A3",
|
|
"dir1": {},
|
|
"dir2": {
|
|
"c": "C",
|
|
"d1": "D1",
|
|
"d2": "D2",
|
|
"d3": "D3",
|
|
"dir3": {},
|
|
"dir4": {}
|
|
}
|
|
}),
|
|
)
|
|
.await;
|
|
|
|
let project = Project::test(app_state.fs.clone(), [path!("/root").as_ref()], cx).await;
|
|
|
|
let (picker, cx) = build_open_path_prompt(project, false, false, cx);
|
|
|
|
insert_query(path!("sadjaoislkdjasldj"), &picker, cx).await;
|
|
assert_eq!(collect_match_candidates(&picker, cx), Vec::<String>::new());
|
|
|
|
let query = path!("/root");
|
|
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 and not show any of the hidden entries.
|
|
let query = path!("/root/");
|
|
insert_query(query, &picker, cx).await;
|
|
assert_eq!(
|
|
collect_match_candidates(&picker, cx),
|
|
vec![expected_separator, "dir1", "dir2", "a1", "a2", "a3"]
|
|
);
|
|
|
|
// Show candidates for the query "a".
|
|
let query = path!("/root/a");
|
|
insert_query(query, &picker, cx).await;
|
|
assert_eq!(
|
|
collect_match_candidates(&picker, cx),
|
|
vec!["a1", "a2", "a3"]
|
|
);
|
|
|
|
// Show candidates for the query "d".
|
|
let query = path!("/root/d");
|
|
insert_query(query, &picker, cx).await;
|
|
assert_eq!(collect_match_candidates(&picker, cx), vec!["dir1", "dir2"]);
|
|
|
|
let query = path!("/root/dir2");
|
|
insert_query(query, &picker, cx).await;
|
|
assert_eq!(collect_match_candidates(&picker, cx), vec!["dir2"]);
|
|
|
|
let query = path!("/root/dir2/");
|
|
insert_query(query, &picker, cx).await;
|
|
assert_eq!(
|
|
collect_match_candidates(&picker, cx),
|
|
vec![expected_separator, "dir3", "dir4", "c", "d1", "d2", "d3"]
|
|
);
|
|
|
|
// Show candidates for the query "d".
|
|
let query = path!("/root/dir2/d");
|
|
insert_query(query, &picker, cx).await;
|
|
assert_eq!(
|
|
collect_match_candidates(&picker, cx),
|
|
vec!["dir3", "dir4", "d1", "d2", "d3"]
|
|
);
|
|
|
|
let query = path!("/root/dir2/di");
|
|
insert_query(query, &picker, cx).await;
|
|
assert_eq!(collect_match_candidates(&picker, cx), vec!["dir3", "dir4"]);
|
|
|
|
// Don't show candidates for the query ".".
|
|
let query = path!("/root/.");
|
|
insert_query(query, &picker, cx).await;
|
|
assert_eq!(collect_match_candidates(&picker, cx), Vec::<String>::new());
|
|
|
|
// Don't show any candidates for the query ".a".
|
|
let query = path!("/root/.a");
|
|
insert_query(query, &picker, cx).await;
|
|
assert_eq!(collect_match_candidates(&picker, cx), Vec::<String>::new());
|
|
|
|
// Show candidates for the query "./".
|
|
// Should show current directory and contents.
|
|
let query = path!("/root/./");
|
|
insert_query(query, &picker, cx).await;
|
|
assert_eq!(
|
|
collect_match_candidates(&picker, cx),
|
|
vec![expected_separator, "dir1", "dir2", "a1", "a2", "a3"]
|
|
);
|
|
|
|
// Show candidates for the query "../". Show parent contents.
|
|
let query = path!("/root/dir1/../");
|
|
insert_query(query, &picker, cx).await;
|
|
assert_eq!(
|
|
collect_match_candidates(&picker, cx),
|
|
vec![expected_separator, "dir1", "dir2", "a1", "a2", "a3"]
|
|
);
|
|
}
|
|
|
|
#[gpui::test]
|
|
async fn test_open_path_prompt_completion(cx: &mut TestAppContext) {
|
|
let app_state = init_test(cx);
|
|
app_state
|
|
.fs
|
|
.as_fake()
|
|
.insert_tree(
|
|
path!("/root"),
|
|
json!({
|
|
"a": "A",
|
|
"dir1": {},
|
|
"dir2": {
|
|
"c": "C",
|
|
"d": "D",
|
|
"dir3": {},
|
|
"dir4": {}
|
|
}
|
|
}),
|
|
)
|
|
.await;
|
|
|
|
let project = Project::test(app_state.fs.clone(), [path!("/root").as_ref()], cx).await;
|
|
|
|
let (picker, cx) = build_open_path_prompt(project, false, false, cx);
|
|
|
|
// 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).unwrap(),
|
|
path!("/root/")
|
|
);
|
|
|
|
// Confirm completion for the query "/root/", selecting the first 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, 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/dir1/"),
|
|
"Second entry is the first directory"
|
|
);
|
|
|
|
// Confirm completion for the query "/root/", selecting the third 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, 3, &picker, cx).unwrap(),
|
|
path!("/root/a")
|
|
);
|
|
|
|
let query = path!("/root/a");
|
|
insert_query(query, &picker, cx).await;
|
|
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).unwrap(),
|
|
path!("/root/dir2/")
|
|
);
|
|
|
|
let query = path!("/root/dir2");
|
|
insert_query(query, &picker, cx).await;
|
|
assert_eq!(
|
|
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, 1, &picker, cx).unwrap(),
|
|
path!("/root/dir2/dir3/")
|
|
);
|
|
|
|
let query = path!("/root/dir2/");
|
|
insert_query(query, &picker, cx).await;
|
|
assert_eq!(
|
|
confirm_completion(query, 3, &picker, cx).unwrap(),
|
|
path!("/root/dir2/c")
|
|
);
|
|
|
|
let query = path!("/root/dir2/d");
|
|
insert_query(query, &picker, cx).await;
|
|
assert_eq!(
|
|
confirm_completion(query, 0, &picker, cx).unwrap(),
|
|
path!("/root/dir2/dir3/")
|
|
);
|
|
|
|
let query = path!("/root/dir2/d");
|
|
insert_query(query, &picker, cx).await;
|
|
assert_eq!(
|
|
confirm_completion(query, 1, &picker, cx).unwrap(),
|
|
path!("/root/dir2/dir4/")
|
|
);
|
|
|
|
let query = path!("/root/dir2/di");
|
|
insert_query(query, &picker, cx).await;
|
|
assert_eq!(
|
|
confirm_completion(query, 1, &picker, cx).unwrap(),
|
|
path!("/root/dir2/dir4/")
|
|
);
|
|
}
|
|
|
|
#[gpui::test]
|
|
#[cfg_attr(not(target_os = "windows"), ignore)]
|
|
async fn test_open_path_prompt_on_windows(cx: &mut TestAppContext) {
|
|
let app_state = init_test(cx);
|
|
app_state
|
|
.fs
|
|
.as_fake()
|
|
.insert_tree(
|
|
path!("/root"),
|
|
json!({
|
|
"a": "A",
|
|
"dir1": {},
|
|
"dir2": {}
|
|
}),
|
|
)
|
|
.await;
|
|
|
|
let project = Project::test(app_state.fs.clone(), [path!("/root").as_ref()], cx).await;
|
|
|
|
let (picker, cx) = build_open_path_prompt(project, false, false, cx);
|
|
|
|
// Support both forward and backward slashes.
|
|
let query = "C:/root/";
|
|
insert_query(query, &picker, cx).await;
|
|
assert_eq!(
|
|
collect_match_candidates(&picker, cx),
|
|
vec![".\\", "dir1", "dir2", "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(),
|
|
"C:/root/dir1\\",
|
|
"Second entry is the first directory"
|
|
);
|
|
|
|
let query = "C:\\root/";
|
|
insert_query(query, &picker, cx).await;
|
|
assert_eq!(
|
|
collect_match_candidates(&picker, cx),
|
|
vec![".\\", "dir1", "dir2", "a"]
|
|
);
|
|
assert_eq!(
|
|
confirm_completion(query, 1, &picker, cx).unwrap(),
|
|
"C:\\root/dir1\\"
|
|
);
|
|
|
|
let query = "C:\\root\\";
|
|
insert_query(query, &picker, cx).await;
|
|
assert_eq!(
|
|
collect_match_candidates(&picker, cx),
|
|
vec![".\\", "dir1", "dir2", "a"]
|
|
);
|
|
assert_eq!(
|
|
confirm_completion(query, 1, &picker, cx).unwrap(),
|
|
"C:\\root\\dir1\\"
|
|
);
|
|
|
|
// 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).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).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).unwrap(),
|
|
"C:\\root\\dir1\\"
|
|
);
|
|
}
|
|
|
|
#[gpui::test]
|
|
async fn test_new_path_prompt(cx: &mut TestAppContext) {
|
|
let app_state = init_test(cx);
|
|
app_state
|
|
.fs
|
|
.as_fake()
|
|
.insert_tree(
|
|
path!("/root"),
|
|
json!({
|
|
"a1": "A1",
|
|
"a2": "A2",
|
|
"a3": "A3",
|
|
"dir1": {},
|
|
"dir2": {
|
|
"c": "C",
|
|
"d1": "D1",
|
|
"d2": "D2",
|
|
"d3": "D3",
|
|
"dir3": {},
|
|
"dir4": {}
|
|
}
|
|
}),
|
|
)
|
|
.await;
|
|
|
|
let project = Project::test(app_state.fs.clone(), [path!("/root").as_ref()], cx).await;
|
|
|
|
let (picker, cx) = build_open_path_prompt(project, true, false, cx);
|
|
|
|
insert_query(path!("/root"), &picker, cx).await;
|
|
assert_eq!(collect_match_candidates(&picker, cx), vec!["root"]);
|
|
|
|
insert_query(path!("/root/d"), &picker, cx).await;
|
|
assert_eq!(
|
|
collect_match_candidates(&picker, cx),
|
|
vec!["d", "dir1", "dir2"]
|
|
);
|
|
|
|
insert_query(path!("/root/dir1"), &picker, cx).await;
|
|
assert_eq!(collect_match_candidates(&picker, cx), vec!["dir1"]);
|
|
|
|
insert_query(path!("/root/dir12"), &picker, cx).await;
|
|
assert_eq!(collect_match_candidates(&picker, cx), vec!["dir12"]);
|
|
|
|
insert_query(path!("/root/dir1"), &picker, cx).await;
|
|
assert_eq!(collect_match_candidates(&picker, cx), vec!["dir1"]);
|
|
}
|
|
|
|
#[gpui::test]
|
|
async fn test_open_path_prompt_panics_with_stale_highlight_positions(cx: &mut TestAppContext) {
|
|
let app_state = init_test(cx);
|
|
app_state
|
|
.fs
|
|
.as_fake()
|
|
.insert_tree(path!("/root"), json!({}))
|
|
.await;
|
|
|
|
let project = Project::test(app_state.fs.clone(), [path!("/root").as_ref()], cx).await;
|
|
let (picker, cx) = build_open_path_prompt(project, false, false, cx);
|
|
|
|
picker.update_in(cx, |picker, window, cx| {
|
|
picker.delegate.prompt_root = "/".to_string();
|
|
picker.delegate.directory_state = DirectoryState::List {
|
|
parent_path: picker.delegate.prompt_root.clone(),
|
|
entries: vec![CandidateInfo {
|
|
path: StringMatchCandidate::new(0, "éclair"),
|
|
is_dir: false,
|
|
}],
|
|
error: None,
|
|
};
|
|
picker.delegate.string_matches = vec![StringMatch {
|
|
candidate_id: 0,
|
|
score: 0.0,
|
|
positions: vec![1],
|
|
string: "ab".to_string(),
|
|
}];
|
|
|
|
picker.delegate.render_match(0, false, window, cx);
|
|
});
|
|
}
|
|
|
|
#[gpui::test]
|
|
async fn test_open_path_prompt_with_show_hidden(cx: &mut TestAppContext) {
|
|
let app_state = init_test(cx);
|
|
app_state
|
|
.fs
|
|
.as_fake()
|
|
.insert_tree(
|
|
path!("/root"),
|
|
json!({
|
|
"directory_1": {},
|
|
"directory_2": {},
|
|
".hidden": {}
|
|
}),
|
|
)
|
|
.await;
|
|
|
|
let project = Project::test(app_state.fs.clone(), [path!("/root").as_ref()], cx).await;
|
|
|
|
let (picker, cx) = build_open_path_prompt(project, false, true, cx);
|
|
|
|
#[cfg(not(windows))]
|
|
let expected_separator = "./";
|
|
#[cfg(windows)]
|
|
let expected_separator = ".\\";
|
|
|
|
insert_query(path!("/root/"), &picker, cx).await;
|
|
assert_eq!(
|
|
collect_match_candidates(&picker, cx),
|
|
vec![expected_separator, ".hidden", "directory_1", "directory_2"]
|
|
);
|
|
|
|
insert_query(path!("/root/."), &picker, cx).await;
|
|
assert_eq!(collect_match_candidates(&picker, cx), vec![".hidden"]);
|
|
}
|
|
|
|
#[gpui::test]
|
|
async fn test_dismiss_cancels_in_flight_match(cx: &mut TestAppContext) {
|
|
let app_state = init_test(cx);
|
|
app_state
|
|
.fs
|
|
.as_fake()
|
|
.insert_tree(
|
|
path!("/root"),
|
|
json!({
|
|
"a1": "A1",
|
|
"a2": "A2",
|
|
"a3": "A3",
|
|
}),
|
|
)
|
|
.await;
|
|
|
|
let project = Project::test(app_state.fs.clone(), [path!("/root").as_ref()], cx).await;
|
|
let (picker, cx) = build_open_path_prompt(project, false, false, cx);
|
|
|
|
insert_query(path!("/root/a"), &picker, cx).await;
|
|
|
|
let cancel_flag = picker.read_with(cx, |picker, _| picker.delegate.cancel_flag.clone());
|
|
assert!(
|
|
!cancel_flag.load(atomic::Ordering::Acquire),
|
|
"cancel flag should be clear while the prompt is active"
|
|
);
|
|
|
|
picker.update_in(cx, |picker, window, cx| {
|
|
picker.delegate.dismissed(window, cx);
|
|
});
|
|
|
|
assert!(
|
|
cancel_flag.load(atomic::Ordering::Acquire),
|
|
"dismissing the prompt must cancel the in-flight fuzzy match"
|
|
);
|
|
}
|
|
|
|
fn init_test(cx: &mut TestAppContext) -> Arc<AppState> {
|
|
cx.update(|cx| {
|
|
let state = AppState::test(cx);
|
|
theme_settings::init(theme::LoadThemes::JustBase, cx);
|
|
|
|
editor::init(cx);
|
|
state
|
|
})
|
|
}
|
|
|
|
fn build_open_path_prompt(
|
|
project: Entity<Project>,
|
|
creating_path: bool,
|
|
show_hidden: bool,
|
|
cx: &mut TestAppContext,
|
|
) -> (Entity<Picker<OpenPathDelegate>>, &mut VisualTestContext) {
|
|
let (tx, _) = futures::channel::oneshot::channel();
|
|
let lister = project::DirectoryLister::Project(project.clone());
|
|
|
|
let (multi_workspace, cx) =
|
|
cx.add_window_view(|window, cx| MultiWorkspace::test_new(project, window, cx));
|
|
let workspace = multi_workspace.read_with(cx, |mw, _| mw.workspace().clone());
|
|
(
|
|
workspace.update_in(cx, |_, window, cx| {
|
|
let delegate = OpenPathDelegate::new(tx, lister.clone(), creating_path, cx);
|
|
let delegate = if show_hidden {
|
|
delegate.show_hidden()
|
|
} else {
|
|
delegate
|
|
};
|
|
cx.new(|cx| {
|
|
let picker = Picker::uniform_list(delegate, window, cx).embedded();
|
|
let query = lister.default_query(cx);
|
|
picker.set_query(&query, window, cx);
|
|
picker
|
|
})
|
|
}),
|
|
cx,
|
|
)
|
|
}
|
|
|
|
async fn insert_query(
|
|
query: &str,
|
|
picker: &Entity<Picker<OpenPathDelegate>>,
|
|
cx: &mut VisualTestContext,
|
|
) {
|
|
picker
|
|
.update_in(cx, |f, window, cx| {
|
|
f.delegate.update_matches(query.to_string(), window, cx)
|
|
})
|
|
.await;
|
|
}
|
|
|
|
fn confirm_completion(
|
|
query: &str,
|
|
select: usize,
|
|
picker: &Entity<Picker<OpenPathDelegate>>,
|
|
cx: &mut VisualTestContext,
|
|
) -> 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(
|
|
picker: &Entity<Picker<OpenPathDelegate>>,
|
|
cx: &mut VisualTestContext,
|
|
) -> Vec<String> {
|
|
picker.update(cx, |f, _| f.delegate.collect_match_candidates())
|
|
}
|