* [PATCH RFC 0/2] builtin/history: change git history reword behavior and feedback
@ 2026-06-07 20:07 Pablo Sabater
2026-06-07 20:07 ` [PATCH RFC 1/2] builtin/history: abort reword on unchanged message Pablo Sabater
` (2 more replies)
0 siblings, 3 replies; 29+ messages in thread
From: Pablo Sabater @ 2026-06-07 20:07 UTC (permalink / raw)
To: git; +Cc: Patrick Steinhardt, Kaartic Sivaraam, Pablo Sabater
This small series contains two commits that aim to improve
`git history reword`:
1. Abort the reword when the original message and the new message are
the same to avoid unnecessary history changes.
2. Print feedback after a successful reword so the user knows about it.
`git commit --amend` and `git rebase -i` with reword don't abort if
the commit message is the same as the original and they update as if
it was a new message in favor of changing this behavior for
`git history reword`:
- As noted in the `git history` documentation, the command by
default updates all branches that contain the original commit [1]
this makes `git history reword` more expensive than other options
like `git rebase -i` that only updates the current branch.
- `git history` works in-memory without touching the worktree or index
[2], because it doesn't use the sequencer and `git history reword`
doesn't care about the staged files only about the commit message, it
should have no problems.
About the last fact in favor of 1, I'm not completely sure if it's
because of staged files that's the reason why `git commit --amend` or
`git rebase -i` with reword still updates even if the commit message
is the same one. I'm not very up to sequencer.c to be sure but maybe
there's a historical reason about it that someone knows. Anyways I
believe that given this new command is a good idea to discuss it.
The commit message of 1 mentions staged files as a possible justification
for why --amend and rebase behave this way, but that's just an
assumption that I'll be happy to change if I'm wrong.
[1]: https://git-scm.com/docs/git-history#_description
[2]: https://lore.kernel.org/git/20260113-b4-pks-history-builtin-v11-8-e74ebfa2652d@pks.im/
To: git@vger.kernel.org
Cc: Patrick Steinhardt <ps@pks.im>
Cc: Kaartic Sivaraam <kaartic.sivaraam@gmail.com>
Signed-off-by: Pablo Sabater <pabloosabaterr@gmail.com>
---
Pablo Sabater (2):
builtin/history: abort reword on unchanged message
builtin/history: print feedback after successful reword
builtin/history.c | 14 ++++++++++++++
t/t3451-history-reword.sh | 34 ++++++++++++++++++++++++++++++++++
2 files changed, 48 insertions(+)
---
base-commit: 9ac3f193c05c2237e2b14ebaa1149e9fc8a1abe0
change-id: 20260607-ps-history-reword-fcb70eaa4aa9
Best regards,
--
Pablo Sabater <pabloosabaterr@gmail.com>
^ permalink raw reply [flat|nested] 29+ messages in thread* [PATCH RFC 1/2] builtin/history: abort reword on unchanged message 2026-06-07 20:07 [PATCH RFC 0/2] builtin/history: change git history reword behavior and feedback Pablo Sabater @ 2026-06-07 20:07 ` Pablo Sabater 2026-06-08 9:30 ` Patrick Steinhardt ` (2 more replies) 2026-06-07 20:07 ` [PATCH RFC 2/2] builtin/history: print feedback after successful reword Pablo Sabater 2026-06-09 10:42 ` [PATCH RFC v2 0/2] builtin/history: abort reword on same message Pablo Sabater 2 siblings, 3 replies; 29+ messages in thread From: Pablo Sabater @ 2026-06-07 20:07 UTC (permalink / raw) To: git; +Cc: Patrick Steinhardt, Kaartic Sivaraam, Pablo Sabater When using `git history reword` if the new message is the same as the original it continues anyway creating a new commit with the same message and updates its descendants, modifying the history after this 'reworded' commit even though there was no actual change. `git commit --amend` and `git rebase -i` + reword share this behavior, however `git history reword` is different: 1. Works in-memory without touching the index or the worktree [1], so there are no side effects like staged files that could justify rewriting the history when the commit message is the same. 2. `git history` by default updates all the branches [2] that contain the original commit making it more costly than `git rebase -i` that only updates the current branch. Add a check if the original commit message is the same as the new one and abort if so. [1]: https://lore.kernel.org/git/20260113-b4-pks-history-builtin-v11-8-e74ebfa2652d@pks.im/ [2]: https://git-scm.com/docs/git-history#_description Signed-off-by: Pablo Sabater <pabloosabaterr@gmail.com> --- builtin/history.c | 10 ++++++++++ t/t3451-history-reword.sh | 20 ++++++++++++++++++++ 2 files changed, 30 insertions(+) diff --git a/builtin/history.c b/builtin/history.c index 0fc06fb204..51a22a9a1c 100644 --- a/builtin/history.c +++ b/builtin/history.c @@ -135,6 +135,13 @@ static int commit_tree_ext(struct repository *repo, original_body, action, &commit_message); if (ret < 0) goto out; + + if (!strcmp(original_body, commit_message.buf)) { + fprintf(stderr, _("Message unchanged," + " aborting reword.\n")); + ret = 1; + goto out; + } } else { strbuf_addstr(&commit_message, original_body); } @@ -718,6 +725,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..54ea8a7207 100755 --- a/t/t3451-history-reword.sh +++ b/t/t3451-history-reword.sh @@ -396,4 +396,24 @@ 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 && + write_script fake-editor.sh <<-\EOF && + true + EOF + test_set_editor "$(pwd)"/fake-editor.sh && + 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 -- 2.54.0 ^ permalink raw reply related [flat|nested] 29+ messages in thread
* Re: [PATCH RFC 1/2] builtin/history: abort reword on unchanged message 2026-06-07 20:07 ` [PATCH RFC 1/2] builtin/history: abort reword on unchanged message Pablo Sabater @ 2026-06-08 9:30 ` Patrick Steinhardt 2026-06-08 10:52 ` Pablo Sabater 2026-06-08 12:16 ` Junio C Hamano 2026-06-08 16:37 ` Ben Knoble 2 siblings, 1 reply; 29+ messages in thread From: Patrick Steinhardt @ 2026-06-08 9:30 UTC (permalink / raw) To: Pablo Sabater; +Cc: git, Kaartic Sivaraam On Sun, Jun 07, 2026 at 10:07:20PM +0200, Pablo Sabater wrote: > When using `git history reword` if the new message is the same as the > original it continues anyway creating a new commit with the same > message and updates its descendants, modifying the history after this > 'reworded' commit even though there was no actual change. > > `git commit --amend` and `git rebase -i` + reword share this behavior, > however `git history reword` is different: > 1. Works in-memory without touching the index or the worktree [1], so > there are no side effects like staged files that could justify > rewriting the history when the commit message is the same. > 2. `git history` by default updates all the branches [2] that contain the > original commit making it more costly than `git rebase -i` that only > updates the current branch. > > Add a check if the original commit message is the same as the new one > and abort if so. > > [1]: https://lore.kernel.org/git/20260113-b4-pks-history-builtin-v11-8-e74ebfa2652d@pks.im/ > [2]: https://git-scm.com/docs/git-history#_description Nit: I feel like both of the links don't really add much value. > Signed-off-by: Pablo Sabater <pabloosabaterr@gmail.com> > --- > builtin/history.c | 10 ++++++++++ > t/t3451-history-reword.sh | 20 ++++++++++++++++++++ > 2 files changed, 30 insertions(+) > > diff --git a/builtin/history.c b/builtin/history.c > index 0fc06fb204..51a22a9a1c 100644 > --- a/builtin/history.c > +++ b/builtin/history.c > @@ -135,6 +135,13 @@ static int commit_tree_ext(struct repository *repo, > original_body, action, &commit_message); > if (ret < 0) > goto out; > + > + if (!strcmp(original_body, commit_message.buf)) { > + fprintf(stderr, _("Message unchanged," > + " aborting reword.\n")); > + ret = 1; > + goto out; > + } > } else { > strbuf_addstr(&commit_message, original_body); > } We also execute this logic via "git history fixup --reedit-message", and here it wouldn't make sense to abort the commit in case the message is unchanged. Patrick ^ permalink raw reply [flat|nested] 29+ messages in thread
* Re: [PATCH RFC 1/2] builtin/history: abort reword on unchanged message 2026-06-08 9:30 ` Patrick Steinhardt @ 2026-06-08 10:52 ` Pablo Sabater 0 siblings, 0 replies; 29+ messages in thread From: Pablo Sabater @ 2026-06-08 10:52 UTC (permalink / raw) To: Patrick Steinhardt; +Cc: git, Kaartic Sivaraam El lun, 8 jun 2026 a las 11:30, Patrick Steinhardt (<ps@pks.im>) escribió: > > On Sun, Jun 07, 2026 at 10:07:20PM +0200, Pablo Sabater wrote: > > When using `git history reword` if the new message is the same as the > > original it continues anyway creating a new commit with the same > > message and updates its descendants, modifying the history after this > > 'reworded' commit even though there was no actual change. > > > > `git commit --amend` and `git rebase -i` + reword share this behavior, > > however `git history reword` is different: > > 1. Works in-memory without touching the index or the worktree [1], so > > there are no side effects like staged files that could justify > > rewriting the history when the commit message is the same. > > 2. `git history` by default updates all the branches [2] that contain the > > original commit making it more costly than `git rebase -i` that only > > updates the current branch. > > > > Add a check if the original commit message is the same as the new one > > and abort if so. > > > > [1]: https://lore.kernel.org/git/20260113-b4-pks-history-builtin-v11-8-e74ebfa2652d@pks.im/ > > [2]: https://git-scm.com/docs/git-history#_description > > Nit: I feel like both of the links don't really add much value. I'll just drop em. > > > Signed-off-by: Pablo Sabater <pabloosabaterr@gmail.com> > > --- > > builtin/history.c | 10 ++++++++++ > > t/t3451-history-reword.sh | 20 ++++++++++++++++++++ > > 2 files changed, 30 insertions(+) > > > > diff --git a/builtin/history.c b/builtin/history.c > > index 0fc06fb204..51a22a9a1c 100644 > > --- a/builtin/history.c > > +++ b/builtin/history.c > > @@ -135,6 +135,13 @@ static int commit_tree_ext(struct repository *repo, > > original_body, action, &commit_message); > > if (ret < 0) > > goto out; > > + > > + if (!strcmp(original_body, commit_message.buf)) { > > + fprintf(stderr, _("Message unchanged," > > + " aborting reword.\n")); > > + ret = 1; > > + goto out; > > + } > > } else { > > strbuf_addstr(&commit_message, original_body); > > } > > We also execute this logic via "git history fixup --reedit-message", and > here it wouldn't make sense to abort the commit in case the message is > unchanged. True I hadn't thought that, I made it here because we have both the original and new message before creating the new commit. We could let ret = 1 mean that the commit message is the same and then cmd_history_fixup ignores ret = 1 and for cmd_history_reword handle the abort. What do you think? > > Patrick -- Pablo ^ permalink raw reply [flat|nested] 29+ messages in thread
* Re: [PATCH RFC 1/2] builtin/history: abort reword on unchanged message 2026-06-07 20:07 ` [PATCH RFC 1/2] builtin/history: abort reword on unchanged message Pablo Sabater 2026-06-08 9:30 ` Patrick Steinhardt @ 2026-06-08 12:16 ` Junio C Hamano 2026-06-08 16:44 ` Ben Knoble 2026-06-09 10:14 ` Pablo Sabater 2026-06-08 16:37 ` Ben Knoble 2 siblings, 2 replies; 29+ messages in thread From: Junio C Hamano @ 2026-06-08 12:16 UTC (permalink / raw) To: Pablo Sabater; +Cc: git, Patrick Steinhardt, Kaartic Sivaraam Pablo Sabater <pabloosabaterr@gmail.com> writes: > When using `git history reword` if the new message is the same as the > original it continues anyway creating a new commit with the same > message and updates its descendants, modifying the history after this > 'reworded' commit even though there was no actual change. > > `git commit --amend` and `git rebase -i` + reword share this behavior, > however `git history reword` is different: > 1. Works in-memory without touching the index or the worktree [1], so > there are no side effects like staged files that could justify > rewriting the history when the commit message is the same. > 2. `git history` by default updates all the branches [2] that contain the > original commit making it more costly than `git rebase -i` that only > updates the current branch. I think the reasoning is flawed. Both "git commit --amend" and "git rebase -i", even with no changes to the tree, parents, or the message, update the committer timestamp (and perhaps the committer identity running the command may be different from the original). Updating this info is one of the important effects of the command. And "history" being more capable than "rebase" is a wrong excuse to make the system behave inconsistently between commands that have similar features [*1*]. In a situation where letting 'history' update all the relevant branches, if a command behaves differently from the way the user likes (and if the way 'rebase -i' works is the one the user likes), you'd end up forcing the user to use 'rebase -i' when 'history' would have been more appropriate. Having said that, I personally think that the current behaviour of `commit --amend` and `history reword` are both _wrong_ [*2*]. You may start `git commit --amend`, and after staring at the existing commit log message for some time in your editor, it is quite natural for you to decide that leaving the commit as-is is the right thing [*3*] in your situation. It may have been a better design for the system to notice this situation and leave the commit as-is, with an override option `--force` to allow users to forcibly update the committer ident and timestamp in the commit header. I am not a `history reword` user (yet), but from the motivation you described for this patch, I sense that the story is the same there. `git rebase -i A`, when A is truly an ancestor at the bottom of a linear history leading to HEAD, behaves slightly better. It gives you a todo list with a bunch of `pick` insns, and when you do not edit earliest 'pick's the todo list, these earliest commits are left as-is. It may still share the same issue that a 'reword' that you ended up not rewording (or 'edit' that you ended up not touching its tree or log message) does still recreate a new commit object, though. `git rebase -i` may have an excuse that because it, unlike "git commit --amend", operates on multiple commits by design. A single "--force" option given to the command would not have worked as an escape hatch to allow the user to tell the command "in this reword of this particular commit, I ended up doing nothing, but I still want an updated committer log timestamp". Perhaps giving the "--force" (or --force-rewrite") option at "rebase --continue" time may work, but in any case, unless we plan to transition to these "better" default behaviour at a big version boundary, speculating what a "better" behaviour would have been may be fun but not very productive. [Footnote] *1* Besides, doesn't "--update-refs" in "rebase -i" allow you to adjust the branches? *2* But it is an established behaviour people _rely_ on, so even though it may have been better if these commands behaved differently, it probably is a bit too late to change it now. *3* This includes the case where the original author is especially difficult to work with and would complain any change to their commits, even if the only change you made for them is a typofix. Fixing a small typo/grammo may not be worth your time and unpleasant exchanges with them after touching their commit. ^ permalink raw reply [flat|nested] 29+ messages in thread
* Re: [PATCH RFC 1/2] builtin/history: abort reword on unchanged message 2026-06-08 12:16 ` Junio C Hamano @ 2026-06-08 16:44 ` Ben Knoble 2026-06-09 10:03 ` Pablo Sabater 2026-06-09 10:14 ` Pablo Sabater 1 sibling, 1 reply; 29+ messages in thread From: Ben Knoble @ 2026-06-08 16:44 UTC (permalink / raw) To: Junio C Hamano; +Cc: Pablo Sabater, git, Patrick Steinhardt, Kaartic Sivaraam > Le 8 juin 2026 à 08:23, Junio C Hamano <gitster@pobox.com> a écrit : > [snip] > Having said that, I personally think that the current behaviour of > `commit --amend` and `history reword` are both _wrong_ [*2*]. > > You may start `git commit --amend`, and after staring at the > existing commit log message for some time in your editor, it is > quite natural for you to decide that leaving the commit as-is is the > right thing [*3*] in your situation. It may have been a better > design for the system to notice this situation and leave the commit > as-is, with an override option `--force` to allow users to forcibly > update the committer ident and timestamp in the commit header. I am > not a `history reword` user (yet), but from the motivation you > described for this patch, I sense that the story is the same there. FWIW, in this situation I abort my editor (:cquit in Vim) so that the amend gets an error-valued exit code from the subprocess and aborts itself. Perhaps there could/should be a better side-channel for communicating that, though? I do not know how easy it is to tell other editors to « quit with errors ». > [Footnote] > > *1* Besides, doesn't "--update-refs" in "rebase -i" allow you to > adjust the branches? > > *2* But it is an established behaviour people _rely_ on, so even > though it may have been better if these commands behaved > differently, it probably is a bit too late to change it now. > > *3* This includes the case where the original author is especially > difficult to work with and would complain any change to their > commits, even if the only change you made for them is a > typofix. Fixing a small typo/grammo may not be worth your time > and unpleasant exchanges with them after touching their commit. ^ permalink raw reply [flat|nested] 29+ messages in thread
* Re: [PATCH RFC 1/2] builtin/history: abort reword on unchanged message 2026-06-08 16:44 ` Ben Knoble @ 2026-06-09 10:03 ` Pablo Sabater 0 siblings, 0 replies; 29+ messages in thread From: Pablo Sabater @ 2026-06-09 10:03 UTC (permalink / raw) To: Ben Knoble; +Cc: Junio C Hamano, git, Patrick Steinhardt, Kaartic Sivaraam El lun, 8 jun 2026 a las 18:44, Ben Knoble (<ben.knoble@gmail.com>) escribió: > > > > Le 8 juin 2026 à 08:23, Junio C Hamano <gitster@pobox.com> a écrit : > > > [snip] > > > Having said that, I personally think that the current behaviour of > > `commit --amend` and `history reword` are both _wrong_ [*2*]. > > > > You may start `git commit --amend`, and after staring at the > > existing commit log message for some time in your editor, it is > > quite natural for you to decide that leaving the commit as-is is the > > right thing [*3*] in your situation. It may have been a better > > design for the system to notice this situation and leave the commit > > as-is, with an override option `--force` to allow users to forcibly > > update the committer ident and timestamp in the commit header. I am > > not a `history reword` user (yet), but from the motivation you > > described for this patch, I sense that the story is the same there. > > FWIW, in this situation I abort my editor (:cquit in Vim) so that the amend gets an error-valued exit code from the subprocess and aborts itself. > > Perhaps there could/should be a better side-channel for communicating that, though? I do not know how easy it is to tell other editors to « quit with errors ». Well, I didn't know that I could exit with errors (:cq in NeoVim), can't say much about other editors, but It would be better to abort if the messages are the same and forget about editors. > > > [Footnote] > > > > *1* Besides, doesn't "--update-refs" in "rebase -i" allow you to > > adjust the branches? > > > > *2* But it is an established behaviour people _rely_ on, so even > > though it may have been better if these commands behaved > > differently, it probably is a bit too late to change it now. > > > > *3* This includes the case where the original author is especially > > difficult to work with and would complain any change to their > > commits, even if the only change you made for them is a > > typofix. Fixing a small typo/grammo may not be worth your time > > and unpleasant exchanges with them after touching their commit. Thanks, Pablo ^ permalink raw reply [flat|nested] 29+ messages in thread
* Re: [PATCH RFC 1/2] builtin/history: abort reword on unchanged message 2026-06-08 12:16 ` Junio C Hamano 2026-06-08 16:44 ` Ben Knoble @ 2026-06-09 10:14 ` Pablo Sabater 2026-06-09 10:30 ` Kristoffer Haugsbakk 2026-06-09 13:21 ` Junio C Hamano 1 sibling, 2 replies; 29+ messages in thread From: Pablo Sabater @ 2026-06-09 10:14 UTC (permalink / raw) To: Junio C Hamano; +Cc: git, Patrick Steinhardt, Kaartic Sivaraam El lun, 8 jun 2026 a las 14:16, Junio C Hamano (<gitster@pobox.com>) escribió: > [snip] > > `git rebase -i` may have an excuse that because it, unlike "git > commit --amend", operates on multiple commits by design. A single > "--force" option given to the command would not have worked as an > escape hatch to allow the user to tell the command "in this reword > of this particular commit, I ended up doing nothing, but I still > want an updated committer log timestamp". Perhaps giving the > "--force" (or --force-rewrite") option at "rebase --continue" time > may work, but in any case, unless we plan to transition to these > "better" default behaviour at a big version boundary, speculating > what a "better" behaviour would have been may be fun but not very > productive. > > > [Footnote] > > *1* Besides, doesn't "--update-refs" in "rebase -i" allow you to > adjust the branches? > > *2* But it is an established behaviour people _rely_ on, so even > though it may have been better if these commands behaved > differently, it probably is a bit too late to change it now. > > *3* This includes the case where the original author is especially > difficult to work with and would complain any change to their > commits, even if the only change you made for them is a > typofix. Fixing a small typo/grammo may not be worth your time > and unpleasant exchanges with them after touching their commit. True, after reading it, history being more costly or the in memory are not good args. I do agree that these commands that do reword should check if the reword ends up being the same message, given that history is a new command we can have it from the start so users do not really expect other behavior. About the --force sounds good to me. I could seek to implement it in this series if it's ok. The footnote 3 is indeed a good example haha, but yeah, why rewrite the history unnecessarily. Thanks, Pablo ^ permalink raw reply [flat|nested] 29+ messages in thread
* Re: [PATCH RFC 1/2] builtin/history: abort reword on unchanged message 2026-06-09 10:14 ` Pablo Sabater @ 2026-06-09 10:30 ` Kristoffer Haugsbakk 2026-06-09 13:21 ` Junio C Hamano 1 sibling, 0 replies; 29+ messages in thread From: Kristoffer Haugsbakk @ 2026-06-09 10:30 UTC (permalink / raw) To: Pablo Sabater, Junio C Hamano; +Cc: git, Patrick Steinhardt, Kaartic Sivaraam On Tue, Jun 9, 2026, at 12:14, Pablo Sabater wrote: > El lun, 8 jun 2026 a las 14:16, Junio C Hamano (<gitster@pobox.com>) escribió: >> > [snip] >> >> `git rebase -i` may have an excuse that because it, unlike "git >> commit --amend", operates on multiple commits by design. A single >> "--force" option given to the command would not have worked as an >> escape hatch to allow the user to tell the command "in this reword >> of this particular commit, I ended up doing nothing, but I still >> want an updated committer log timestamp". Perhaps giving the >> "--force" (or --force-rewrite") option at "rebase --continue" time >> may work, but in any case, unless we plan to transition to these >> "better" default behaviour at a big version boundary, speculating >> what a "better" behaviour would have been may be fun but not very >> productive. >> >> >> [Footnote] >> >> *1* Besides, doesn't "--update-refs" in "rebase -i" allow you to >> adjust the branches? >> >> *2* But it is an established behaviour people _rely_ on, so even >> though it may have been better if these commands behaved >> differently, it probably is a bit too late to change it now. >> >> *3* This includes the case where the original author is especially >> difficult to work with and would complain any change to their >> commits, even if the only change you made for them is a >> typofix. Fixing a small typo/grammo may not be worth your time >> and unpleasant exchanges with them after touching their commit. > >[snip] > > About the --force sounds good to me. I could seek to implement it in > this series if it's ok. When starting without historical baggage anyway, I have doubts about the `--force` name in general. This often just begs me to ask what it is forcing. Why not name the thing that is being forced? Verbosity shouldn’t be a problem for a “force” option. So `--force-rewrite` if you are forcing new commits to be created (like already mentioned). See git-clean(1) which has two levels of `--force`. > The footnote 3 is indeed a good example haha, but yeah, why rewrite > the history unnecessarily. ^ permalink raw reply [flat|nested] 29+ messages in thread
* Re: [PATCH RFC 1/2] builtin/history: abort reword on unchanged message 2026-06-09 10:14 ` Pablo Sabater 2026-06-09 10:30 ` Kristoffer Haugsbakk @ 2026-06-09 13:21 ` Junio C Hamano 2026-06-09 15:51 ` Pablo Sabater 1 sibling, 1 reply; 29+ messages in thread From: Junio C Hamano @ 2026-06-09 13:21 UTC (permalink / raw) To: Pablo Sabater; +Cc: git, Patrick Steinhardt, Kaartic Sivaraam Pablo Sabater <pabloosabaterr@gmail.com> writes: > True, after reading it, history being more costly or the in memory are > not good args. And no argument, including that history is new, is a good excuse to make these three things inconsistent, period. One of the patches in your updated iteration claims When using `git history reword <commit>` if the new message is the same as the original, it continues and rewrites the history when nothing changed. `git commit --amend` and `git rebase -i` with reword share this behavior and it is wrong as well, but changing them breaks what people are used to. Take the opportunity of `git history` being a new command and handle it correctly from the start. and I think this is a totally wrong attitude to go about this. I may have said that it may have been a better default to try hard to avoid making a change that is a no-op, other than that it changes committer timestamp, while making the current "always create a new commit object" behaviour optionally available, for these three commands, and cited that the behaviour of 'pick' in 'rebase -i' that avoids unnecessary rewrite as an example of a good practice. But I do not think the existing behaviour to always rewrite is *wrong* at all. It may be wrong not to offer the other choice of pretending no content change means no commit object change, but that is a different story. I also do not think *aborting* only when the message happens to be the same is a valid mode of operation at all. The most sensible first step, I think, is to add a new command line option to "git history" (which will gain more history editing subcommands) that tells the command to leave the original history as-is when the only change rewriting commits would make would be to the committer ident or timestamp information. If in a future a new replace-tree subcommand is added, e.g. if $ git history replace-tree HEAD~20 HEAD~27^{tree} were a command to rewrite the history in such a way that 20th direct ancestor of the current HEAD had a tree object HEAD~27^{tree}, by derfault the command _should_ rewrite HEAD~10 and everything that has it as an ancestor. With the "--avoid-unnecsssary-rewrite" optimization feature on, however, it may silently become a no-op when HEAD~27^{tree} happened to be the same tree as HEAD~20^{tree} so the only difference between rewritten and original HEAD~20 would be when that commit object was created and by whom. And give the same option to "rebase -i" or "commit --amend". We can discuss, educate the users, and flip the default at a major version boundary, if the "avoid unnecessary rewrite" truly turns out to be a better default (right now it is merely our speculation, and we do not even know if the current behaviour is a worse default). ^ permalink raw reply [flat|nested] 29+ messages in thread
* Re: [PATCH RFC 1/2] builtin/history: abort reword on unchanged message 2026-06-09 13:21 ` Junio C Hamano @ 2026-06-09 15:51 ` Pablo Sabater 0 siblings, 0 replies; 29+ messages in thread From: Pablo Sabater @ 2026-06-09 15:51 UTC (permalink / raw) To: Junio C Hamano; +Cc: git, Patrick Steinhardt, Kaartic Sivaraam El mar, 9 jun 2026 a las 15:21, Junio C Hamano (<gitster@pobox.com>) escribió: > > Pablo Sabater <pabloosabaterr@gmail.com> writes: > > > True, after reading it, history being more costly or the in memory are > > not good args. > > And no argument, including that history is new, is a good excuse to > make these three things inconsistent, period. > > One of the patches in your updated iteration claims > > When using `git history reword <commit>` if the new message is the same > as the original, it continues and rewrites the history when nothing > changed. > > `git commit --amend` and `git rebase -i` with reword share this behavior > and it is wrong as well, but changing them breaks what people are used > to. Take the opportunity of `git history` being a new command and handle > it correctly from the start. > > and I think this is a totally wrong attitude to go about this. > > I may have said that it may have been a better default to try hard > to avoid making a change that is a no-op, other than that it changes > committer timestamp, while making the current "always create a new > commit object" behaviour optionally available, for these three > commands, and cited that the behaviour of 'pick' in 'rebase -i' that > avoids unnecessary rewrite as an example of a good practice. > > But I do not think the existing behaviour to always rewrite is > *wrong* at all. It may be wrong not to offer the other choice of > pretending no content change means no commit object change, but that > is a different story. > > I also do not think *aborting* only when the message happens to be > the same is a valid mode of operation at all. > > The most sensible first step, I think, is to add a new command line > option to "git history" (which will gain more history editing > subcommands) that tells the command to leave the original history > as-is when the only change rewriting commits would make would be to > the committer ident or timestamp information. If in a future a new > replace-tree subcommand is added, e.g. if > > $ git history replace-tree HEAD~20 HEAD~27^{tree} > > were a command to rewrite the history in such a way that 20th direct > ancestor of the current HEAD had a tree object HEAD~27^{tree}, by > derfault the command _should_ rewrite HEAD~10 and everything that > has it as an ancestor. With the "--avoid-unnecsssary-rewrite" > optimization feature on, however, it may silently become a no-op > when HEAD~27^{tree} happened to be the same tree as HEAD~20^{tree} > so the only difference between rewritten and original HEAD~20 would > be when that commit object was created and by whom. > > And give the same option to "rebase -i" or "commit --amend". We can > discuss, educate the users, and flip the default at a major version > boundary, if the "avoid unnecessary rewrite" truly turns out to be a > better default (right now it is merely our speculation, and we do > not even know if the current behaviour is a worse default). Hi Junio, Sorry about how I expressed myself. I didn't mean by wrong to be bad or anything similar, I just noticed this when testing `git history reword` and thought that I would like it this other way. Saying that git history is new or I would like this to be different are not good arguments to have `git history` inconsistent with other commands. My idea was more of a defensive thing, where you would need a "--force-rewrite" opt to explicitly change timestamps. But I see the point of having it in an `--avoid-unnecessary-rewrite` so without options it has the same behavior as other commands. I'll try to express myself better in the next version and go with the opt direction. Sorry again, Pablo ^ permalink raw reply [flat|nested] 29+ messages in thread
* Re: [PATCH RFC 1/2] builtin/history: abort reword on unchanged message 2026-06-07 20:07 ` [PATCH RFC 1/2] builtin/history: abort reword on unchanged message Pablo Sabater 2026-06-08 9:30 ` Patrick Steinhardt 2026-06-08 12:16 ` Junio C Hamano @ 2026-06-08 16:37 ` Ben Knoble 2026-06-09 9:59 ` Pablo Sabater 2 siblings, 1 reply; 29+ messages in thread From: Ben Knoble @ 2026-06-08 16:37 UTC (permalink / raw) To: Pablo Sabater; +Cc: git, Patrick Steinhardt, Kaartic Sivaraam, Pablo Sabater I don’t have any strong opinions on the rest… > Le 7 juin 2026 à 16:08, Pablo Sabater <pabloosabaterr@gmail.com> a écrit : > > When using `git history reword` if the new message is the same as the > original it continues anyway creating a new commit with the same > message and updates its descendants, modifying the history after this > 'reworded' commit even though there was no actual change. > > `git commit --amend` and `git rebase -i` + reword share this behavior, > however `git history reword` is different: > 1. Works in-memory without touching the index or the worktree [1], so > there are no side effects like staged files that could justify > rewriting the history when the commit message is the same. > 2. `git history` by default updates all the branches [2] that contain the > original commit making it more costly than `git rebase -i` that only > updates the current branch. > > Add a check if the original commit message is the same as the new one > and abort if so. > > [1]: https://lore.kernel.org/git/20260113-b4-pks-history-builtin-v11-8-e74ebfa2652d@pks.im/ > [2]: https://git-scm.com/docs/git-history#_description > > Signed-off-by: Pablo Sabater <pabloosabaterr@gmail.com> > --- > builtin/history.c | 10 ++++++++++ > t/t3451-history-reword.sh | 20 ++++++++++++++++++++ > 2 files changed, 30 insertions(+) > > diff --git a/builtin/history.c b/builtin/history.c > index 0fc06fb204..51a22a9a1c 100644 > --- a/builtin/history.c > +++ b/builtin/history.c > @@ -135,6 +135,13 @@ static int commit_tree_ext(struct repository *repo, > original_body, action, &commit_message); > if (ret < 0) > goto out; > + > + if (!strcmp(original_body, commit_message.buf)) { > + fprintf(stderr, _("Message unchanged," > + " aborting reword.\n")); > + ret = 1; > + goto out; > + } > } else { > strbuf_addstr(&commit_message, original_body); > } > @@ -718,6 +725,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..54ea8a7207 100755 > --- a/t/t3451-history-reword.sh > +++ b/t/t3451-history-reword.sh > @@ -396,4 +396,24 @@ 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 && > + write_script fake-editor.sh <<-\EOF && > + true > + EOF > + test_set_editor "$(pwd)"/fake-editor.sh && > + git history reword HEAD 2>err && > + git rev-parse HEAD >oid-after && > + test_cmp oid-before oid-after && > + test_grep "Message unchanged" err > + ) …but I think this test case could do something like "GIT_EDITOR=true git history reword HEAD" and avoid the script? > +' > + > test_done > > -- > 2.54.0 Best, Ben ^ permalink raw reply [flat|nested] 29+ messages in thread
* Re: [PATCH RFC 1/2] builtin/history: abort reword on unchanged message 2026-06-08 16:37 ` Ben Knoble @ 2026-06-09 9:59 ` Pablo Sabater 0 siblings, 0 replies; 29+ messages in thread From: Pablo Sabater @ 2026-06-09 9:59 UTC (permalink / raw) To: Ben Knoble; +Cc: git, Patrick Steinhardt, Kaartic Sivaraam El lun, 8 jun 2026 a las 18:37, Ben Knoble (<ben.knoble@gmail.com>) escribió: [snip] > > +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 && > > + write_script fake-editor.sh <<-\EOF && > > + true > > + EOF > > + test_set_editor "$(pwd)"/fake-editor.sh && > > + git history reword HEAD 2>err && > > + git rev-parse HEAD >oid-after && > > + test_cmp oid-before oid-after && > > + test_grep "Message unchanged" err > > + ) > > …but I think this test case could do something like "GIT_EDITOR=true git history reword HEAD" and avoid the script? It does work, thanks. > > > +' > > + > > test_done > > > > -- > > 2.54.0 > > Best, > Ben -- Pablo ^ permalink raw reply [flat|nested] 29+ messages in thread
* [PATCH RFC 2/2] builtin/history: print feedback after successful reword 2026-06-07 20:07 [PATCH RFC 0/2] builtin/history: change git history reword behavior and feedback Pablo Sabater 2026-06-07 20:07 ` [PATCH RFC 1/2] builtin/history: abort reword on unchanged message Pablo Sabater @ 2026-06-07 20:07 ` Pablo Sabater 2026-06-08 9:30 ` Patrick Steinhardt 2026-06-08 12:16 ` Junio C Hamano 2026-06-09 10:42 ` [PATCH RFC v2 0/2] builtin/history: abort reword on same message Pablo Sabater 2 siblings, 2 replies; 29+ messages in thread From: Pablo Sabater @ 2026-06-07 20:07 UTC (permalink / raw) To: git; +Cc: Patrick Steinhardt, Kaartic Sivaraam, Pablo Sabater Unlike `git commit --amend` and `git rebase -i`, `git history reword` doesn't print anything, this makes it feel empty for a porcelain command and hard to tell if the command did anything without using other commands like `git log <commit>` to check if the reword was done. Print a message on successful rewords so the user has feedback about it. Signed-off-by: Pablo Sabater <pabloosabaterr@gmail.com> --- builtin/history.c | 4 ++++ t/t3451-history-reword.sh | 14 ++++++++++++++ 2 files changed, 18 insertions(+) diff --git a/builtin/history.c b/builtin/history.c index 51a22a9a1c..0f1ba3b531 100644 --- a/builtin/history.c +++ b/builtin/history.c @@ -739,6 +739,10 @@ static int cmd_history_reword(int argc, goto out; } + fprintf(stderr, _("Successfully reworded commit %s to %s\n"), + repo_find_unique_abbrev(repo, &original->object.oid, DEFAULT_ABBREV), + repo_find_unique_abbrev(repo, &rewritten->object.oid, DEFAULT_ABBREV)); + ret = 0; out: diff --git a/t/t3451-history-reword.sh b/t/t3451-history-reword.sh index 54ea8a7207..4b22d761e3 100755 --- a/t/t3451-history-reword.sh +++ b/t/t3451-history-reword.sh @@ -416,4 +416,18 @@ test_expect_success 'aborts if the commit message is the same' ' ) ' +test_expect_success 'prints feedback on successful reword' ' + test_when_finished "rm -rf repo" && + git init repo && + ( + cd repo && + test_commit first && + + reword_with_message HEAD 2>err <<-EOF && + first reworded + EOF + test_grep "Successfully reworded" err + ) +' + test_done -- 2.54.0 ^ permalink raw reply related [flat|nested] 29+ messages in thread
* Re: [PATCH RFC 2/2] builtin/history: print feedback after successful reword 2026-06-07 20:07 ` [PATCH RFC 2/2] builtin/history: print feedback after successful reword Pablo Sabater @ 2026-06-08 9:30 ` Patrick Steinhardt 2026-06-08 10:45 ` Pablo Sabater 2026-06-08 12:16 ` Junio C Hamano 1 sibling, 1 reply; 29+ messages in thread From: Patrick Steinhardt @ 2026-06-08 9:30 UTC (permalink / raw) To: Pablo Sabater; +Cc: git, Kaartic Sivaraam On Sun, Jun 07, 2026 at 10:07:21PM +0200, Pablo Sabater wrote: > Unlike `git commit --amend` and `git rebase -i`, `git history reword` > doesn't print anything, this makes it feel empty for a porcelain command > and hard to tell if the command did anything without using other > commands like `git log <commit>` to check if the reword was done. > > Print a message on successful rewords so the user has feedback about it. I dunno about this one. My take here is that a command should be silent unless it has something to say, for example when it couldn't honor the user's request [1]. > diff --git a/builtin/history.c b/builtin/history.c > index 51a22a9a1c..0f1ba3b531 100644 > --- a/builtin/history.c > +++ b/builtin/history.c > @@ -739,6 +739,10 @@ static int cmd_history_reword(int argc, > goto out; > } > > + fprintf(stderr, _("Successfully reworded commit %s to %s\n"), > + repo_find_unique_abbrev(repo, &original->object.oid, DEFAULT_ABBREV), > + repo_find_unique_abbrev(repo, &rewritten->object.oid, DEFAULT_ABBREV)); > + Seeing the implementation also raises a couple of questions: - Why do we mention the rewritten commit, only? Shouldn't we also print the changed HEAD? - Why don't we print any of the other rewritten branches? - What makes "git history reword" so special as compared to for example "git history fixup" or "git history split" so that it needs a message while the others don't? It might make sense to maybe introduce a verbose mode where we do print such information. But if so, we should have good answers to the above questions and implement this in a way that makes sense for the other subcommands, too, so that we can apply the same principle to all of them. Thanks! Patrick [1]: https://www.linfo.org/rule_of_silence.html ^ permalink raw reply [flat|nested] 29+ messages in thread
* Re: [PATCH RFC 2/2] builtin/history: print feedback after successful reword 2026-06-08 9:30 ` Patrick Steinhardt @ 2026-06-08 10:45 ` Pablo Sabater 0 siblings, 0 replies; 29+ messages in thread From: Pablo Sabater @ 2026-06-08 10:45 UTC (permalink / raw) To: Patrick Steinhardt; +Cc: git, Kaartic Sivaraam El lun, 8 jun 2026 a las 11:30, Patrick Steinhardt (<ps@pks.im>) escribió: > > On Sun, Jun 07, 2026 at 10:07:21PM +0200, Pablo Sabater wrote: > > Unlike `git commit --amend` and `git rebase -i`, `git history reword` > > doesn't print anything, this makes it feel empty for a porcelain command > > and hard to tell if the command did anything without using other > > commands like `git log <commit>` to check if the reword was done. > > > > Print a message on successful rewords so the user has feedback about it. > > I dunno about this one. My take here is that a command should be silent > unless it has something to say, for example when it couldn't honor the > user's request [1]. But neither `git commit --amend` nor `git rebase -i` follow this rule of silence. > > > diff --git a/builtin/history.c b/builtin/history.c > > index 51a22a9a1c..0f1ba3b531 100644 > > --- a/builtin/history.c > > +++ b/builtin/history.c > > @@ -739,6 +739,10 @@ static int cmd_history_reword(int argc, > > goto out; > > } > > > > + fprintf(stderr, _("Successfully reworded commit %s to %s\n"), > > + repo_find_unique_abbrev(repo, &original->object.oid, DEFAULT_ABBREV), > > + repo_find_unique_abbrev(repo, &rewritten->object.oid, DEFAULT_ABBREV)); > > + > > Seeing the implementation also raises a couple of questions: > > - Why do we mention the rewritten commit, only? Shouldn't we also > print the changed HEAD? Because `git history reword <commit>` is for a single commit. After the reword the hash changes and the original hash is no longer useful to check the rewritten message. If I want to see how it is now: $ git history reword aabb $ git log aabb <- I can't check how it is now because this is the old one So to check the new one I have to search the new hash. Imagine if it's the first of 20 long commit messages, I have to git log --oneline, get the hash and then git log new_hash, which IMO is unnecessary when git history reword can output the new hash. > > - Why don't we print any of the other rewritten branches? Haven't thought of that, it's nice that it does modify all branches, I just assumed that the most relevant is the current branch new commit hash. The other rewritten branches have the same commit message, just different hashes. > > - What makes "git history reword" so special as compared to for > example "git history fixup" or "git history split" so that it needs > a message while the others don't? Nothing, I just wanted this specifically for reword and sent this very simple as an RFC to discuss the idea, I could extend this where it fits. > > It might make sense to maybe introduce a verbose mode where we do print > such information. But if so, we should have good answers to the above > questions and implement this in a way that makes sense for the other > subcommands, too, so that we can apply the same principle to all of > them. I like the verbose mode idea but I still think that on non-verbose something should be printed, on verbose it could be printed additionally all the rewritten commits (though it could get very noisy), the changed HEAD, etc. > > Thanks! > > Patrick > > [1]: https://www.linfo.org/rule_of_silence.html -- Pablo ^ permalink raw reply [flat|nested] 29+ messages in thread
* Re: [PATCH RFC 2/2] builtin/history: print feedback after successful reword 2026-06-07 20:07 ` [PATCH RFC 2/2] builtin/history: print feedback after successful reword Pablo Sabater 2026-06-08 9:30 ` Patrick Steinhardt @ 2026-06-08 12:16 ` Junio C Hamano 2026-06-08 13:23 ` Pablo Sabater 1 sibling, 1 reply; 29+ messages in thread From: Junio C Hamano @ 2026-06-08 12:16 UTC (permalink / raw) To: Pablo Sabater; +Cc: git, Patrick Steinhardt, Kaartic Sivaraam Pablo Sabater <pabloosabaterr@gmail.com> writes: > Unlike `git commit --amend` and `git rebase -i`, `git history reword` > doesn't print anything, this makes it feel empty for a porcelain command > and hard to tell if the command did anything without using other > commands like `git log <commit>` to check if the reword was done. > > Print a message on successful rewords so the user has feedback about it. > > Signed-off-by: Pablo Sabater <pabloosabaterr@gmail.com> > --- > builtin/history.c | 4 ++++ > t/t3451-history-reword.sh | 14 ++++++++++++++ > 2 files changed, 18 insertions(+) > > diff --git a/builtin/history.c b/builtin/history.c > index 51a22a9a1c..0f1ba3b531 100644 > --- a/builtin/history.c > +++ b/builtin/history.c > @@ -739,6 +739,10 @@ static int cmd_history_reword(int argc, > goto out; > } > > + fprintf(stderr, _("Successfully reworded commit %s to %s\n"), > + repo_find_unique_abbrev(repo, &original->object.oid, DEFAULT_ABBREV), > + repo_find_unique_abbrev(repo, &rewritten->object.oid, DEFAULT_ABBREV)); > + > ret = 0; > > out: Do other commands in "git history" (split is in 'master', drop and fixup are cooking) behave with similar verbosity? Consistency within the same "history" umbrella matters more than being similar with other commands that can be used for similar purposes. > diff --git a/t/t3451-history-reword.sh b/t/t3451-history-reword.sh > index 54ea8a7207..4b22d761e3 100755 > --- a/t/t3451-history-reword.sh > +++ b/t/t3451-history-reword.sh > @@ -416,4 +416,18 @@ test_expect_success 'aborts if the commit message is the same' ' > ) > ' > > +test_expect_success 'prints feedback on successful reword' ' > + test_when_finished "rm -rf repo" && > + git init repo && > + ( > + cd repo && > + test_commit first && > + > + reword_with_message HEAD 2>err <<-EOF && > + first reworded > + EOF > + test_grep "Successfully reworded" err > + ) > +' > + > test_done ^ permalink raw reply [flat|nested] 29+ messages in thread
* Re: [PATCH RFC 2/2] builtin/history: print feedback after successful reword 2026-06-08 12:16 ` Junio C Hamano @ 2026-06-08 13:23 ` Pablo Sabater 2026-06-08 16:47 ` Ben Knoble 0 siblings, 1 reply; 29+ messages in thread From: Pablo Sabater @ 2026-06-08 13:23 UTC (permalink / raw) To: Junio C Hamano; +Cc: git, Patrick Steinhardt, Kaartic Sivaraam El lun, 8 jun 2026 a las 14:16, Junio C Hamano (<gitster@pobox.com>) escribió: > > Pablo Sabater <pabloosabaterr@gmail.com> writes: > > > Unlike `git commit --amend` and `git rebase -i`, `git history reword` > > doesn't print anything, this makes it feel empty for a porcelain command > > and hard to tell if the command did anything without using other > > commands like `git log <commit>` to check if the reword was done. > > > > Print a message on successful rewords so the user has feedback about it. > > > > Signed-off-by: Pablo Sabater <pabloosabaterr@gmail.com> > > --- > > builtin/history.c | 4 ++++ > > t/t3451-history-reword.sh | 14 ++++++++++++++ > > 2 files changed, 18 insertions(+) > > > > diff --git a/builtin/history.c b/builtin/history.c > > index 51a22a9a1c..0f1ba3b531 100644 > > --- a/builtin/history.c > > +++ b/builtin/history.c > > @@ -739,6 +739,10 @@ static int cmd_history_reword(int argc, > > goto out; > > } > > > > + fprintf(stderr, _("Successfully reworded commit %s to %s\n"), > > + repo_find_unique_abbrev(repo, &original->object.oid, DEFAULT_ABBREV), > > + repo_find_unique_abbrev(repo, &rewritten->object.oid, DEFAULT_ABBREV)); > > + > > ret = 0; > > > > out: > > Do other commands in "git history" (split is in 'master', drop and > fixup are cooking) behave with similar verbosity? Consistency within > the same "history" umbrella matters more than being similar with > other commands that can be used for similar purposes. They do not, they are thought with the rule of silence in mind. However I think that this output is valuable information I might have explained myself better at [1] but my thought is: git history reword aabb Now that I have my commit aabb rewritten I want to check it again just to make sure I did what I wanted correctly, but git log aabb is still the old commit, the rewritten one has a different hash which I do not know unless I search for it, if it's far from HEAD I'd have to git log --oneline, get the hash and then git log new_hash. I think that git history reword that does have the information about the new hash should print it to avoid this search. What I want is something like: git history reword aabb Successfully reworded aabb to ccdd So I can just git log ccdd without having to search. I want to say I haven't looked as much as I'd like to split, drop and fixup, but I think it would be a good addition for them also. On [1] Patrick wrote about a --verbose for git history, I think that the basic information i.e. at reword which is the new hash should be always printed but if it's preferred it could go there. For split it can print the hashes of the new commits like: "...split into ccdd and eeff." For fixup the commit hash also changes, so the same as reword. The one that will have more friction would be drop is the one that doesn't end up with new commits. [1]: https://lore.kernel.org/git/CAN5EUNSAOMRvmLGVfzQiwWoOn9VGNVU5rVMZizOryn_q2fbCNA@mail.gmail.com/ > > > diff --git a/t/t3451-history-reword.sh b/t/t3451-history-reword.sh > > index 54ea8a7207..4b22d761e3 100755 > > --- a/t/t3451-history-reword.sh > > +++ b/t/t3451-history-reword.sh > > @@ -416,4 +416,18 @@ test_expect_success 'aborts if the commit message is the same' ' > > ) > > ' > > > > +test_expect_success 'prints feedback on successful reword' ' > > + test_when_finished "rm -rf repo" && > > + git init repo && > > + ( > > + cd repo && > > + test_commit first && > > + > > + reword_with_message HEAD 2>err <<-EOF && > > + first reworded > > + EOF > > + test_grep "Successfully reworded" err > > + ) > > +' > > + > > test_done ^ permalink raw reply [flat|nested] 29+ messages in thread
* Re: [PATCH RFC 2/2] builtin/history: print feedback after successful reword 2026-06-08 13:23 ` Pablo Sabater @ 2026-06-08 16:47 ` Ben Knoble 0 siblings, 0 replies; 29+ messages in thread From: Ben Knoble @ 2026-06-08 16:47 UTC (permalink / raw) To: Pablo Sabater; +Cc: Junio C Hamano, git, Patrick Steinhardt, Kaartic Sivaraam > Le 8 juin 2026 à 09:29, Pablo Sabater <pabloosabaterr@gmail.com> a écrit : > > El lun, 8 jun 2026 a las 14:16, Junio C Hamano (<gitster@pobox.com>) escribió: >> >> Pablo Sabater <pabloosabaterr@gmail.com> writes: >> >>> Unlike `git commit --amend` and `git rebase -i`, `git history reword` >>> doesn't print anything, this makes it feel empty for a porcelain command >>> and hard to tell if the command did anything without using other >>> commands like `git log <commit>` to check if the reword was done. >>> >>> Print a message on successful rewords so the user has feedback about it. >>> >>> Signed-off-by: Pablo Sabater <pabloosabaterr@gmail.com> >>> --- >>> builtin/history.c | 4 ++++ >>> t/t3451-history-reword.sh | 14 ++++++++++++++ >>> 2 files changed, 18 insertions(+) >>> >>> diff --git a/builtin/history.c b/builtin/history.c >>> index 51a22a9a1c..0f1ba3b531 100644 >>> --- a/builtin/history.c >>> +++ b/builtin/history.c >>> @@ -739,6 +739,10 @@ static int cmd_history_reword(int argc, >>> goto out; >>> } >>> >>> + fprintf(stderr, _("Successfully reworded commit %s to %s\n"), >>> + repo_find_unique_abbrev(repo, &original->object.oid, DEFAULT_ABBREV), >>> + repo_find_unique_abbrev(repo, &rewritten->object.oid, DEFAULT_ABBREV)); >>> + >>> ret = 0; >>> >>> out: >> >> Do other commands in "git history" (split is in 'master', drop and >> fixup are cooking) behave with similar verbosity? Consistency within >> the same "history" umbrella matters more than being similar with >> other commands that can be used for similar purposes. > > They do not, they are thought with the rule of silence in mind. > However I think that this output is valuable information I might have > explained myself better at [1] but my thought is: > > git history reword aabb > > Now that I have my commit aabb rewritten I want to check it again just > to make sure I did what I wanted correctly, Some thoughts: - If the rewritten commit is an ancestor of HEAD, look at the log of HEAD@{1} or the log between HEAD and the aforementioned reflog entry. (git-range-diff may also be helpful there.) - Similarly, if the rewritten commit is reachable from some ref R, check R@{1} etc. > but git log aabb is still > the old commit, the rewritten one has a different hash which I do not > know unless I search for it, if it's far from HEAD I'd have to git log > --oneline, get the hash and then git log new_hash. I think that git > history reword that does have the information about the new hash > should print it to avoid this search. > What I want is something like: > > git history reword aabb > Successfully reworded aabb to ccdd > > So I can just git log ccdd without having to search. > > I want to say I haven't looked as much as I'd like to split, drop and > fixup, but I think it would be a good addition for them also. On [1] > Patrick wrote about a --verbose for git history, I think that the > basic information i.e. at reword which is the new hash should be > always printed but if it's preferred it could go there. > > For split it can print the hashes of the new commits like: > "...split into ccdd and eeff." > For fixup the commit hash also changes, so the same as reword. > The one that will have more friction would be drop is the one that > doesn't end up with new commits. > > [1]: https://lore.kernel.org/git/CAN5EUNSAOMRvmLGVfzQiwWoOn9VGNVU5rVMZizOryn_q2fbCNA@mail.gmail.com/ ^ permalink raw reply [flat|nested] 29+ messages in thread
* [PATCH RFC v2 0/2] builtin/history: abort reword on same message 2026-06-07 20:07 [PATCH RFC 0/2] builtin/history: change git history reword behavior and feedback Pablo Sabater 2026-06-07 20:07 ` [PATCH RFC 1/2] builtin/history: abort reword on unchanged message Pablo Sabater 2026-06-07 20:07 ` [PATCH RFC 2/2] builtin/history: print feedback after successful reword Pablo Sabater @ 2026-06-09 10:42 ` Pablo Sabater 2026-06-09 10:42 ` [PATCH RFC v2 1/2] builtin/history: refactor function signature Pablo Sabater 2026-06-09 10:42 ` [PATCH RFC v2 2/2] builtin/history: abort reword on same message Pablo Sabater 2 siblings, 2 replies; 29+ messages in thread From: Pablo Sabater @ 2026-06-09 10:42 UTC (permalink / raw) To: git; +Cc: cat, ps, kaartic.sivaraam, pabloosabaterr, ben.knoble, gitster This short series aims to improve the behavior of `git history reword` to abort when the new commit message is the same as the original, avoiding unnecessary history rewrites. `git commit --amend` and `git rebase -i` with reword share this flaw but changing them faces not just technical challenges but also breaks what people are used to, so that is not a viable option. Let's take the opportunity that `git history` is a new command and handle this correctly from the start. This is made so any other future subcommand or option that does want this behavior just has to add the abort flag. A questions I have is why don't we want this abort behavior on `git history fixup --reedit-message` it makes more sense on `git history reword` because if the message is the same then it has nothing to do while fixup can still have files to update, but --reedit-message is still a redundant option there. Signed-off-by: Pablo Sabater <pabloosabaterr@gmail.com> --- Changes in v2: - Changed the reason on why is this needed. - Changed tests with same message to use GIT_EDITOR=true instead of the script. - Abort on same message only happens when its own flag is set so no other subcommand that does not want this behavior and depend on commit_tree_ext() is affected. - Dropped the feedback on successful reword for another series. --- Pablo Sabater (2): builtin/history: refactor function signature builtin/history: abort reword on same message builtin/history.c | 21 ++++++++++++++++++--- t/t3451-history-reword.sh | 16 ++++++++++++++++ t/t3453-history-fixup.sh | 22 ++++++++++++++++++++++ 3 files changed, 56 insertions(+), 3 deletions(-) --- base-commit: 9ac3f193c05c2237e2b14ebaa1149e9fc8a1abe0 change-id: 20260607-ps-history-reword-fcb70eaa4aa9 Best regards, -- Pablo Sabater <pabloosabaterr@gmail.com> ^ permalink raw reply [flat|nested] 29+ messages in thread
* [PATCH RFC v2 1/2] builtin/history: refactor function signature 2026-06-09 10:42 ` [PATCH RFC v2 0/2] builtin/history: abort reword on same message Pablo Sabater @ 2026-06-09 10:42 ` Pablo Sabater 2026-06-09 10:42 ` [PATCH RFC v2 2/2] builtin/history: abort reword on same message Pablo Sabater 1 sibling, 0 replies; 29+ messages in thread From: Pablo Sabater @ 2026-06-09 10:42 UTC (permalink / raw) To: git; +Cc: cat, ps, kaartic.sivaraam, pabloosabaterr, ben.knoble, gitster commit_tree_with_edited_message() calls commit_tree_ext() with the flag COMMIT_TREE_EDIT_MESSAGE hardcoded and we can't set new flags on callers like cmd_history_reword() to choose their own flags. This refactor is needed for a subsequent commit. Refactor commit_tree_with_edited_message() signature to accept flags which are passed down to commit_tree_ext() instead of the hardcoded one. Signed-off-by: Pablo Sabater <pabloosabaterr@gmail.com> --- builtin/history.c | 9 ++++++--- 1 file changed, 6 insertions(+), 3 deletions(-) diff --git a/builtin/history.c b/builtin/history.c index 0fc06fb204..b3e2e5270d 100644 --- a/builtin/history.c +++ b/builtin/history.c @@ -160,7 +160,8 @@ static int commit_tree_ext(struct repository *repo, static int commit_tree_with_edited_message(struct repository *repo, const char *action, struct commit *original, - struct commit **out) + struct commit **out, + enum commit_tree_flags flags) { struct object_id parent_tree_oid; const struct object_id *tree_oid; @@ -181,7 +182,7 @@ static int commit_tree_with_edited_message(struct repository *repo, } return commit_tree_ext(repo, action, original, original->parents, - &parent_tree_oid, tree_oid, out, COMMIT_TREE_EDIT_MESSAGE); + &parent_tree_oid, tree_oid, out, flags); } enum ref_action { @@ -692,6 +693,7 @@ 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; int ret; argc = parse_options(argc, argv, prefix, options, usage, 0); @@ -714,7 +716,8 @@ static int cmd_history_reword(int argc, if (ret) goto out; - ret = commit_tree_with_edited_message(repo, "reworded", original, &rewritten); + ret = commit_tree_with_edited_message(repo, "reworded", original, + &rewritten, flags); if (ret < 0) { ret = error(_("failed writing reworded commit")); goto out; -- 2.54.0 ^ permalink raw reply related [flat|nested] 29+ messages in thread
* [PATCH RFC v2 2/2] builtin/history: abort reword on same message 2026-06-09 10:42 ` [PATCH RFC v2 0/2] builtin/history: abort reword on same message Pablo Sabater 2026-06-09 10:42 ` [PATCH RFC v2 1/2] builtin/history: refactor function signature Pablo Sabater @ 2026-06-09 10:42 ` Pablo Sabater 2026-06-09 13:25 ` Phillip Wood 1 sibling, 1 reply; 29+ messages in thread From: Pablo Sabater @ 2026-06-09 10:42 UTC (permalink / raw) To: git; +Cc: cat, ps, kaartic.sivaraam, pabloosabaterr, ben.knoble, gitster When using `git history reword <commit>` if the new message is the same as the original, it continues and rewrites the history when nothing changed. `git commit --amend` and `git rebase -i` with reword share this behavior and it is wrong as well, but changing them breaks what people are used to. Take the opportunity of `git history` being a new command and handle it correctly from the start. Create COMMIT_TREE_ABORT_ON_SAME_MESSAGE and make a check for if the messages are the same and the flag is set so other subcommands like fixup that do not want this behavior just don't send the abort flag. Make commit_tree_ext() return 1 when facing the same message so its callers can choose what to do. Signed-off-by: Pablo Sabater <pabloosabaterr@gmail.com> --- builtin/history.c | 14 +++++++++++++- t/t3451-history-reword.sh | 16 ++++++++++++++++ t/t3453-history-fixup.sh | 22 ++++++++++++++++++++++ 3 files changed, 51 insertions(+), 1 deletion(-) diff --git a/builtin/history.c b/builtin/history.c index b3e2e5270d..be07690da4 100644 --- a/builtin/history.c +++ b/builtin/history.c @@ -96,6 +96,7 @@ static int fill_commit_message(struct repository *repo, enum commit_tree_flags { COMMIT_TREE_EDIT_MESSAGE = (1 << 0), + COMMIT_TREE_ABORT_ON_SAME_MESSAGE = (1 << 1), }; 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; + } } 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 && -- 2.54.0 ^ permalink raw reply related [flat|nested] 29+ messages in thread
* Re: [PATCH RFC v2 2/2] builtin/history: abort reword on same message 2026-06-09 10:42 ` [PATCH RFC v2 2/2] builtin/history: abort reword on same message Pablo Sabater @ 2026-06-09 13:25 ` Phillip Wood 2026-06-09 16:20 ` Junio C Hamano 0 siblings, 1 reply; 29+ messages in thread From: Phillip Wood @ 2026-06-09 13:25 UTC (permalink / raw) To: Pablo Sabater, git; +Cc: cat, ps, kaartic.sivaraam, ben.knoble, gitster 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 && > ^ permalink raw reply [flat|nested] 29+ messages in thread
* Re: [PATCH RFC v2 2/2] builtin/history: abort reword on same message 2026-06-09 13:25 ` Phillip Wood @ 2026-06-09 16:20 ` Junio C Hamano 2026-06-09 17:12 ` Pablo Sabater 2026-06-09 18:02 ` Justin Tobler 0 siblings, 2 replies; 29+ messages in thread From: Junio C Hamano @ 2026-06-09 16:20 UTC (permalink / raw) To: Phillip Wood; +Cc: Pablo Sabater, git, cat, ps, kaartic.sivaraam, ben.knoble Phillip Wood <phillip.wood123@gmail.com> writes: > 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. I am not sure why it would be a good idea. The user wanted to make the commit have this message, and the commit ended up having the same message as the user gave. That message may have been identical to what the commit originally had, or it may be different. Why is the former an abort-worthy event? A simple note, I may understand, but aborting with an error message? Thanks. ^ permalink raw reply [flat|nested] 29+ messages in thread
* Re: [PATCH RFC v2 2/2] builtin/history: abort reword on same message 2026-06-09 16:20 ` Junio C Hamano @ 2026-06-09 17:12 ` Pablo Sabater 2026-06-09 19:17 ` Junio C Hamano 2026-06-09 18:02 ` Justin Tobler 1 sibling, 1 reply; 29+ messages in thread From: Pablo Sabater @ 2026-06-09 17:12 UTC (permalink / raw) To: Junio C Hamano; +Cc: Phillip Wood, git, cat, ps, kaartic.sivaraam, ben.knoble El mar, 9 jun 2026 a las 18:20, Junio C Hamano (<gitster@pobox.com>) escribió: > > Phillip Wood <phillip.wood123@gmail.com> writes: > > > 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. I think that if you reword a commit committed by someone else but end up with no changes I want it to be kept as it was. > > > > Aborting when the message and committer identity are unchanged seems > > like a good idea. > > I am not sure why it would be a good idea. The user wanted to make > the commit have this message, and the commit ended up having the > same message as the user gave. That message may have been identical > to what the commit originally had, or it may be different. Why is > the former an abort-worthy event? A simple note, I may understand, > but aborting with an error message? With what you said at [1], having this in an "--avoid-unnecessary-rewrite" I think that the abort might be too much as with the flag the user already expects this to happen and silent might be better. By the way, I feel that "--avoid-unnecessary-rewrite" is too long, could it be something shorter? If not it could be set "-r" as the short and leave the long as it is. > > Thanks. [1]: https://lore.kernel.org/git/xmqqtsrbsvcm.fsf@gitster.g/ Thanks, Pablo ^ permalink raw reply [flat|nested] 29+ messages in thread
* Re: [PATCH RFC v2 2/2] builtin/history: abort reword on same message 2026-06-09 17:12 ` Pablo Sabater @ 2026-06-09 19:17 ` Junio C Hamano 0 siblings, 0 replies; 29+ messages in thread From: Junio C Hamano @ 2026-06-09 19:17 UTC (permalink / raw) To: Pablo Sabater; +Cc: Phillip Wood, git, cat, ps, kaartic.sivaraam, ben.knoble Pablo Sabater <pabloosabaterr@gmail.com> writes: >> > 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. > > I think that if you reword a commit committed by someone else but end > up with no changes I want it to be kept as it was. That depends on the reason why the feature to "reword" the commit is being used, and the use case Phillip is talking about is a bit different. A very common mistake a new user makes when starting a repository is to make commits before they realize that they used a wrong identity to create them. They are happy with what they committed, except that they want these commits to be attributed to user.{name,email} they corrected. Also, people often use multiple identities (e.g., corp vs personal), and when making commits to the project for their employer they do not want to use their personal identity (and vice versa). After making a mistake to create commits under wrong identity, they want to fix these commits. In such situations, there is no room for leaving the committer name as "someone else". The user wants to get rid of the "someone else"s identity out of these commits. ^ permalink raw reply [flat|nested] 29+ messages in thread
* Re: [PATCH RFC v2 2/2] builtin/history: abort reword on same message 2026-06-09 16:20 ` Junio C Hamano 2026-06-09 17:12 ` Pablo Sabater @ 2026-06-09 18:02 ` Justin Tobler 2026-06-09 19:30 ` Junio C Hamano 1 sibling, 1 reply; 29+ messages in thread From: Justin Tobler @ 2026-06-09 18:02 UTC (permalink / raw) To: Junio C Hamano Cc: Phillip Wood, Pablo Sabater, git, cat, ps, kaartic.sivaraam, ben.knoble On 26/06/09 09:20AM, Junio C Hamano wrote: > Phillip Wood <phillip.wood123@gmail.com> writes: > > > 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. > > I am not sure why it would be a good idea. The user wanted to make > the commit have this message, and the commit ended up having the > same message as the user gave. That message may have been identical > to what the commit originally had, or it may be different. Why is > the former an abort-worthy event? A simple note, I may understand, > but aborting with an error message? I can see a situation where a user performs: git history reword abcd1234 with the intention to modify a commit message, but then for some reason changes their mind and doesn't want history to change. Maybe the wrong commit was referenced, or they decide the current message is actually fine. From my understanding, there isn't a great way to abort rewording a commit during editing and thus the user would have to reset history afterwards if they care enough to go back to the previous point. So I do see some value in a mechanism to abort rewriting a commit message. An unchanged commit message does seem like a reasonable signal to essentially abort the reword. I'm not sure committer identity should be taken into consideration though since it would inhibit a users ability to abort the reword if they ever touch a commit that they themselves are not the previous committer. I don't think there is a need to have an error message though. Even in the case where the user leaves the commit message unchanged and history is left untouched, git-history(1) would be following exactly what the user instructed it to do. I don't really see why the user should care whether history was actually modified or not in such a scenario. -Justin ^ permalink raw reply [flat|nested] 29+ messages in thread
* Re: [PATCH RFC v2 2/2] builtin/history: abort reword on same message 2026-06-09 18:02 ` Justin Tobler @ 2026-06-09 19:30 ` Junio C Hamano 2026-06-09 20:14 ` Justin Tobler 0 siblings, 1 reply; 29+ messages in thread From: Junio C Hamano @ 2026-06-09 19:30 UTC (permalink / raw) To: Justin Tobler Cc: Phillip Wood, Pablo Sabater, git, cat, ps, kaartic.sivaraam, ben.knoble Justin Tobler <jltobler@gmail.com> writes: > I can see a situation where a user performs: > > git history reword abcd1234 > > with the intention to modify a commit message, but then for some reason > changes their mind and doesn't want history to change. Maybe the wrong > commit was referenced, or they decide the current message is actually > fine. From my understanding, there isn't a great way to abort rewording > a commit during editing and thus the user would have to reset history > afterwards if they care enough to go back to the previous point. > > So I do see some value in a mechanism to abort rewriting a commit > message. I think we are saying the same thing in different ways. I want to see that command "succeed" either case (normally we create a new commit object because we record an updated committer timestamp, but if there is no need to create a new commit object only to record an updated committer timestamp, we may choose not to and leave the history intact) and I do not want it to *abort*. The mechanism to do so may be exactly the same, i.e., accept an updated log message, then try to "hash-object" (without -w) the commit object with everything, except for the updated commit log message, taken from the original commit, plus the updated log message. And if the resulting hash is the same as the original, do not do anything further and return happily. Aborting sounds more like complaining loudly "baa, you asked me to reword but you gave me the same message? is anything wrong with you?" with non-zero exit status, which I think the user does not deserve in such a case. ^ permalink raw reply [flat|nested] 29+ messages in thread
* Re: [PATCH RFC v2 2/2] builtin/history: abort reword on same message 2026-06-09 19:30 ` Junio C Hamano @ 2026-06-09 20:14 ` Justin Tobler 0 siblings, 0 replies; 29+ messages in thread From: Justin Tobler @ 2026-06-09 20:14 UTC (permalink / raw) To: Junio C Hamano Cc: Phillip Wood, Pablo Sabater, git, cat, ps, kaartic.sivaraam, ben.knoble On 26/06/09 12:30PM, Junio C Hamano wrote: > Justin Tobler <jltobler@gmail.com> writes: > > > I can see a situation where a user performs: > > > > git history reword abcd1234 > > > > with the intention to modify a commit message, but then for some reason > > changes their mind and doesn't want history to change. Maybe the wrong > > commit was referenced, or they decide the current message is actually > > fine. From my understanding, there isn't a great way to abort rewording > > a commit during editing and thus the user would have to reset history > > afterwards if they care enough to go back to the previous point. > > > > So I do see some value in a mechanism to abort rewriting a commit > > message. > > I think we are saying the same thing in different ways. I want to > see that command "succeed" either case (normally we create a new > commit object because we record an updated committer timestamp, but > if there is no need to create a new commit object only to record an > updated committer timestamp, we may choose not to and leave the > history intact) and I do not want it to *abort*. > > The mechanism to do so may be exactly the same, i.e., accept an > updated log message, then try to "hash-object" (without -w) the > commit object with everything, except for the updated commit log > message, taken from the original commit, plus the updated log > message. And if the resulting hash is the same as the original, do > not do anything further and return happily. Aborting sounds more > like complaining loudly "baa, you asked me to reword but you gave me > the same message? is anything wrong with you?" with non-zero exit > status, which I think the user does not deserve in such a case. Yes, I completely agree. If the user doesn't update the commit message, for whatever reason, that should still be considered a success since it follows the user's intent. I don't think it makes sense to exit with a non-zero code in such cases. I would also question if we should print any message/note to the user at all for the same reasons. -Justin ^ permalink raw reply [flat|nested] 29+ messages in thread
end of thread, other threads:[~2026-06-09 20:14 UTC | newest] Thread overview: 29+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2026-06-07 20:07 [PATCH RFC 0/2] builtin/history: change git history reword behavior and feedback Pablo Sabater 2026-06-07 20:07 ` [PATCH RFC 1/2] builtin/history: abort reword on unchanged message Pablo Sabater 2026-06-08 9:30 ` Patrick Steinhardt 2026-06-08 10:52 ` Pablo Sabater 2026-06-08 12:16 ` Junio C Hamano 2026-06-08 16:44 ` Ben Knoble 2026-06-09 10:03 ` Pablo Sabater 2026-06-09 10:14 ` Pablo Sabater 2026-06-09 10:30 ` Kristoffer Haugsbakk 2026-06-09 13:21 ` Junio C Hamano 2026-06-09 15:51 ` Pablo Sabater 2026-06-08 16:37 ` Ben Knoble 2026-06-09 9:59 ` Pablo Sabater 2026-06-07 20:07 ` [PATCH RFC 2/2] builtin/history: print feedback after successful reword Pablo Sabater 2026-06-08 9:30 ` Patrick Steinhardt 2026-06-08 10:45 ` Pablo Sabater 2026-06-08 12:16 ` Junio C Hamano 2026-06-08 13:23 ` Pablo Sabater 2026-06-08 16:47 ` Ben Knoble 2026-06-09 10:42 ` [PATCH RFC v2 0/2] builtin/history: abort reword on same message Pablo Sabater 2026-06-09 10:42 ` [PATCH RFC v2 1/2] builtin/history: refactor function signature Pablo Sabater 2026-06-09 10:42 ` [PATCH RFC v2 2/2] builtin/history: abort reword on same message Pablo Sabater 2026-06-09 13:25 ` Phillip Wood 2026-06-09 16:20 ` Junio C Hamano 2026-06-09 17:12 ` Pablo Sabater 2026-06-09 19:17 ` Junio C Hamano 2026-06-09 18:02 ` Justin Tobler 2026-06-09 19:30 ` Junio C Hamano 2026-06-09 20:14 ` Justin Tobler
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox