Repository: VirtusLab/git-machete
Pull request #1313 (2024-08-26 19:32:50)

Use the same algorithm to recognize merged branches in `delete-unmanaged` as for `status` and `traverse`


Patch:
diff --git a/RELEASE_NOTES.md b/RELEASE_NOTES.md
index 059c43e..63d5c18 100644
--- a/RELEASE_NOTES.md
+++ b/RELEASE_NOTES.md
@@ -2,6 +2,8 @@
 
 ## New in git-machete 3.29.1
 
+- improved: `git-machete delete-unmanaged` uses the same algorithm as `status` and `traverse` to recognize merged branches (suggested by @cinnamennen)
+
 ## New in git-machete 3.29.0
 
 - added: git config keys `machete.github.prDescriptionIntroStyle` and `machete.gitlab.mrDescriptionIntroStyle`
diff --git a/docs/man/git-machete.1 b/docs/man/git-machete.1
index 76b0274..de0d9cd 100644
--- a/docs/man/git-machete.1
+++ b/docs/man/git-machete.1
@@ -27,7 +27,7 @@ level margin: \\n[rst2man-indent\\n[rst2man-indent-level]]
 .\" new: \\n[rst2man-indent\\n[rst2man-indent-level]]
 .in \\n[rst2man-indent\\n[rst2man-indent-level]]u
 ..
-.TH "GIT-MACHETE" "1" "Aug 26, 2024" "" "git-machete"
+.TH "GIT-MACHETE" "1" "Aug 28, 2024" "" "git-machete"
 .SH NAME
 git-machete \- git-machete 3.29.1
 .sp
diff --git a/git_machete/cli.py b/git_machete/cli.py
index 538ea84..940f366 100644
--- a/git_machete/cli.py
+++ b/git_machete/cli.py
@@ -671,11 +671,13 @@ def launch(orig_args: List[str]) -> None:
             machete_client.read_branch_layout_file(perform_interactive_slide_out=should_perform_interactive_slide_out)
             if 'checkout_my_github_prs' in parsed_cli:
                 machete_client.checkout_pull_requests(GitHubClient.spec(), pr_numbers=[], mine=True)
-            machete_client.delete_unmanaged(opt_yes=cli_opts.opt_yes)
+            machete_client.delete_unmanaged(opt_squash_merge_detection=SquashMergeDetection.NONE, opt_yes=cli_opts.opt_yes)
             machete_client.delete_untracked(opt_yes=cli_opts.opt_yes)
         elif cmd == "delete-unmanaged":
             machete_client.read_branch_layout_file(perform_interactive_slide_out=should_perform_interactive_slide_out)
-            machete_client.delete_unmanaged(opt_yes=cli_opts.opt_yes)
+            opt_squash_merge_detection = SquashMergeDetection.from_string(
+                cli_opts.opt_squash_merge_detection_string, cli_opts.opt_squash_merge_detection_origin)
+            machete_client.delete_unmanaged(opt_squash_merge_detection=opt_squash_merge_detection, opt_yes=cli_opts.opt_yes)
         elif cmd in {"diff", alias_by_command["diff"]}:
             machete_client.read_branch_layout_file(perform_interactive_slide_out=should_perform_interactive_slide_out)
             diff_branch = get_local_branch_short_name_from_arg(cli_opts.opt_branch) if (cli_opts.opt_branch is not None) else None
@@ -778,7 +780,7 @@ def launch(orig_args: List[str]) -> None:
                 machete_client.retarget_pr(config, head=branch, ignore_if_missing=ignore_if_missing)
             elif subcommand == "sync":  # GitHub only
                 machete_client.checkout_pull_requests(config, pr_numbers=[], mine=True)
-                machete_client.delete_unmanaged(opt_yes=False)
+                machete_client.delete_unmanaged(opt_squash_merge_detection=SquashMergeDetection.NONE, opt_yes=False)
                 machete_client.delete_untracked(opt_yes=cli_opts.opt_yes)
             else:  # an unknown subcommand is handled by argparse
                 raise UnexpectedMacheteException(f"Unknown subcommand: `{subcommand}`")
diff --git a/git_machete/client.py b/git_machete/client.py
index 4299f49..35daee7 100644
--- a/git_machete/client.py
+++ b/git_machete/client.py
@@ -673,7 +673,8 @@ class MacheteClient:
                     opt_no_interactive_rebase)
 
         if opt_delete:
