Re: [PATCH v4 12/12] builtin/history: implement "split" subcommand

From: Karthik Nayak <karthik.188@gmail.com>
Date: Oct 14, 2025, 13:38 UTC
Message-ID: <CAOLa=ZTnsHxK2+rRsAErY_zc1Rg5WNrmcDBiPgDk38zb9TNfkQ@mail.gmail.com>
In-Reply-To: <20251001-b4-pks-history-builtin-v4-12-8e61ddb86317@pks.im>

Patrick Steinhardt <ps@pks.im> writes:

Show 61 quoted lines

> It is quite a common use case that one wants to split up one commit into
> multiple commits by moving parts of the changes of the original commit
> out into a separate commit. This is quite an involved operation though:
>
>   1. Identify the commit in question that is to be dropped.
>
>   2. Perform an interactive rebase on top of that commit's parent.
>
>   3. Modify the instruction sheet to "edit" the commit that is to be
>      split up.
>
>   4. Drop the commit via "git reset HEAD~".
>
>   5. Stage changes that should go into the first commit and commit it.
>
>   6. Stage changes that should go into the second commit and commit it.
>
>   7. Finalize the rebase.
>
> This is quite complex, and overall I would claim that most people who
> are not experts in Git would struggle with this flow.
>
> Introduce a new "split" subcommand for git-history(1) to make this way
> easier. All the user needs to do is to say `git history split $COMMIT`.
> From hereon, Git asks the user which parts of the commit shall be moved
> out into a separate commit and, once done, asks the user for the commit
> message. Git then creates that split-out commit and applies the original
> commit on top of it.
>
> Signed-off-by: Patrick Steinhardt <ps@pks.im>
> ---
>  Documentation/git-history.adoc |  62 ++++++
>  builtin/history.c              | 225 +++++++++++++++++++++
>  t/meson.build                  |   1 +
>  t/t3452-history-split.sh       | 432 +++++++++++++++++++++++++++++++++++++++++
>  4 files changed, 720 insertions(+)
>
> diff --git a/Documentation/git-history.adoc b/Documentation/git-history.adoc
> index b55babe206..83d675afea 100644
> --- a/Documentation/git-history.adoc
> +++ b/Documentation/git-history.adoc
> @@ -10,6 +10,7 @@ SYNOPSIS
>  [synopsis]
>  git history [<options>]
>  git history reword [<options>] <commit>
> +git history split [<options>] <commit> [--] [<pathspec>...]
>
>  DESCRIPTION
>  -----------
> @@ -40,6 +41,26 @@ rewrite history in different ways:
>  	provided, then this command will spawn an editor with the current
>  	message of that commit.
>
> +`split [--message=<message>] <commit> [--] [<pathspec>...]`::
> +	Interactively split up <commit> into two commits by choosing
> +	hunks introduced by it that will be moved into the new split-out
> +	commit. These hunks will then be written into a new commit that
> +	becomes the parent of the previous commit. The original commit
> +	stays intact, except that its parent will be the newly split-out
> +	commit.
>

So in essence we do this:

  Before split:
  P1 ── C0 ── C1 ── ... ── CN
        └─(target)         └─(HEAD)

  After split:
  P1 ── S0 ── C0' ── C1 ── ...... ── CN
        │     └─(modified original)  └─(HEAD)
        └─(split-out hunks)

I do wonder if S0 should contain the existing message and the new message should go to C0'. So perhaps more like

  After split:
  P1 ── C0' ── S0 ── C1 ── ..... ── CN
        │      └─(split-out hunks)  └─(HEAD)
        └─(modified original)

Mostly because when you say split, I would assume we keep the original as is and add on top of it. I don't really have a strong argument though :)

[snip]

Show 41 quoted lines

> +EXAMPLES
> +--------
> +
> +Split a commit
> +~~~~~~~~~~~~~~
> +
> +----------
> +$ git log --stat --oneline
> +3f81232 (HEAD -> main) original
> + bar | 1 +
> + foo | 1 +
> + 2 files changed, 2 insertions(+)
> +
> +$ git history split HEAD --message="split-out commit"
> +diff --git a/bar b/bar
> +new file mode 100644
> +index 0000000..5716ca5
> +--- /dev/null
> ++++ b/bar
> +@@ -0,0 +1 @@
> ++bar
> +(1/1) Stage addition [y,n,q,a,d,e,p,?]? y
> +
> +diff --git a/foo b/foo
> +new file mode 100644
> +index 0000000..257cc56
> +--- /dev/null
> ++++ b/foo
> +@@ -0,0 +1 @@
> ++foo
> +(1/1) Stage addition [y,n,q,a,d,e,p,?]? n
> +
> +$ git log --stat --oneline
> +7cebe64 (HEAD -> main) original
> + foo | 1 +
> + 1 file changed, 1 insertion(+)
> +d1582f3 split-out commit
> + bar | 1 +
> + 1 file changed, 1 insertion(+)
> +----------
> +

