On Tue, Jun 02, 2026 at 09:31:17AM +0200, Pablo Sabater wrote:

El mar, 2 jun 2026 a las 8:16, Patrick Steinhardt ([off-list ref]) escribió:

[snip]

quoted

+       original = lookup_commit_reference_by_name(argv[0]);
+       if (!original) {
+               ret = error(_("commit cannot be found: %s"), argv[0]);
+               goto out;
+       }
+
+       if (!original->parents) {
+               ret = error(_("cannot drop root commit %s: "
+                             "it has no parent to replay onto"),
+                           argv[0]);
+               goto out;
+       } else if (original->parents->next) {
+               ret = error(_("cannot drop merge commit"));

Why the if block adds which commit context, but not on the else if block?

True indeed, will adapt.

quoted

diff --git a/t/t3454-history-drop.sh b/t/t3454-history-drop.sh
new file mode 100755
index 0000000000..b320ff09b3
--- /dev/null
+++ b/t/t3454-history-drop.sh

@@ -0,0 +1,513 @@
+#!/bin/sh
+
+test_description='tests for git-history drop subcommand'
+
+. ./test-lib.sh
+. "$TEST_DIRECTORY/lib-log-graph.sh"
+
+expect_graph () {
+       cat >expect &&
+       lib_test_cmp_graph --graph --format=%s "$@"
+}

This function appears exactly the same at t6016 and t4215 but named as
check_graph. I was gonna do a cleanup for a commit series I'm working
on to bring that function to `lib-log-graph.sh` because all these test
files share that they import graph functions from `lib-log-graph.c`,
maybe you could do it?

I'd rather keep this series focussed, but I wouldn't mind a follow up
that deduplicates these call sites.

Also:

lib_test_cmp_graph () {
        git log --graph "$@" >output &&
        sed 's/ *$//' >output.sanitized <output &&
        test_cmp expect output.sanitized
}

Already uses `--graph` you can drop it from expect_graph()

True indeed, dropped the "--graph" argument.

Thanks!

Patrick