From: Phillip Wood <phillip.wood123@gmail.com>
Date: Tue, 09 Jun 2026 13:25:39 GMT
Subject: Re: [PATCH RFC v2 2/2] builtin/history: abort reword on same message
Message-ID: <54bd36e9-3d21-4f83-86d6-2882a14779de@gmail.com>
In-Reply-To: <20260609-ps-history-reword-v2-2-a0e6028ca9b4@gmail.com>

Hi Pablo

On 09/06/2026 11:42, Pablo Sabater wrote:
>   static int commit_tree_ext(struct repository *repo,
> @@ -135,6 +136,13 @@ static int commit_tree_ext(struct repository *repo,
>   					  original_body, action, &commit_message);
>   		if (ret < 0)
>   			goto out;
> +
> +		if (flags & COMMIT_TREE_ABORT_ON_SAME_MESSAGE &&
> +		    !strcmp(original_body, commit_message.buf)) {
> +			fprintf(stderr, _("Message unchanged, aborting reword.\n"));
> +			ret = 1;
> +			goto out;
> +		}

I wonder if we should check that the committer identity is unchanged as 
well in case anyone is using this to fix commits after committing with 
the wrong identity.

Aborting when the message and committer identity are unchanged seems 
like a good idea.

Thanks

Phillip

>   	} else {
>   		strbuf_addstr(&commit_message, original_body);
>   	}
> @@ -693,7 +701,8 @@ static int cmd_history_reword(int argc,
>   	struct strbuf reflog_msg = STRBUF_INIT;
>   	struct commit *original, *rewritten;
>   	struct rev_info revs = { 0 };
> -	enum commit_tree_flags flags = COMMIT_TREE_EDIT_MESSAGE;
> +	enum commit_tree_flags flags = COMMIT_TREE_EDIT_MESSAGE |
> +				       COMMIT_TREE_ABORT_ON_SAME_MESSAGE;
>   	int ret;
>   
>   	argc = parse_options(argc, argv, prefix, options, usage, 0);
> @@ -721,6 +730,9 @@ static int cmd_history_reword(int argc,
>   	if (ret < 0) {
>   		ret = error(_("failed writing reworded commit"));
>   		goto out;
> +	} else if (ret == 1) {
> +		ret = 0;
> +		goto out;
>   	}
>   
>   	strbuf_addf(&reflog_msg, "reword: updating %s", argv[0]);
> diff --git a/t/t3451-history-reword.sh b/t/t3451-history-reword.sh
> index de7b357685..6e0e278c42 100755
> --- a/t/t3451-history-reword.sh
> +++ b/t/t3451-history-reword.sh
> @@ -396,4 +396,20 @@ test_expect_success 'retains changes in the worktree and index' '
>   	)
>   '
>   
> +test_expect_success 'aborts if the commit message is the same' '
> +	test_when_finished "rm -rf repo" &&
> +	git init repo &&
> +	(
> +		cd repo &&
> +		test_commit first &&
> +		test_commit second &&
> +
> +		git rev-parse HEAD >oid-before &&
> +		GIT_EDITOR=true git history reword HEAD 2>err &&
> +		git rev-parse HEAD >oid-after &&
> +		test_cmp oid-before oid-after &&
> +		test_grep "Message unchanged" err
> +	)
> +'
> +
>   test_done
> diff --git a/t/t3453-history-fixup.sh b/t/t3453-history-fixup.sh
> index 868298e248..9f9a3c93de 100755
> --- a/t/t3453-history-fixup.sh
> +++ b/t/t3453-history-fixup.sh
> @@ -443,6 +443,28 @@ test_expect_success '--reedit-message opens editor for the commit message' '
>   	)
>   '
>   
> +test_expect_success 'fixup --reedit-message does not abort with the same commit message' '
> +	test_when_finished "rm -rf repo" &&
> +	git init repo &&
> +	(
> +		cd repo &&
> +		test_commit initial &&
> +		echo content > file.txt &&
> +		git add file.txt &&
> +		git commit -m "add file" &&
> +
> +		echo fix >>file.txt &&
> +		git add file.txt &&
> +		GIT_EDITOR=true git history fixup --reedit-message HEAD &&
> +		expect_changes --branches <<-\EOF
> +		add file
> +		2	0	file.txt
> +		initial
> +		1	0	initial.t
> +		EOF
> +	)
> +'
> +
>   test_expect_success 'retains unstaged working tree changes after fixup' '
>   	test_when_finished "rm -rf repo" &&
>   	git init repo &&
>