workspace: Skip read-only paths when choosing default save location (#53100)

Pedro Guedes and Lukas Wirth created 1 day ago

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 <[e-mail]>

Change summary

crates/workspace/src/item.rs      | 11 +++++
crates/workspace/src/workspace.rs | 62 ++++++++++++++++++++++++++++++--
2 files changed, 68 insertions(+), 5 deletions(-)

Detailed changes

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> {
+            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<Self> {
             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 {

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::<SettingsStore, _>(|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);
+    }
 }