-            self.__delete_branches(branches_to_delete=branches_to_slide_out, opt_yes=False)
+            self.__delete_branches(branches_to_delete=branches_to_slide_out,
+                                   opt_squash_merge_detection=SquashMergeDetection.NONE, opt_yes=False)
 
     def advance(self, *, branch: LocalBranchShortName, opt_yes: bool) -> None:
         down_branches = self.__down_branches.get(branch)
@@ -1283,46 +1284,47 @@ class MacheteClient:
         else:
             self.__git.rebase(onto, from_exclusive, branch, opt_no_interactive_rebase, extra_rebase_opts)
 
-    def delete_unmanaged(self, *, opt_yes: bool) -> None:
+    def delete_unmanaged(self, *, opt_squash_merge_detection: SquashMergeDetection, opt_yes: bool) -> None:
         print('Checking for unmanaged branches...')
-        branches_to_delete = excluding(self.__git.get_local_branches(), self.managed_branches)
-        self.__delete_branches(branches_to_delete=branches_to_delete, opt_yes=opt_yes)
+        branches_to_delete = sorted(excluding(self.__git.get_local_branches(), self.managed_branches))
+        self.__delete_branches(branches_to_delete=branches_to_delete,
+                               opt_squash_merge_detection=opt_squash_merge_detection, opt_yes=opt_yes)
 
-    def __delete_branches(self, branches_to_delete: List[LocalBranchShortName], opt_yes: bool) -> None:
+    def __delete_branches(self, branches_to_delete: List[LocalBranchShortName],
+                          opt_squash_merge_detection: SquashMergeDetection, opt_yes: bool) -> None:
         current_branch = self.__git.get_current_branch_or_none()
         if current_branch and current_branch in branches_to_delete:
             branches_to_delete = excluding(branches_to_delete, [current_branch])
             print(f"Skipping current branch {bold(current_branch)}")
-        if branches_to_delete:
-            branches_merged_to_head = self.__git.get_merged_local_branches()
-
-            branches_to_delete_merged_to_head = [branch for branch in branches_to_delete if branch in branches_merged_to_head]
-            for branch in branches_to_delete_merged_to_head:
-                remote_branch = self.__git.get_strict_counterpart_for_fetching_of_branch(branch)
-                is_merged_to_remote = self.__git.is_ancestor_or_equal(branch.full_name(),
-                                                                      remote_branch.full_name()) if remote_branch else True
-                msg_core_suffix = '' if is_merged_to_remote else f', but not merged to {bold(remote_branch)}'  # type: ignore[arg-type]
-                msg_core = f"{bold(branch)} (merged to HEAD{msg_core_suffix})"
-                msg = f"Delete branch {msg_core}?" + get_pretty_choices('y', 'N', 'q')
-                opt_yes_msg = f"Deleting branch {msg_core}..."
-                ans = self.ask_if(msg, opt_yes_msg, opt_yes=opt_yes)
-                if ans in ('y', 'yes'):
-                    self.__git.delete_branch(branch, force=is_merged_to_remote)
-                elif ans in ('q', 'quit'):
-                    return
+        if not branches_to_delete:
+            print("No branches to delete")
+            return
 
-            branches_to_delete_unmerged_to_head = [branch for branch in branches_to_delete if branch not in branches_merged_to_head]
-            for branch in branches_to_delete_unmerged_to_head:
-                msg_core = f"{bold(branch)} (unmerged to HEAD)"
+        if opt_yes:
+            for branch in branches_to_delete:
+                print(f"Deleting branch {bold(branch)}...")
+                self.__git.delete_branch(branch, force=True)
+        else:
+            for branch in branches_to_delete:
+                if self.is_merged_to(branch.full_name(), AnyBranchName('HEAD'), opt_squash_merge_detection):
+                    remote_branch = self.__git.get_strict_counterpart_for_fetching_of_branch(branch)
+                    if remote_branch:
+                        is_merged_to_remote = self.is_merged_to(branch.full_name(), remote_branch.full_name(), opt_squash_merge_detection)
+                    else:
+                        is_merged_to_remote = True
+                    if is_merged_to_remote:
+                        msg_core_suffix = ''
+                    else:
+                        msg_core_suffix = f', but not merged to {bold(str(remote_branch))}'
+                    msg_core = f"{bold(branch)} (merged to HEAD{msg_core_suffix})"
+                else:
+                    msg_core = f"{bold(branch)} (unmerged to HEAD)"
                 msg = f"Delete branch {msg_core}?" + get_pretty_choices('y', 'N', 'q')
