workspace: Add trailing / to directories on completion when using OpenPathPrompt (#25430)

Closes #25045

With the setting `"use_system_path_prompts": false`, previously, if the
completion target was a directory, no separator would be added after it,
requiring us to manually append a `/` or `\`. Now, if the completion
target is a directory, a `/` or `\` will be automatically added. On
Windows, both `/` and `\` are considered valid path separators.



https://github.com/user-attachments/assets/0594ce27-9693-4a49-ae0e-3ed29f62526a



Release Notes:

- N/A
This commit is contained in:
张小白 2025-03-04 14:01:08 +08:00 committed by GitHub
parent 8c4da9fba0
commit 11b79d0ab9
No known key found for this signature in database
GPG key ID: B5690EEEBB952194
8 changed files with 472 additions and 41 deletions

View file

@ -1,5 +1,7 @@
#[cfg(test)]
mod file_finder_tests;
#[cfg(test)]
mod open_path_prompt_tests;
pub mod file_finder_settings;
mod new_path_prompt;

View file

@ -3,7 +3,7 @@ use fuzzy::StringMatchCandidate;
use picker::{Picker, PickerDelegate};
use project::DirectoryLister;
use std::{
path::{Path, PathBuf},
path::{Path, PathBuf, MAIN_SEPARATOR_STR},
sync::{
atomic::{self, AtomicBool},
Arc,
@ -38,14 +38,38 @@ impl OpenPathDelegate {
should_dismiss: true,
}
}
#[cfg(any(test, feature = "test-support"))]
pub fn collect_match_candidates(&self) -> Vec<String> {
if let Some(state) = self.directory_state.as_ref() {
self.matches
.iter()
.filter_map(|&index| {
state
.match_candidates
.get(index)
.map(|candidate| candidate.path.string.clone())
})
.collect()
} else {
Vec::new()
}
}
}
#[derive(Debug)]
struct DirectoryState {
path: String,
match_candidates: Vec<StringMatchCandidate>,
match_candidates: Vec<CandidateInfo>,
error: Option<SharedString>,
}
#[derive(Debug, Clone)]
struct CandidateInfo {
path: StringMatchCandidate,
is_dir: bool,
}
impl OpenPathPrompt {
pub(crate) fn register(
workspace: &mut Workspace,
@ -93,8 +117,6 @@ impl PickerDelegate for OpenPathDelegate {
cx.notify();
}
// todo(windows)
// Is this method woring correctly on Windows? This method uses `/` for path separator.
fn update_matches(
&mut self,
query: String,
@ -102,13 +124,26 @@ impl PickerDelegate for OpenPathDelegate {
cx: &mut Context<Picker<Self>>,
) -> gpui::Task<()> {
let lister = self.lister.clone();
let (mut dir, suffix) = if let Some(index) = query.rfind('/') {
(query[..index].to_string(), query[index + 1..].to_string())
let query_path = Path::new(&query);
let last_item = query_path
.file_name()
.unwrap_or_default()
.to_string_lossy()
.to_string();
let (mut dir, suffix) = if let Some(dir) = query.strip_suffix(&last_item) {
(dir.to_string(), last_item)
} else {
(query, String::new())
};
if dir == "" {
dir = "/".to_string();
#[cfg(not(target_os = "windows"))]
{
dir = "/".to_string();
}
#[cfg(target_os = "windows")]
{
dir = "C:\\".to_string();
}
}
let query = if self
@ -134,12 +169,16 @@ impl PickerDelegate for OpenPathDelegate {
this.update(&mut cx, |this, _| {
this.delegate.directory_state = Some(match paths {
Ok(mut paths) => {
paths.sort_by(|a, b| compare_paths((a, true), (b, true)));
paths.sort_by(|a, b| compare_paths((&a.path, true), (&b.path, true)));
let match_candidates = paths
.iter()
.enumerate()
.map(|(ix, path)| {
StringMatchCandidate::new(ix, &path.to_string_lossy())
.map(|(ix, item)| CandidateInfo {
path: StringMatchCandidate::new(
ix,
&item.path.to_string_lossy(),
),
is_dir: item.is_dir,
})
.collect::<Vec<_>>();
@ -178,7 +217,7 @@ impl PickerDelegate for OpenPathDelegate {
};
if !suffix.starts_with('.') {
match_candidates.retain(|m| !m.string.starts_with('.'));
match_candidates.retain(|m| !m.path.string.starts_with('.'));
}
if suffix == "" {
@ -186,7 +225,7 @@ impl PickerDelegate for OpenPathDelegate {
this.delegate.matches.clear();
this.delegate
.matches
.extend(match_candidates.iter().map(|m| m.id));
.extend(match_candidates.iter().map(|m| m.path.id));
cx.notify();
})
@ -194,8 +233,9 @@ impl PickerDelegate for OpenPathDelegate {
return;
}
let candidates = match_candidates.iter().map(|m| &m.path).collect::<Vec<_>>();
let matches = fuzzy::match_strings(
match_candidates.as_slice(),
candidates.as_slice(),
&suffix,
false,
100,
@ -217,7 +257,7 @@ impl PickerDelegate for OpenPathDelegate {
this.delegate.directory_state.as_ref().and_then(|d| {
d.match_candidates
.get(*m)
.map(|c| !c.string.starts_with(&suffix))
.map(|c| !c.path.string.starts_with(&suffix))
}),
*m,
)
@ -239,7 +279,16 @@ impl PickerDelegate for OpenPathDelegate {
let m = self.matches.get(self.selected_index)?;
let directory_state = self.directory_state.as_ref()?;
let candidate = directory_state.match_candidates.get(*m)?;
Some(format!("{}/{}", directory_state.path, candidate.string))
Some(format!(
"{}{}{}",
directory_state.path,
candidate.path.string,
if candidate.is_dir {
MAIN_SEPARATOR_STR
} else {
""
}
))
})
.unwrap_or(query),
)
@ -260,7 +309,7 @@ impl PickerDelegate for OpenPathDelegate {
.resolve_tilde(&directory_state.path, cx)
.as_ref(),
)
.join(&candidate.string);
.join(&candidate.path.string);
if let Some(tx) = self.tx.take() {
tx.send(Some(vec![result])).ok();
}
@ -294,7 +343,7 @@ impl PickerDelegate for OpenPathDelegate {
.spacing(ListItemSpacing::Sparse)
.inset(true)
.toggle_state(selected)
.child(LabelLike::new().child(candidate.string.clone())),
.child(LabelLike::new().child(candidate.path.string.clone())),
)
}
@ -307,6 +356,6 @@ impl PickerDelegate for OpenPathDelegate {
}
fn placeholder_text(&self, _window: &mut Window, _cx: &mut App) -> Arc<str> {
Arc::from("[directory/]filename.ext")
Arc::from(format!("[directory{MAIN_SEPARATOR_STR}]filename.ext"))
}
}

View file

@ -0,0 +1,324 @@
use std::sync::Arc;
use gpui::{AppContext, Entity, TestAppContext, VisualTestContext};
use picker::{Picker, PickerDelegate};
use project::Project;
use serde_json::json;
use ui::rems;
use util::path;
use workspace::{AppState, Workspace};
use crate::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",
"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, cx);
let query = path!("/root");
insert_query(query, &picker, cx).await;
assert_eq!(collect_match_candidates(&picker, cx), vec!["root"]);
// 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"]
);
// 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!["c", "d1", "d2", "d3", "dir3", "dir4"]
);
// 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!["d1", "d2", "d3", "dir3", "dir4"]
);
let query = path!("/root/dir2/di");
insert_query(query, &picker, cx).await;
assert_eq!(collect_match_candidates(&picker, cx), vec!["dir3", "dir4"]);
}
#[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, 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), 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"));
// 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),
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"));
let query = path!("/root/d");
insert_query(query, &picker, cx).await;
assert_eq!(
confirm_completion(query, 1, &picker, cx),
path!("/root/dir2/")
);
let query = path!("/root/dir2");
insert_query(query, &picker, cx).await;
assert_eq!(
confirm_completion(query, 0, &picker, cx),
path!("/root/dir2/")
);
let query = path!("/root/dir2/");
insert_query(query, &picker, cx).await;
assert_eq!(
confirm_completion(query, 0, &picker, cx),
path!("/root/dir2/c")
);
let query = path!("/root/dir2/");
insert_query(query, &picker, cx).await;
assert_eq!(
confirm_completion(query, 2, &picker, cx),
path!("/root/dir2/dir3/")
);
let query = path!("/root/dir2/d");
insert_query(query, &picker, cx).await;
assert_eq!(
confirm_completion(query, 0, &picker, cx),
path!("/root/dir2/d")
);
let query = path!("/root/dir2/d");
insert_query(query, &picker, cx).await;
assert_eq!(
confirm_completion(query, 1, &picker, cx),
path!("/root/dir2/dir3/")
);
let query = path!("/root/dir2/di");
insert_query(query, &picker, cx).await;
assert_eq!(
confirm_completion(query, 1, &picker, cx),
path!("/root/dir2/dir4/")
);
}
#[gpui::test]
#[cfg(target_os = "windows")]
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, 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!["a", "dir1", "dir2"]
);
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"]
);
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"]
);
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\\");
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\\");
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\\"
);
}
fn init_test(cx: &mut TestAppContext) -> Arc<AppState> {
cx.update(|cx| {
let state = AppState::test(cx);
theme::init(theme::LoadThemes::JustBase, cx);
language::init(cx);
super::init(cx);
editor::init(cx);
workspace::init_settings(cx);
Project::init_settings(cx);
state
})
}
fn build_open_path_prompt(
project: Entity<Project>,
cx: &mut TestAppContext,
) -> (Entity<Picker<OpenPathDelegate>>, &mut VisualTestContext) {
let (tx, _) = futures::channel::oneshot::channel();
let lister = project::DirectoryLister::Project(project.clone());
let delegate = OpenPathDelegate::new(tx, lister.clone());
let (workspace, cx) = cx.add_window_view(|window, cx| Workspace::test_new(project, window, cx));
(
workspace.update_in(cx, |_, window, cx| {
cx.new(|cx| {
let picker = Picker::uniform_list(delegate, window, cx)
.width(rems(34.))
.modal(false);
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,
) -> 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()
}
fn collect_match_candidates(
picker: &Entity<Picker<OpenPathDelegate>>,
cx: &mut VisualTestContext,
) -> Vec<String> {
picker.update(cx, |f, _| f.delegate.collect_match_candidates())
}

View file

@ -1,5 +1,5 @@
use std::{
borrow::Cow,
borrow::{Borrow, Cow},
sync::atomic::{self, AtomicBool},
};
@ -50,22 +50,24 @@ impl<'a> Matcher<'a> {
/// Filter and score fuzzy match candidates. Results are returned unsorted, in the same order as
/// the input candidates.
pub fn match_candidates<C: MatchCandidate, R, F>(
pub fn match_candidates<C, R, F, T>(
&mut self,
prefix: &[char],
lowercase_prefix: &[char],
candidates: impl Iterator<Item = C>,
candidates: impl Iterator<Item = T>,
results: &mut Vec<R>,
cancel_flag: &AtomicBool,
build_match: F,
) where
C: MatchCandidate,
T: Borrow<C>,
F: Fn(&C, f64, &Vec<usize>) -> R,
{
let mut candidate_chars = Vec::new();
let mut lowercase_candidate_chars = Vec::new();
for candidate in candidates {
if !candidate.has_chars(self.query_char_bag) {
if !candidate.borrow().has_chars(self.query_char_bag) {
continue;
}
@ -75,7 +77,7 @@ impl<'a> Matcher<'a> {
candidate_chars.clear();
lowercase_candidate_chars.clear();
for c in candidate.to_string().chars() {
for c in candidate.borrow().to_string().chars() {
candidate_chars.push(c);
lowercase_candidate_chars.append(&mut c.to_lowercase().collect::<Vec<_>>());
}
@ -98,7 +100,11 @@ impl<'a> Matcher<'a> {
);
if score > 0.0 {
results.push(build_match(&candidate, score, &self.match_positions));
results.push(build_match(
candidate.borrow(),
score,
&self.match_positions,
));
}
}
}

View file

@ -4,7 +4,7 @@ use crate::{
};
use gpui::BackgroundExecutor;
use std::{
borrow::Cow,
borrow::{Borrow, Cow},
cmp::{self, Ordering},
iter,
ops::Range,
@ -113,14 +113,17 @@ impl Ord for StringMatch {
}
}
pub async fn match_strings(
candidates: &[StringMatchCandidate],
pub async fn match_strings<T>(
candidates: &[T],
query: &str,
smart_case: bool,
max_results: usize,
cancel_flag: &AtomicBool,
executor: BackgroundExecutor,
) -> Vec<StringMatch> {
) -> Vec<StringMatch>
where
T: Borrow<StringMatchCandidate> + Sync,
{
if candidates.is_empty() || max_results == 0 {
return Default::default();
}
@ -129,10 +132,10 @@ pub async fn match_strings(
return candidates
.iter()
.map(|candidate| StringMatch {
candidate_id: candidate.id,
candidate_id: candidate.borrow().id,
score: 0.,
positions: Default::default(),
string: candidate.string.clone(),
string: candidate.borrow().string.clone(),
})
.collect();
}
@ -163,10 +166,12 @@ pub async fn match_strings(
matcher.match_candidates(
&[],
&[],
candidates[segment_start..segment_end].iter(),
candidates[segment_start..segment_end]
.iter()
.map(|c| c.borrow()),
results,
cancel_flag,
|candidate, score, positions| StringMatch {
|candidate: &&StringMatchCandidate, score, positions| StringMatch {
candidate_id: candidate.id,
score,
positions: positions.clone(),

View file

@ -524,6 +524,12 @@ enum EntitySubscription {
SettingsObserver(PendingEntitySubscription<SettingsObserver>),
}
#[derive(Debug, Clone)]
pub struct DirectoryItem {
pub path: PathBuf,
pub is_dir: bool,
}
#[derive(Clone)]
pub enum DirectoryLister {
Project(Entity<Project>),
@ -552,10 +558,10 @@ impl DirectoryLister {
return worktree.read(cx).abs_path().to_string_lossy().to_string();
}
};
"~/".to_string()
format!("~{}", std::path::MAIN_SEPARATOR_STR)
}
pub fn list_directory(&self, path: String, cx: &mut App) -> Task<Result<Vec<PathBuf>>> {
pub fn list_directory(&self, path: String, cx: &mut App) -> Task<Result<Vec<DirectoryItem>>> {
match self {
DirectoryLister::Project(project) => {
project.update(cx, |project, cx| project.list_directory(path, cx))
@ -568,8 +574,12 @@ impl DirectoryLister {
let query = Path::new(expanded.as_ref());
let mut response = fs.read_dir(query).await?;
while let Some(path) = response.next().await {
if let Some(file_name) = path?.file_name() {
results.push(PathBuf::from(file_name.to_os_string()));
let path = path?;
if let Some(file_name) = path.file_name() {
results.push(DirectoryItem {
path: PathBuf::from(file_name.to_os_string()),
is_dir: fs.is_dir(&path).await,
});
}
}
Ok(results)
@ -3491,7 +3501,7 @@ impl Project {
&self,
query: String,
cx: &mut Context<Self>,
) -> Task<Result<Vec<PathBuf>>> {
) -> Task<Result<Vec<DirectoryItem>>> {
if self.is_local() {
DirectoryLister::Local(self.fs.clone()).list_directory(query, cx)
} else if let Some(session) = self.ssh_client.as_ref() {
@ -3499,12 +3509,23 @@ impl Project {
let request = proto::ListRemoteDirectory {
dev_server_id: SSH_PROJECT_ID,
path: path_buf.to_proto(),
config: Some(proto::ListRemoteDirectoryConfig { is_dir: true }),
};
let response = session.read(cx).proto_client().request(request);
cx.background_spawn(async move {
let response = response.await?;
Ok(response.entries.into_iter().map(PathBuf::from).collect())
let proto::ListRemoteDirectoryResponse {
entries,
entry_info,
} = response.await?;
Ok(entries
.into_iter()
.zip(entry_info)
.map(|(entry, info)| DirectoryItem {
path: PathBuf::from(entry),
is_dir: info.is_dir,
})
.collect())
})
} else {
Task::ready(Err(anyhow!("cannot list directory in remote project")))

View file

@ -572,13 +572,23 @@ message JoinProject {
uint64 project_id = 1;
}
message ListRemoteDirectoryConfig {
bool is_dir = 1;
}
message ListRemoteDirectory {
uint64 dev_server_id = 1;
string path = 2;
ListRemoteDirectoryConfig config = 3;
}
message EntryInfo {
bool is_dir = 1;
}
message ListRemoteDirectoryResponse {
repeated string entries = 1;
repeated EntryInfo entry_info = 2;
}
message JoinProjectResponse {

View file

@ -554,15 +554,29 @@ impl HeadlessProject {
) -> Result<proto::ListRemoteDirectoryResponse> {
let fs = cx.read_entity(&this, |this, _| this.fs.clone())?;
let expanded = PathBuf::from_proto(shellexpand::tilde(&envelope.payload.path).to_string());
let check_info = envelope
.payload
.config
.as_ref()
.is_some_and(|config| config.is_dir);
let mut entries = Vec::new();
let mut entry_info = Vec::new();
let mut response = fs.read_dir(&expanded).await?;
while let Some(path) = response.next().await {
if let Some(file_name) = path?.file_name() {
let path = path?;
if let Some(file_name) = path.file_name() {
entries.push(file_name.to_string_lossy().to_string());
if check_info {
let is_dir = fs.is_dir(&path).await;
entry_info.push(proto::EntryInfo { is_dir });
}
}
}
Ok(proto::ListRemoteDirectoryResponse { entries })
Ok(proto::ListRemoteDirectoryResponse {
entries,
entry_info,
})
}
pub async fn handle_get_path_metadata(