file_finder: Don’t use focused file’s directory for CreateNew unless it belongs to project worktrees (#42076)
## Summary Creating a new file via the file picker incorrectly used the focused file's directory as the target even when that file did not belong to any open worktree (e.g., Zed opened to a single file, settings.json focused without a worktree, or an external file focused). This PR changes the selection logic so the picker only uses the focused file's worktree if that file actually belongs to one of the visible project worktrees; otherwise, it falls back to the existing project heuristic (query-root match or project default), which prevents misdirected file creation. This addresses [zed-industries/zed#41940](https://github.com/zed-industries/zed/issues/41940). --- ## Problem ### Scenarios that errored or created in the wrong place: 1. **Zed opened to a single file from the CLI** (no worktree) - Create New was offered and then failed or targeted a non-project directory 2. **No worktree open with settings.json focused** - Same failure mode as above 3. **A worktree is open but settings.json is focused** - Create New used settings.json's directory 4. **A worktree is open but an external (non-worktree) file is focused** - Create New used the external file's directory ### Root Cause `set_search_matches` unconditionally overwrote `expect_worktree` using the `currently_opened_path` worktree_id, even if the focused file was not part of any visible worktree. --- ## Fix ### Query-derived worktree remains primary: - Try to match the query path against the roots of visible worktrees - If it matches, use that worktree and strip the root prefix from the query to get the relative path ### Only use the focused file as a secondary signal if it is relevant: - Check whether the focused file's `worktree_id` exists within visible worktrees - Only then override `expect_worktree` If no worktree is determined (e.g., no worktrees are open), Create New is not offered. --- ## Implementation Details - **Iterate over `&available_worktree`** when scanning roots and clone the matched entity to avoid moving out of the iterator - **Validate focused file worktree membership:** - Compute `focused_file_in_available_worktree` by scanning visible worktrees for a matching id - Override `expect_worktree` only if that check passes - **Preserve existing guard rails for Create New:** - Only push `Match::CreateNew` when a concrete `expect_worktree` is present and the query doesn't end with a trailing separator --- ## Key Code Changes ### Before Always overwrote `expect_worktree` with the focused file's `worktree_id`, even for external or non-project files. ### After Only override `expect_worktree` when the focused file belongs to a visible worktree. Otherwise, keep the query-derived or default project worktree. --- ## User-Facing Behavior ### No Worktree Open *Example: Zed launched with a single file or only settings.json visible* - The file picker will **not** offer "Create New" ### Worktree Open + Non-Project File Focused *Example: A non-project file or settings.json is in focus* - "Create New" is offered - New file is created within the project worktree (based on root match or project default) - New file is **never** created beside the external file ### Multiple Worktrees Open + Query Rooted to One *Example: Query specifies a particular worktree root* - The worktree is selected by root-name match - Project default selection applies if no match is found --- ## Tests ### Added: `test_create_file_focused_file_not_belong_to_available_worktrees` 1. Set up two worktrees A and B; open an external file that belongs to neither 2. Use the file picker to create "new-file.txt" 3. Assert the new file opens in an editor whose `ProjectPath.worktree_id` equals either A or B, and the relative path is "new-file.txt" Existing tests for Create New, absolute/relative matching, and multi-worktree behavior continue to pass. --- ## Reproduction Steps (Pre-Fix) 1. Launch Zed with a single file: ```bash zed /tmp/foo.txt ``` Or open settings.json with no project 2. Open the file finder and type a new filename, then press Enter 3. Observe Create New trying to use the focused file's directory or failing unexpectedly --- ## Expected vs Actual ### Expected: - No Create New when there is no project worktree - When a project exists, Create New targets the appropriate worktree, not the focused external file's directory ### Actual (pre-fix): - Create New used the focused file's directory, even if it was external or unrelated to the project --- ## Migration and Compatibility - No user configuration changes required - Behavior now aligns with user expectation: Create New is only offered when a project worktree is available and it always targets a project worktree --- ## Reference **Fixes:** [zed-industries/zed#41940](https://github.com/zed-industries/zed/issues/41940) Fixes #41940 --- ## Appendix: Code Snippets ### Guard before overriding with focused file: ```rust let focused_file_in_available_worktree = available_worktree.iter().any(|wt| wt.read(cx).id() == worktree_id); if focused_file_in_available_worktree { expect_worktree = project.worktree_for_id(worktree_id, cx); } ``` ### Root-based worktree selection with non-moving iteration: ```rust for worktree in &available_worktree { if query_path.strip_prefix(worktree.read(cx).root_name()).is_ok() { expect_worktree = Some(worktree.clone()); … } } ``` --- This PR includes a targeted test ensuring that when the focused file is outside all worktrees, Create New still creates within the project worktree(s), preventing regressions. --- ## Release Notes - Fixed "Create New" in the file picker targeting the wrong directory when a non-project file was focused. --- --------- Co-authored-by: Kirill Bulatov <kirill@zed.dev> Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
This commit is contained in:
1 parent
b8b55f5def
commit
2d85fe4bd5
2 files changed
+96
-3
No files matched your search
@@ -980,12 +980,12 @@ impl FileFinderDelegate {
|
||||
.collect::<Vec<_>>();
|
||||
let worktree_count = available_worktree.len();
|
||||
let mut expect_worktree = available_worktree.first().cloned();
|
||||
for worktree in available_worktree {
|
||||
for worktree in &available_worktree {
|
||||
let worktree_root = worktree.read(cx).root_name();
|
||||
if worktree_count > 1 {
|
||||
if let Ok(suffix) = query_path.strip_prefix(worktree_root) {
|
||||
query_path = Cow::Owned(suffix.to_owned());
|
||||
expect_worktree = Some(worktree);
|
||||
expect_worktree = Some(worktree.clone());
|
||||
break;
|
||||
}
|
||||
}
|
||||
@@ -993,7 +993,13 @@ impl FileFinderDelegate {
|
||||
|
||||
if let Some(FoundPath { ref project, .. }) = self.currently_opened_path {
|
||||
let worktree_id = project.worktree_id;
|
||||
expect_worktree = self.project.read(cx).worktree_for_id(worktree_id, cx);
|
||||
let focused_file_in_available_worktree = available_worktree
|
||||
.iter()
|
||||
.any(|wt| wt.read(cx).id() == worktree_id);
|
||||
|
||||
if focused_file_in_available_worktree {
|
||||
expect_worktree = self.project.read(cx).worktree_for_id(worktree_id, cx);
|
||||
}
|
||||
}
|
||||
|
||||
if let Some(worktree) = expect_worktree {
|
||||
|
||||
@@ -1260,6 +1260,93 @@ async fn test_create_file_for_multiple_worktrees(cx: &mut TestAppContext) {
|
||||
});
|
||||
}
|
||||
|
||||
#[gpui::test]
|
||||
async fn test_create_file_focused_file_does_not_belong_to_available_worktrees(
|
||||
cx: &mut TestAppContext,
|
||||
) {
|
||||
let app_state = init_test(cx);
|
||||
app_state
|
||||
.fs
|
||||
.as_fake()
|
||||
.insert_tree(path!("/roota"), json!({ "the-parent-dira": { "filea": ""}}))
|
||||
.await;
|
||||
|
||||
app_state
|
||||
.fs
|
||||
.as_fake()
|
||||
.insert_tree(path!("/rootb"), json!({"the-parent-dirb":{ "fileb": ""}}))
|
||||
.await;
|
||||
|
||||
let project = Project::test(
|
||||
app_state.fs.clone(),
|
||||
[path!("/roota").as_ref(), path!("/rootb").as_ref()],
|
||||
cx,
|
||||
)
|
||||
.await;
|
||||
|
||||
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());
|
||||
|
||||
let (worktree_id_a, worktree_id_b) = cx.read(|cx| {
|
||||
let worktrees = workspace.read(cx).worktrees(cx).collect::<Vec<_>>();
|
||||
(worktrees[0].read(cx).id(), worktrees[1].read(cx).id())
|
||||
});
|
||||
workspace
|
||||
.update_in(cx, |workspace, window, cx| {
|
||||
workspace.open_abs_path(
|
||||
PathBuf::from(path!("/external/external-file.txt")),
|
||||
OpenOptions {
|
||||
visible: Some(OpenVisible::None),
|
||||
..OpenOptions::default()
|
||||
},
|
||||
window,
|
||||
cx,
|
||||
)
|
||||
})
|
||||
.await
|
||||
.unwrap();
|
||||
|
||||
cx.run_until_parked();
|
||||
let finder = open_file_picker(&workspace, cx);
|
||||
|
||||
finder
|
||||
.update_in(cx, |f, window, cx| {
|
||||
f.delegate
|
||||
.spawn_search(test_path_position("new-file.txt"), window, cx)
|
||||
})
|
||||
.await;
|
||||
|
||||
cx.run_until_parked();
|
||||
finder.update_in(cx, |f, window, cx| {
|
||||
assert_eq!(f.delegate.matches.len(), 1);
|
||||
f.delegate.confirm(false, window, cx); // ✓ works
|
||||
});
|
||||
cx.run_until_parked();
|
||||
|
||||
cx.read(|cx| {
|
||||
let active_editor = workspace.read(cx).active_item_as::<Editor>(cx).unwrap();
|
||||
|
||||
let project_path = active_editor.read(cx).project_path(cx);
|
||||
|
||||
assert!(
|
||||
project_path.is_some(),
|
||||
"Active editor should have a project path"
|
||||
);
|
||||
|
||||
let project_path = project_path.unwrap();
|
||||
|
||||
assert!(
|
||||
project_path.worktree_id == worktree_id_a || project_path.worktree_id == worktree_id_b,
|
||||
"New file should be created in one of the available worktrees (A or B), \
|
||||
not in a directory derived from the external file. Got worktree_id: {:?}",
|
||||
project_path.worktree_id
|
||||
);
|
||||
|
||||
assert_eq!(project_path.path.as_ref(), rel_path("new-file.txt"));
|
||||
});
|
||||
}
|
||||
|
||||
#[gpui::test]
|
||||
async fn test_create_file_no_focused_with_multiple_worktrees(cx: &mut TestAppContext) {
|
||||
let app_state = init_test(cx);
|
||||
|
||||
Reference in new issue
Block a user