-                opt_yes_msg = f"Deleting branch {msg_core}..."
-                ans = self.ask_if(msg, opt_yes_msg, opt_yes=opt_yes)
+                ans = self.ask_if(msg, opt_yes_msg=None, opt_yes=False)
                 if ans in ('y', 'yes'):
                     self.__git.delete_branch(branch, force=True)
                 elif ans in ('q', 'quit'):
                     return
-        else:
-            print("No branches to delete")
 
     def edit(self) -> int:
         default_editor_with_args: List[str] = self.__get_editor_with_args()
@@ -2087,7 +2089,7 @@ class MacheteClient:
 
     def is_merged_to(
             self,
-            branch: LocalBranchShortName,
+            branch: AnyBranchName,
             upstream: AnyBranchName,
             opt_squash_merge_detection: SquashMergeDetection
     ) -> bool:
@@ -3063,7 +3065,7 @@ class MacheteClient:
                 branches_to_delete.append(branch)
 
         self.__remove_branches_from_layout(branches_to_delete)
-        self.__delete_branches(branches_to_delete, opt_yes)
+        self.__delete_branches(branches_to_delete, opt_squash_merge_detection=SquashMergeDetection.NONE, opt_yes=opt_yes)
 
     def slide_out_removed_from_remote(self, opt_delete: bool) -> None:
         slid_out_branches: List[LocalBranchShortName] = []
@@ -3074,7 +3076,7 @@ class MacheteClient:
 
         self.__remove_branches_from_layout(slid_out_branches)
         if opt_delete:
-            self.__delete_branches(slid_out_branches, opt_yes=True)
+            self.__delete_branches(slid_out_branches, opt_squash_merge_detection=SquashMergeDetection.NONE, opt_yes=True)
 
     def __remove_branches_from_layout(self, branches_to_delete: List[LocalBranchShortName]) -> None:
         for branch in branches_to_delete:
diff --git a/git_machete/git_operations.py b/git_machete/git_operations.py
index 29cc057..1fad397 100644
--- a/git_machete/git_operations.py
+++ b/git_machete/git_operations.py
@@ -802,8 +802,6 @@ class GitContext:
             self.__merge_base_cached[hash1, hash2] = FullCommitHash.of(merge_base) if merge_base else None
         return self.__merge_base_cached[hash1, hash2]
 
-    # Note: the 'git rev-parse --verify' validation is not performed in case for either of earlier/later
-    # if the corresponding prefix is empty AND the revision is a 40 hex digit hash.
     def is_ancestor_or_equal(self, earlier_revision: AnyRevision, later_revision: AnyRevision) -> bool:
         earlier_hash = self.get_commit_hash_by_revision(earlier_revision)
         later_hash = self.get_commit_hash_by_revision(later_revision)
@@ -934,19 +932,6 @@ class GitContext:
         matching_remotes = [remote for remote in self.get_remotes() if (remote + "/" + branch) in remote_branches]
         return RemoteBranchShortName(matching_remotes[0] + "/" + branch) if len(matching_remotes) == 1 else None
 
-    def get_merged_local_branches(self) -> List[LocalBranchShortName]:
-        if self.get_git_version() >= (2, 7, 6):  # earliest version of git to support 'for-each-ref --merged'
-            return list(
-                map(lambda branch: LocalBranchFullName.of(branch).to_short_name(),
-                    utils.get_non_empty_lines(
-                        self._popen_git("for-each-ref", "--format=%(refname)", "--merged", "HEAD", "refs/heads").stdout)))
-        else:
-            return list(
-                filter(lambda branch: self.is_ancestor_or_equal(branch, AnyRevision.of('HEAD')),
-                       map(lambda branch: LocalBranchFullName.of(branch).to_short_name(),
-                           utils.get_non_empty_lines(
-                               self._popen_git("for-each-ref", "--format=%(refname)", "refs/heads").stdout))))
-
     def get_hook_path(self, hook_name: str) -> str:
         hook_dir: str = self.get_config_attr_or_none("core.hooksPath") or self.get_main_git_subpath("hooks")
         return os.path.join(hook_dir, hook_name)


