From: "Kristoffer Haugsbakk" <kristofferhaugsbakk@fastmail•com>
To: "Patrick Steinhardt" <ps@pks•im>, git@vger•kernel.org
Cc: "Pablo Sabater" <pabloosabaterr@gmail•com>,
"Junio C Hamano" <gitster@pobox•com>
Subject: Re: [PATCH v2 9/9] builtin/history: implement "drop" subcommand
Date: Wed, 03 Jun 2026 21:04:33 +0200 [thread overview]
Message-ID: <4b4672de-17cc-426f-8498-6384b1ad0d06@app.fastmail.com> (raw)
In-Reply-To: <20260603-b4-pks-history-drop-v2-9-742cb5b5176d@pks.im>
On Wed, Jun 3, 2026, at 18:14, Patrick Steinhardt wrote:
>[snip]
> ---
> Documentation/git-history.adoc | 38 ++-
> builtin/history.c | 187 +++++++++++++++
> t/meson.build | 1 +
> t/t3454-history-drop.sh | 513 +++++++++++++++++++++++++++++++++++++++++
> 4 files changed, 738 insertions(+), 1 deletion(-)
>
> diff --git a/Documentation/git-history.adoc
> b/Documentation/git-history.adoc
> index 2ba8121795..4eac732fd2 100644
> --- a/Documentation/git-history.adoc
> +++ b/Documentation/git-history.adoc
> @@ -8,6 +8,7 @@ git-history - EXPERIMENTAL: Rewrite history
> SYNOPSIS
> --------
> [synopsis]
> +git history drop <commit> [--dry-run] [--update-refs=(branches|head)]
> [--empty=(drop|keep|abort)]
> git history fixup <commit> [--dry-run] [--update-refs=(branches|head)]
> [--reedit-message] [--empty=(drop|keep|abort)]
> git history reword <commit> [--dry-run] [--update-refs=(branches|head)]
> git history split <commit> [--dry-run] [--update-refs=(branches|head)]
> [--] [<pathspec>...]
> @@ -51,13 +52,28 @@ be stateful operations. The limitation can be
> lifted once (if) Git learns about
> first-class conflicts.
>
> When using `fixup` with `--empty=drop`, dropping the root commit is not yet
> -supported.
> +supported. Likewise, `drop` cannot remove the root commit or a merge commit.
>
> COMMANDS
> --------
>
> The following commands are available to rewrite history in different ways:
>
> +`drop <commit>`::
> + Remove the specified commit from the history. All descendants of the
> + commit are replayed directly onto its parent.
> ++
> +The root commit cannot be dropped as that may lead to edge cases where refs
> +end up with no commits anymore. Merge commits cannot be dropped either; see
> +LIMITATIONS.
Should section names be “bare” or quoted like "LIMITATIONS"?
I don’t know.
Maybe add “above” since it’s a previous section.
> ++
> +If `HEAD` points at a commit that is to be rewritten, the index and working
>[snip]
> +Drop a commit
> +~~~~~~~~~~~~~
> +
> +----------
> +$ git log --oneline
> +abc1234 (HEAD -> main) third
> +def5678 second
> +ghi9012 first
> +
> +$ git history drop def5678
I know this is only the most simple example. And I might be dragging in
something beyond the scope of this example. But I recall one
demonstration on the first git-history(1) series which used a lot of
revision expressions and someone saying that they couldn’t imagine a
workflow where this would be more interactive than bringing up the
git-rebase(1) todo editor.
(I couldn’t find back to this right now.)
Although it is slower in terms of machine cycles, the keyboard instinct
for dropping a nearby commit might be to do `git rebase -i @~10`
(sufficiently high number) and navigating quickly in the configured
editor, deleting the line or using the keybind for `drop`. This example
which by implication brings up the log in order to paste the abbreviated
hash isn’t as ergonomic in comparison.
But using a revision expression like searching the subject with
`main^{/second}`, while not quicker probably, does distinguish itself
from git-rebase(1) by being a pretty fast ad hoc invocation that can be
done in one command without futzing with some weird sed(1) editor in
order to navigate to the `second` line and deleting it, or
something. And that’s a small win in isolation, but it segues much more
naturally into letting you script, say, dropping the last commit that
starts with the subject `TEMP`.
Or maybe revision expressions is too much in this context?
> +
> +$ git log --oneline
>[snip]
> diff --git a/t/t3454-history-drop.sh b/t/t3454-history-drop.sh
> new file mode 100755
> index 0000000000..37d8413e7e
> --- /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 --format=%s "$@"
> +}
> +
> +expect_log () {
> + git log --format="%s" "$@" >actual &&
> + cat >expect &&
> + test_cmp expect actual
> +}
> +
> +test_expect_success 'errors on missing commit argument' '
> + test_when_finished "rm -rf repo" &&
> + git init repo &&
> + (
> + cd repo &&
> + test_commit initial &&
> + test_must_fail git history drop 2>err &&
> + test_grep "command expects a single revision" err
Why not `test_cmp` since it’s a fixed error?
Same for a few other tests like `errors on unknown revision`.
> + )
> +'
>[snip]
> +test_expect_success 'errors with invalid --empty= value' '
> + test_when_finished "rm -rf repo" &&
> + git init repo &&
> + test_commit -C repo initial &&
> + test_commit -C repo second &&
> + test_must_fail git -C repo history drop --empty=bogus HEAD 2>err &&
> + test_grep "unrecognized.*--empty.*bogus" err
> +'
Style related I guess. Most tests here use a subshell but this one uses
`git -C`? Why is that?
>[snip]
> +test_expect_success 'updates branches on other lines of descent' '
> + test_when_finished "rm -rf repo" &&
> + git init repo &&
> + (
> + cd repo &&
> + test_commit base &&
> + test_commit target &&
> + git branch theirs &&
> + test_commit ours &&
> + git switch theirs &&
> + test_commit theirs &&
> +
> + expect_graph --branches <<-\EOF &&
> + * theirs
> + | * ours
> + |/
> + * target
> + * base
> + EOF
Oh, `expect_graph` is a cool tool.
> +
> + git history drop target &&
> +
> + expect_graph --branches <<-\EOF
> + * ours
> + | * theirs
> + |/
> + * base
> + EOF
> + )
> +'
>[snip]
next prev parent reply other threads:[~2026-06-03 19:04 UTC|newest]
Thread overview: 26+ messages / expand[flat|nested] mbox.gz Atom feed top
2026-06-01 15:36 [PATCH 0/2] builtin/history: introduce "drop" subcommand Patrick Steinhardt
2026-06-01 15:36 ` [PATCH 1/2] builtin/history: split handling of ref updates into two phases Patrick Steinhardt
2026-06-01 15:36 ` [PATCH 2/2] builtin/history: implement "drop" subcommand Patrick Steinhardt
2026-06-01 23:43 ` Junio C Hamano
2026-06-03 10:06 ` Patrick Steinhardt
2026-06-02 7:31 ` Pablo Sabater
2026-06-03 10:06 ` Patrick Steinhardt
2026-06-03 16:13 ` [PATCH v2 0/9] builtin/history: introduce " Patrick Steinhardt
2026-06-03 16:14 ` [PATCH v2 1/9] read-cache: split out function to drop unmerged entries to stage 0 Patrick Steinhardt
2026-06-03 16:14 ` [PATCH v2 2/9] reset: drop `USE_THE_REPOSITORY_VARIABLE` Patrick Steinhardt
2026-06-03 16:14 ` [PATCH v2 3/9] reset: modernize flags passed to `reset_head()` Patrick Steinhardt
2026-06-03 18:01 ` Kristoffer Haugsbakk
2026-06-03 16:14 ` [PATCH v2 4/9] reset: introduce dry-run mode Patrick Steinhardt
2026-06-03 18:18 ` Kristoffer Haugsbakk
2026-06-03 23:49 ` Junio C Hamano
2026-06-03 16:14 ` [PATCH v2 5/9] reset: introduce ability to skip reference updates Patrick Steinhardt
2026-06-03 23:51 ` Junio C Hamano
2026-06-04 9:01 ` Patrick Steinhardt
2026-06-03 16:14 ` [PATCH v2 6/9] reset: allow the caller to specify the current HEAD object Patrick Steinhardt
2026-06-03 16:14 ` [PATCH v2 7/9] reset: stop assuming that the caller passes in a clean index Patrick Steinhardt
2026-06-03 16:14 ` [PATCH v2 8/9] builtin/history: split handling of ref updates into two phases Patrick Steinhardt
2026-06-03 16:14 ` [PATCH v2 9/9] builtin/history: implement "drop" subcommand Patrick Steinhardt
2026-06-03 19:04 ` Kristoffer Haugsbakk [this message]
2026-06-04 9:02 ` Patrick Steinhardt
2026-06-03 23:58 ` Junio C Hamano
2026-06-04 9:02 ` Patrick Steinhardt
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=4b4672de-17cc-426f-8498-6384b1ad0d06@app.fastmail.com \
--to=kristofferhaugsbakk@fastmail$(echo .)com \
--cc=git@vger$(echo .)kernel.org \
--cc=gitster@pobox$(echo .)com \
--cc=pabloosabaterr@gmail$(echo .)com \
--cc=ps@pks$(echo .)im \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox