diff --git a/e2e_tests/test_worktrees.py b/e2e_tests/test_worktrees.py index f008330..15f5136 100644 --- a/e2e_tests/test_worktrees.py +++ b/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) diff --git a/src/grm/main.rs b/src/grm/main.rs index d3748cc..09d142c 100644 --- a/src/grm/main.rs +++ b/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, ) { diff --git a/src/lib.rs b/src/lib.rs index d406b95..0c17e62 100644 --- a/src/lib.rs +++ b/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 diff --git a/src/repo.rs b/src/repo.rs index 206b1fe..494ae62 100644 --- a/src/repo.rs +++ b/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, ) -> 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 - && !branch_name.ends_with(&format!("{}{}", super::BRANCH_NAMESPACE_SEPARATOR, name)) - { + if branch_name != name { return Err(WorktreeRemoveFailureReason::Error(format!( - "Branch \"{}\" is checked out in worktree, this does not look correct", - &branch_name + "Branch \"{}\" is checked out in worktree \"{}\", this does not look correct", + &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 {}: {}", + ¤t_dir.display(), + error + )) + })? + .next() + .is_none() + { + if let Err(e) = std::fs::remove_dir_all(¤t_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) => {