It's really nice to have examples.

[snip]

Show 10 quoted lines

> +
> +static int cmd_history_split(int argc,
> +			     const char **argv,
> +			     const char *prefix,
> +			     struct repository *repo)
> +{
> +	const char * const usage[] = {
> +		N_("git history split [<options>] <commit>"),
> +		NULL,
> +	};

should be '*const' here.

Show 30 quoted lines

> +	const char *commit_message = NULL;
> +	struct option options[] = {
> +		OPT_STRING('m', "message", &commit_message, N_("message"), N_("commit message")),
> +		OPT_END(),
> +	};
> +	struct oidmap rewritten_commits = OIDMAP_INIT;
> +	struct commit *original_commit, *parent, *head;
> +	struct strvec commits = STRVEC_INIT;
> +	struct commit_list *list = NULL;
> +	struct object_id split_commits[2];
> +	struct pathspec pathspec = { 0 };
> +	int ret;
> +
> +	argc = parse_options(argc, argv, prefix, options, usage, 0);
> +	if (argc < 1) {
> +		ret = error(_("command expects a revision"));
> +		goto out;
> +	}
> +	repo_config(repo, git_default_config, NULL);
> +
> +	original_commit = lookup_commit_reference_by_name(argv[0]);
> +	if (!original_commit) {
> +		ret = error(_("commit to be split cannot be found: %s"), argv[0]);
> +		goto out;
> +	}
> +
> +	if (original_commit->parents && original_commit->parents->next) {
> +		ret = error(_("commit to be split must not be a merge commit"));
> +		goto out;
> +	}

Do we need this? Since we also check for merges in `collect_commits()` below.

Show 16 quoted lines

> +	parent = original_commit->parents ? original_commit->parents->item : NULL;
> +	if (parent && repo_parse_commit(repo, parent)) {
> +		ret = error(_("unable to parse commit %s"),
> +			    oid_to_hex(&parent->object.oid));
> +		goto out;
> +	}
> +
> +	head = lookup_commit_reference_by_name("HEAD");
> +	if (!head) {
> +		ret = error(_("could not resolve HEAD to a commit"));
> +		goto out;
> +	}
> +
> +	commit_list_append(original_commit, &list);
> +	if (!repo_is_descendant_of(repo, original_commit, list)) {
> +		ret = error (_("split commit must be reachable from current HEAD commit"));

s/error /error/

> +		goto out;
> +	}
> +

This is also checked within collect_commits(), no?

Show 9 quoted lines

> +	parse_pathspec(&pathspec, 0,
> +		PATHSPEC_PREFER_FULL | PATHSPEC_SYMLINK_LEADING_PATH | PATHSPEC_PREFIX_ORIGIN,
> +		prefix, argv + 1);
> +
> +	/*
> +	* Collect the list of commits that we'll have to reapply now already.
> +	* This ensures that we'll abort early on in case the range of commits
> +	* contains merges, which we do not yet handle.
> +	*/

Comment spacing is off here.

Show 8 quoted lines

> +	ret = collect_commits(repo, parent, head, &commits);
> +	if (ret < 0)
> +		goto out;
> +
> +	/*
> +	 * Then we split up the commit and replace the original commit with the
> +	 * new new ones.
> +	 */

s/new/new

Show 5 quoted lines

> +	ret = split_commit(repo, original_commit, &pathspec,
> +			   commit_message, split_commits);
> +	if (ret < 0)
> +		goto out;
> +

This one was straight forward, it handles adding the hunks to the index creating the two commits and linking them.

> +	replace_commits(&commits, &original_commit->object.oid,
> +			split_commits, ARRAY_SIZE(split_commits));
> +

Nice. We use the function introduced earlier to replace the replace the commits with the new two commits.

Show 13 quoted lines

> +	ret = apply_commits(repo, &commits, parent, head, "split");
> +	if (ret < 0)
> +		goto out;
> +
> +	ret = 0;
> +
> +out:
> +	oidmap_clear(&rewritten_commits, 0);
> +	clear_pathspec(&pathspec);
> +	strvec_clear(&commits);
> +	free_commit_list(list);
> +	return ret;
> +}

[snip]

The tests look good. Nothing to add there.