From: Phillip Wood <phillip.wood123@gmail.com>
Date: Wed, 10 Sep 2025 14:05:04 GMT
Subject: Re: [PATCH RFC v3 18/18] builtin/history: implement "reword" subcommand
Message-ID: <10696a16-3f2b-4844-9f9c-9815976b3e1e@gmail.com>
In-Reply-To: <20250904-b4-pks-history-builtin-v3-18-509053514755@pks.im>

Hi Patrick

On 04/09/2025 15:27, Patrick Steinhardt wrote:
> Implement a new "reword" subcommand for git-history(1). This subcommand
> is essentially the same as if a user performed an interactive rebase
> with a single commit changed to use the "reword" verb.

The sequencer already knows how to reword a commit, it would be much 
simpler to reuse that code.

Thanks

Phillip

> 
> Signed-off-by: Patrick Steinhardt <ps@pks.im>
> ---
>   Documentation/git-history.adoc |   5 +
>   builtin/history.c              | 104 +++++++++++++++++++++
>   t/meson.build                  |   1 +
>   t/t3454-history-reword.sh      | 202 +++++++++++++++++++++++++++++++++++++++++
>   4 files changed, 312 insertions(+)
> 
> diff --git a/Documentation/git-history.adoc b/Documentation/git-history.adoc
> index 6f0c64b90e..cbbcef3582 100644
> --- a/Documentation/git-history.adoc
> +++ b/Documentation/git-history.adoc
> @@ -13,6 +13,7 @@ git history continue
>   git history quit
>   git history drop <commit>
>   git history reorder <commit> (--before=<following-commit>|--after=<preceding-commit>)
> +git history reword [<options>] <commit>
>   git history split [<options>] <commit> [--] [<pathspec>...]
>   
>   DESCRIPTION
> @@ -53,6 +54,10 @@ child commits, as that would lead to an empty branch.
>   	be related to one another and must be reachable from the current `HEAD`
>   	commit.
>   
> +`reword <commit> [--message=<message>]`::
> +	Rewrite the commit message of the specified commit. All the other
> +	details of this commit remain unchanged.
> +
>   `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
> diff --git a/builtin/history.c b/builtin/history.c
> index df04b8dfc6..39acf4df28 100644
> --- a/builtin/history.c
> +++ b/builtin/history.c
> @@ -723,6 +723,108 @@ static int split_commit(struct repository *repo,
>   	return ret;
>   }
>   
> +static int cmd_history_reword(int argc,
> +			      const char **argv,
> +			      const char *prefix,
> +			      struct repository *repo)
> +{
> +	const char * const usage[] = {
> +		N_("git history reword [<options>] <commit>"),
> +		NULL,
> +	};
> +	const char *commit_message = NULL;
> +	struct option options[] = {
> +		OPT_STRING('m', "message", &commit_message, N_("message"), N_("commit message")),
> +		OPT_END(),
> +	};
> +	struct strbuf final_message = STRBUF_INIT;
> +	struct commit *original_commit, *head;
> +	struct strvec commits = STRVEC_INIT;
> +	struct object_id parent_tree_oid, original_commit_tree_oid;
> +	struct object_id rewritten_commit;
> +	const char *original_message, *original_body, *ptr;
> +	struct oidmap rewritten_commits = OIDMAP_INIT;
> +	struct replay_oid_mapping mapping = { 0 };
> +	char *original_author = NULL;
> +	size_t len;
> +	int ret;
> +
> +	argc = parse_options(argc, argv, prefix, options, usage, 0);
> +	if (argc != 1) {
> +		ret = error(_("command expects a single 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 reworded cannot be found: %s"), argv[0]);
> +		goto out;
> +	}
> +
> +	head = lookup_commit_reference_by_name("HEAD");
> +	if (!head) {
> +		ret = error(_("could not resolve HEAD to a commit"));
> +		goto out;
> +	}
> +
> +	/*
> +	 * 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.
> +	 */
> +	ret = collect_commits(repo, original_commit->parents ? original_commit->parents->item : NULL,
> +			      head, &commits);
> +	if (ret < 0)
> +		goto out;
> +
> +	/* We retain authorship of the original commit. */
> +	original_message = repo_logmsg_reencode(repo, original_commit, NULL, NULL);
> +	ptr = find_commit_header(original_message, "author", &len);
> +	if (ptr)
> +		original_author = xmemdupz(ptr, len);
> +	find_commit_subject(original_message, &original_body);
> +
> +	if (original_commit->parents)
> +		parent_tree_oid = *get_commit_tree_oid(original_commit->parents->item);
> +	else
> +		oidcpy(&parent_tree_oid, repo->hash_algo->empty_tree);
> +	original_commit_tree_oid = *get_commit_tree_oid(original_commit);
> +
> +	ret = fill_commit_message(repo, &parent_tree_oid, &original_commit_tree_oid,
> +				  original_body, commit_message, "reworded", &final_message);
> +	if (ret < 0)
> +		goto out;
> +
> +	ret = commit_tree(final_message.buf, final_message.len,
> +			  &repo_get_commit_tree(repo, original_commit)->object.oid,
> +			  original_commit->parents, &rewritten_commit, original_author, NULL);
> +	if (ret < 0) {
> +		ret = error(_("failed writing reworded commit"));
> +		goto out;
> +	}
> +
> +	replace_commits(&commits, &original_commit->object.oid, &rewritten_commit, 1);
> +
> +	mapping.entry.oid = rewritten_commit;
> +	mapping.rewritten_oid = original_commit->object.oid;
> +	oidmap_put(&rewritten_commits, &mapping);
> +
> +	ret = apply_commits(repo, &commits, head, original_commit,
> +			    &rewritten_commits, "reword");
> +	if (ret < 0)
> +		goto out;
> +
> +	ret = 0;
> +
> +out:
> +	oidmap_clear(&rewritten_commits, 0);
> +	strbuf_release(&final_message);
> +	strvec_clear(&commits);
> +	free(original_author);
> +	return ret;
> +}
> +
>   static int cmd_history_split(int argc,
>   			     const char **argv,
>   			     const char *prefix,
> @@ -835,6 +937,7 @@ int cmd_history(int argc,
>   		N_("git history quit"),
>   		N_("git history drop <commit>"),
>   		N_("git history reorder <commit> (--before=<following-commit>|--after=<preceding-commit>)"),
> +		N_("git history reword [<options>] <commit>"),
>   		N_("git history split [<options>] <commit> [--] [<pathspec>...]"),
>   		NULL,
>   	};
> @@ -845,6 +948,7 @@ int cmd_history(int argc,
>   		OPT_SUBCOMMAND("quit", &fn, cmd_history_quit),
>   		OPT_SUBCOMMAND("drop", &fn, cmd_history_drop),
>   		OPT_SUBCOMMAND("reorder", &fn, cmd_history_reorder),
> +		OPT_SUBCOMMAND("reword", &fn, cmd_history_reword),
>   		OPT_SUBCOMMAND("split", &fn, cmd_history_split),
>   		OPT_END(),
>   	};
> diff --git a/t/meson.build b/t/meson.build
> index b3d33c8588..948223f453 100644
> --- a/t/meson.build
> +++ b/t/meson.build
> @@ -380,6 +380,7 @@ integration_tests = [
>     't3451-history-drop.sh',
>     't3452-history-reorder.sh',
>     't3453-history-split.sh',
> +  't3454-history-reword.sh',
>     't3500-cherry.sh',
>     't3501-revert-cherry-pick.sh',
>     't3502-cherry-pick-merge.sh',
> diff --git a/t/t3454-history-reword.sh b/t/t3454-history-reword.sh
> new file mode 100755
> index 0000000000..97bdd755fa
> --- /dev/null
> +++ b/t/t3454-history-reword.sh
> @@ -0,0 +1,202 @@
> +#!/bin/sh
> +
> +test_description='tests for git-history reword subcommand'
> +
> +. ./test-lib.sh
> +
> +test_expect_success 'refuses to work with merge commits' '
> +	test_when_finished "rm -rf repo" &&
> +	git init repo &&
> +	(
> +		cd repo &&
> +		test_commit base &&
> +		git branch branch &&
> +		test_commit ours &&
> +		git switch branch &&
> +		test_commit theirs &&
> +		git switch - &&
> +		git merge theirs &&
> +		test_must_fail git history reword HEAD~ 2>err &&
> +		test_grep "cannot rearrange commit history with merges" err &&
> +		test_must_fail git history reword HEAD 2>err &&
> +		test_grep "cannot rearrange commit history with merges" err
> +	)
> +'
> +
> +test_expect_success 'refuses to work with changes in the worktree or index' '
> +	test_when_finished "rm -rf repo" &&
> +	git init repo &&
> +	(
> +		cd repo &&
> +		test_commit base file &&
> +		echo foo >file &&
> +		test_must_fail git history reword HEAD 2>err &&
> +		test_grep "Your local changes to the following files would be overwritten" err &&
> +		git add file &&
> +		test_must_fail git history reword HEAD 2>err &&
> +		test_grep "Your local changes to the following files would be overwritten" err
> +	)
> +'
> +
> +test_expect_success 'can reword tip of a branch' '
> +	test_when_finished "rm -rf repo" &&
> +	git init repo &&
> +	(
> +		cd repo &&
> +		test_commit first &&
> +		test_commit second &&
> +		test_commit third &&
> +
> +		git symbolic-ref HEAD >expect &&
> +		git history reword -m "third reworded" HEAD &&
> +		git symbolic-ref HEAD >actual &&
> +		test_cmp expect actual &&
> +
> +		cat >expect <<-EOF &&
> +		third reworded
> +		second
> +		first
> +		EOF
> +		git log --format=%s >actual &&
> +		test_cmp expect actual
> +	)
> +'
> +
> +test_expect_success 'can reword commit in the middle' '
> +	test_when_finished "rm -rf repo" &&
> +	git init repo &&
> +	(
> +		cd repo &&
> +		test_commit first &&
> +		test_commit second &&
> +		test_commit third &&
> +
> +		git symbolic-ref HEAD >expect &&
> +		git history reword -m "second reworded" HEAD~ &&
> +		git symbolic-ref HEAD >actual &&
> +		test_cmp expect actual &&
> +
> +		cat >expect <<-EOF &&
> +		third
> +		second reworded
> +		first
> +		EOF
> +		git log --format=%s >actual &&
> +		test_cmp expect actual
> +	)
> +'
> +
> +test_expect_success 'can reword root commit' '
> +	test_when_finished "rm -rf repo" &&
> +	git init repo &&
> +	(
> +		cd repo &&
> +		test_commit first &&
> +		test_commit second &&
> +		test_commit third &&
> +		git history reword -m "first reworded" HEAD~2 &&
> +
> +		cat >expect <<-EOF &&
> +		third
> +		second
> +		first reworded
> +		EOF
> +		git log --format=%s >actual &&
> +		test_cmp expect actual
> +	)
> +'
> +
> +test_expect_success 'can use editor to rewrite commit message' '
> +	test_when_finished "rm -rf repo" &&
> +	git init repo &&
> +	(
> +		cd repo &&
> +		test_commit first &&
> +
> +		write_script fake-editor.sh <<-\EOF &&
> +		cp "$1" . &&
> +		printf "\namend a comment\n" >>"$1"
> +		EOF
> +		test_set_editor "$(pwd)"/fake-editor.sh &&
> +		git history reword HEAD &&
> +
> +		cat >expect <<-EOF &&
> +		first
> +
> +		# Please enter the commit message for the reworded changes. Lines starting
> +		# with ${SQ}#${SQ} will be kept; you may remove them yourself if you want to.
> +		# Changes to be committed:
> +		#	new file:   first.t
> +		#
> +		EOF
> +		test_cmp expect COMMIT_EDITMSG &&
> +
> +		cat >expect <<-EOF &&
> +		first
> +
> +		amend a comment
> +
> +		EOF
> +		git log --format=%B >actual &&
> +		test_cmp expect actual
> +	)
> +'
> +
> +test_expect_success 'hooks are executed for rewritten commits' '
> +	test_when_finished "rm -rf repo" &&
> +	git init repo &&
> +	(
> +		cd repo &&
> +		test_commit first &&
> +		test_commit second &&
> +		test_commit third &&
> +
> +		write_script .git/hooks/prepare-commit-msg <<-EOF &&
> +		echo "prepare-commit-msg: \$@" >>"$(pwd)/hooks.log"
> +		EOF
> +		write_script .git/hooks/post-commit <<-EOF &&
> +		echo "post-commit" >>"$(pwd)/hooks.log"
> +		EOF
> +		write_script .git/hooks/post-rewrite <<-EOF &&
> +		{
> +			echo "post-rewrite: \$@"
> +			cat
> +		} >>"$(pwd)/hooks.log"
> +		EOF
> +
> +		git history reword -m "second reworded" HEAD~ &&
> +
> +		cat >expect <<-EOF &&
> +		third
> +		second reworded
> +		first
> +		EOF
> +		git log --format=%s >actual &&
> +		test_cmp expect actual &&
> +
> +		cat >expect <<-EOF &&
> +		prepare-commit-msg: .git/COMMIT_EDITMSG message
> +		post-commit
> +		prepare-commit-msg: .git/COMMIT_EDITMSG message
> +		post-commit
> +		post-rewrite: history
> +		$(git rev-parse second) $(git rev-parse HEAD~)
> +		$(git rev-parse third) $(git rev-parse HEAD)
> +		EOF
> +		test_cmp expect hooks.log
> +	)
> +'
> +
> +test_expect_success 'aborts with empty commit message' '
> +	test_when_finished "rm -rf repo" &&
> +	git init repo &&
> +	(
> +		cd repo &&
> +		test_commit first &&
> +
> +		test_must_fail git history reword -m "" HEAD 2>err &&
> +		test_grep "Aborting commit due to empty commit message." err
> +	)
> +'
> +
> +test_done
>