Properly handle deletion of nested worktrees

e2e_tests/test_worktrees.py

@@ -589,6 +589,30 @@ def test_worktree_delete():
         assert "test" not in [str(b) for b in repo.branches]
 
 
+@pytest.mark.parametrize("has_other_worktree", [True, False])
+def test_worktree_delete_in_subfolder(has_other_worktree):
+    with TempGitRepositoryWorktree.get(funcname()) as (base_dir, _commit):
+        cmd = grm(["wt", "add", "dir/test", "--track", "origin/test"], cwd=base_dir)
+        assert cmd.returncode == 0
+        assert "dir" in os.listdir(base_dir)
+
+        if has_other_worktree is True:
+            cmd = grm(
+                ["wt", "add", "dir/test2", "--track", "origin/test"], cwd=base_dir
+            )
+            assert cmd.returncode == 0
+            assert {"test", "test2"} == set(os.listdir(os.path.join(base_dir, "dir")))
+        else:
+            assert {"test"} == set(os.listdir(os.path.join(base_dir, "dir")))
+
+        cmd = grm(["wt", "delete", "dir/test"], cwd=base_dir)
+        assert cmd.returncode == 0
+        if has_other_worktree is True:
+            assert {"test2"} == set(os.listdir(os.path.join(base_dir, "dir")))
+        else:
+            assert "dir" not in os.listdir(base_dir)
+
+
 def test_worktree_delete_refusal_no_tracking_branch():
     with TempGitRepositoryWorktree.get(funcname()) as (base_dir, _commit):
         cmd = grm(["wt", "add", "test"], cwd=base_dir)

src/grm/main.rs

@@ -528,8 +528,6 @@ fn main() {
                     }
                 }
                 cmd::WorktreeAction::Delete(action_args) => {
-                    let worktree_dir = cwd.join(&action_args.name);
-
                     let worktree_config = match repo::read_worktree_root_config(&cwd) {
                         Ok(config) => config,
                         Err(error) => {
@@ -547,8 +545,9 @@ fn main() {
                     });
 
                     match repo.remove_worktree(
+                        &cwd,
                         &action_args.name,
-                        &worktree_dir,
+                        Path::new(&action_args.name),
                         action_args.force,
                         &worktree_config,
                     ) {

src/lib.rs

@@ -14,8 +14,6 @@ pub mod table;
 pub mod tree;
 pub mod worktree;
 
-const BRANCH_NAMESPACE_SEPARATOR: &str = "/";
-
 /// Find all git repositories under root, recursively
 ///
 /// The bool in the return value specifies whether there is a repository

src/repo.rs

@@ -1153,18 +1153,21 @@ impl RepoHandle {
 
     pub fn remove_worktree(
         &self,
+        base_dir: &Path,
         name: &str,
         worktree_dir: &Path,
         force: bool,
         worktree_config: &Option<WorktreeRootConfig>,
     ) -> Result<(), WorktreeRemoveFailureReason> {
-        if !worktree_dir.exists() {
+        let fullpath = base_dir.join(worktree_dir);
+
+        if !fullpath.exists() {
             return Err(WorktreeRemoveFailureReason::Error(format!(
                 "{} does not exist",
                 name
             )));
         }
-        let worktree_repo = RepoHandle::open(worktree_dir, false).map_err(|error| {
+        let worktree_repo = RepoHandle::open(&fullpath, false).map_err(|error| {
             WorktreeRemoveFailureReason::Error(format!("Error opening repo: {}", error))
         })?;
 
@@ -1176,12 +1179,11 @@ impl RepoHandle {
             WorktreeRemoveFailureReason::Error(format!("Failed getting name of branch: {}", error))
         })?;
 
-        if branch_name != name
+        if branch_name != name {
-            && !branch_name.ends_with(&format!("{}{}", super::BRANCH_NAMESPACE_SEPARATOR, name))
-        {
             return Err(WorktreeRemoveFailureReason::Error(format!(
-                "Branch \"{}\" is checked out in worktree, this does not look correct",
+                "Branch \"{}\" is checked out in worktree \"{}\", this does not look correct",
-                &branch_name
+                &branch_name,
+                &worktree_dir.display(),
             )));
         }
 
@@ -1251,13 +1253,47 @@ impl RepoHandle {
             }
         }
 
-        if let Err(e) = std::fs::remove_dir_all(&worktree_dir) {
+        // worktree_dir is a relative path, starting from base_dir. We walk it
+        // upwards (from subdirectory to parent directories) and remove each
+        // component, in case it is empty. Only the leaf directory can be
+        // removed unconditionally (as it contains the worktree itself).
+        if let Err(e) = std::fs::remove_dir_all(&fullpath) {
             return Err(WorktreeRemoveFailureReason::Error(format!(
                 "Error deleting {}: {}",
                 &worktree_dir.display(),
                 e
             )));
         }
+
+        if let Some(current_dir) = worktree_dir.parent() {
+            for current_dir in current_dir.ancestors() {
+                let current_dir = base_dir.join(current_dir);
+                println!("deleting {}", current_dir.display());
+                if current_dir
+                    .read_dir()
+                    .map_err(|error| {
+                        WorktreeRemoveFailureReason::Error(format!(
+                            "Error reading {}: {}",
+                            &current_dir.display(),
+                            error
+                        ))
+                    })?
+                    .next()
+                    .is_none()
+                {
+                    if let Err(e) = std::fs::remove_dir_all(&current_dir) {
+                        return Err(WorktreeRemoveFailureReason::Error(format!(
+                            "Error deleting {}: {}",
+                            &worktree_dir.display(),
+                            e
+                        )));
+                    }
+                } else {
+                    break;
+                }
+            }
+        }
+
         self.prune_worktree(name)
             .map_err(WorktreeRemoveFailureReason::Error)?;
         branch
@@ -1310,7 +1346,13 @@ impl RepoHandle {
         {
             let repo_dir = &directory.join(&worktree.name());
             if repo_dir.exists() {
-                match self.remove_worktree(worktree.name(), repo_dir, false, &config) {
+                match self.remove_worktree(
+                    directory,
+                    worktree.name(),
+                    Path::new(worktree.name()),
+                    false,
+                    &config,
+                ) {
                     Ok(_) => print_success(&format!("Worktree {} deleted", &worktree.name())),
                     Err(error) => match error {
                         WorktreeRemoveFailureReason::Changes(changes) => {