Test patch:
diff --git a/tests/test_delete_unmanaged.py b/tests/test_delete_unmanaged.py
index ed8756f..5f9875a 100644
--- a/tests/test_delete_unmanaged.py
+++ b/tests/test_delete_unmanaged.py
@@ -14,11 +14,20 @@ class TestDeleteUnmanaged(BaseTest):
         with fixed_author_and_committer_date_in_past():
             (
                 self.repo_sandbox.new_branch("master")
-                    .commit("master commit.")
+                    .commit()
                     .new_branch("develop")
-                    .commit("develop commit.")
+                    .commit()
+                    .push()
+                    .amend_commit("Different commit message")
+                    .add_file_and_commit()
+                    .new_branch("refactor")
+                    .commit()
                     .new_branch("feature")
-                    .commit("feature commit.")
+                    .commit()
+                    .check_out("develop")
+                    .new_branch("bugfix")
+                    .commit()
+                    .check_out("feature")
             )
         body: str = \
             """
@@ -28,16 +37,20 @@ class TestDeleteUnmanaged(BaseTest):
 
         self.patch_symbol(mocker, "builtins.input", mock_input_returning("q"))
         launch_command("delete-unmanaged", "-v")
-        self.patch_symbol(mocker, "builtins.input", mock_input_returning("n"))
+        self.patch_symbol(mocker, "builtins.input", mock_input_returning("n", "n", "n"))
         launch_command("delete-unmanaged")
-        self.patch_symbol(mocker, "builtins.input", mock_input_returning("y"))
+        self.patch_symbol(mocker, "builtins.input", mock_input_returning("y", "y", "y"))
         assert_success(
             ["delete-unmanaged"],
             """
             Checking for unmanaged branches...
             Skipping current branch feature
-            Delete branch develop (merged to HEAD)? (y, N, q)
-            Deleted branch develop (was 03e727b).
+            Delete branch bugfix (unmerged to HEAD)? (y, N, q)
+            Deleted branch bugfix (was 4e4943a).
+            Delete branch develop (merged to HEAD, but not merged to origin/develop)? (y, N, q)
+            Deleted branch develop (was 02df5cb).
+            Delete branch refactor (merged to HEAD)? (y, N, q)
+            Deleted branch refactor (was 73f2ac5).
             """
         )
 
@@ -50,8 +63,8 @@ class TestDeleteUnmanaged(BaseTest):
             ["delete-unmanaged", "--yes"],
             """
             Checking for unmanaged branches...
-            Deleting branch feature (unmerged to HEAD)...
-            Deleted branch feature (was 87e00e9).
+            Deleting branch feature...
+            Deleted branch feature (was a18c571).
             """
         )
 
@@ -67,3 +80,32 @@ class TestDeleteUnmanaged(BaseTest):
         self.patch_symbol(mocker, "builtins.input", mock_input_returning("n"))
         launch_command("delete-unmanaged")
         assert self.repo_sandbox.get_local_branches() == ["foo", "master"]
+
+    def test_delete_unmanaged_for_squash_merged_branch(self, mocker: MockerFixture) -> None:
+        (
+            self.repo_sandbox
+            .new_branch("master")
+            .remove_remote("origin")
+            .commit("master first commit")
+            .new_branch("feature")
+            .commit("feature commit")
+            .check_out("master")
+            .commit("extra commit")
+            .execute("git merge --squash feature")
+            .execute("git commit -m squashed")
+        )
+
+        rewrite_branch_layout_file("master")
+
+        # Here the simple method will not detect the squash merge, as there are commits in master before we merged feature so
+        # there's no tree hash in master that matches the tree hash of feature
+        self.patch_symbol(mocker, "builtins.input", mock_input_returning("n"))
+        assert_success(['delete-unmanaged'],
+                       "Checking for unmanaged branches...\n"
+                       "Delete branch feature (unmerged to HEAD)? (y, N, q)\n")
+
+        self.repo_sandbox.set_git_config_key('machete.squashMergeDetection', 'exact')
+        self.patch_symbol(mocker, "builtins.input", mock_input_returning("n"))
+        assert_success(['delete-unmanaged'],
+                       "Checking for unmanaged branches...\n"
+                       "Delete branch feature (merged to HEAD)? (y, N, q)\n")
diff --git a/tests/test_slide_out.py b/tests/test_slide_out.py
index 50ee45f..480b5ff 100644
--- a/tests/test_slide_out.py
+++ b/tests/test_slide_out.py
@@ -461,7 +461,7 @@ class TestSlideOut(BaseTest):
 
         assert_success(
             ['slide-out', '--removed-from-remote', '--delete'],
-            "Sliding out should_be_pruned\nDeleting branch should_be_pruned (unmerged to HEAD)...\n")
+            "Sliding out should_be_pruned\nDeleting branch should_be_pruned...\n")
 
         assert read_branch_layout_file() == "main\n    unpushed\n    not_deleted_remotely\n    has_downstream\n        downstream\n"