From 32fa09132e1a6eaa11709997c1d7edc8aee7938e Mon Sep 17 00:00:00 2001 From: Pedro Guedes <30536640+pdrgds@users.noreply.github.com> Date: Mon, 20 Apr 2026 05:45:48 -0300 Subject: [PATCH] workspace: Skip read-only paths when choosing default save location (#53100) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Fixes #42787 ## Summary - When "Go to Definition" navigates into a dependency (e.g. `.venv/`, `node_modules/`), the save dialog for new files defaulted to that directory - `most_recent_active_path` now checks the `read_only_files` setting and skips matching paths, falling back to the next eligible path, the worktree root, or the home directory ## Design tradeoffs We considered three approaches: 1. **Filter by `is_ignored`/`is_hidden`/`is_external` on worktree entries** — catches `.venv` when gitignored or when it's a dotfile, but also false-positives on directories like `.github/workflows/` that users intentionally edit. 2. **Use preview tab status** — "Go to Definition" opens files as preview tabs, so skipping preview paths targets the right intent. But it doesn't work when preview tabs are disabled, and the signal is transient (preview status changes as you interact with tabs). 3. **Use `read_only_files` setting** (this PR) — an explicit user declaration of "I never want to edit files here." If you can't edit them, you don't want to save new files next to them either. This is the clearest signal of intent and respects user configuration. The tradeoff is that `read_only_files` is empty by default, so users need to configure it. But the kind of user bothered by the save dialog defaulting to a dependency directory is the same kind of user who already configures `read_only_files` (see [#46827](https://github.com/zed-industries/zed/discussions/46827) for an example). ## Test plan - [x] Manual test: configured `read_only_files: ["**/.venv/**"]`, opened project, Go to Definition into `.venv`, created new file — save dialog defaults to project root - [x] Added `test_most_recent_active_path_skips_read_only_paths` - [x] All existing workspace tests pass Release Notes: - Fixed save dialog defaulting to dependency directories (e.g. `.venv/`, `node_modules/`) after using Go to Definition, when those directories are configured as `read_only_files`. --------- Co-authored-by: Lukas Wirth --- crates/workspace/src/item.rs | 11 +++++- crates/workspace/src/workspace.rs | 62 +++++++++++++++++++++++++++++-- 2 files changed, 68 insertions(+), 5 deletions(-) diff --git a/crates/workspace/src/item.rs b/crates/workspace/src/item.rs index c4b664e24803e2e022d1bfb7e744a5b8193a703a..ee74d7f5ebd8d5b8e7664a643bc12bcf1996fb85 100644 --- a/crates/workspace/src/item.rs +++ b/crates/workspace/src/item.rs @@ -1472,9 +1472,18 @@ pub mod test { impl TestProjectItem { pub fn new(id: u64, path: &str, cx: &mut App) -> Entity { + Self::new_in_worktree(id, path, WorktreeId::from_usize(0), cx) + } + + pub fn new_in_worktree( + id: u64, + path: &str, + worktree_id: WorktreeId, + cx: &mut App, + ) -> Entity { let entry_id = Some(ProjectEntryId::from_proto(id)); let project_path = Some(ProjectPath { - worktree_id: WorktreeId::from_usize(0), + worktree_id, path: rel_path(path).into(), }); cx.new(|_| Self { diff --git a/crates/workspace/src/workspace.rs b/crates/workspace/src/workspace.rs index 3ad9de443bf82c92c30d6df39cfbeb83e0b1727c..94b9a168c8b02d98aae009c51a277e34f06c40bd 100644 --- a/crates/workspace/src/workspace.rs +++ b/crates/workspace/src/workspace.rs @@ -3765,11 +3765,18 @@ impl Workspace { .project .read(cx) .worktree_for_id(path.worktree_id, cx)?; - if worktree.read(cx).is_visible() { - abs_path - } else { - None + if !worktree.read(cx).is_visible() { + return None; + } + let settings_location = SettingsLocation { + worktree_id: path.worktree_id, + path: &path.path, + }; + if WorktreeSettings::get(Some(settings_location), cx).is_path_read_only(&path.path) + { + return None; } + abs_path }) .next() } @@ -15156,4 +15163,51 @@ mod tests { ); }); } + + #[gpui::test] + async fn test_most_recent_active_path_skips_read_only_paths(cx: &mut TestAppContext) { + init_test(cx); + + let fs = FakeFs::new(cx.executor()); + fs.insert_tree( + path!("/project"), + json!({ + "src": { "main.py": "" }, + ".venv": { "lib": { "dep.py": "" } }, + }), + ) + .await; + + let project = Project::test(fs.clone(), [path!("/project").as_ref()], cx).await; + let (workspace, cx) = + cx.add_window_view(|window, cx| Workspace::test_new(project.clone(), window, cx)); + let worktree_id = project.update(cx, |project, cx| { + project.worktrees(cx).next().unwrap().read(cx).id() + }); + + // Configure .venv as read-only + workspace.update_in(cx, |_workspace, _window, cx| { + cx.update_global::(|store, cx| { + store + .set_user_settings(r#"{"read_only_files": ["**/.venv/**"]}"#, cx) + .ok(); + }); + }); + + let item_dep = cx.new(|cx| { + TestItem::new(cx).with_project_items(&[TestProjectItem::new_in_worktree( + 1001, + ".venv/lib/dep.py", + worktree_id, + cx, + )]) + }); + + // dep.py is active but matches read_only_files → should be skipped + workspace.update_in(cx, |workspace, window, cx| { + workspace.add_item_to_active_pane(Box::new(item_dep.clone()), None, true, window, cx); + }); + let path = workspace.read_with(cx, |workspace, cx| workspace.most_recent_active_path(cx)); + assert_eq!(path, None); + } }