Re: [PATCH v5 06/12] builtin/history: implement "reword" subcommand
- From
Patrick Steinhardt <ps@pks.im>
- Date
- Oct 27, 2025, 09:58 UTC
- Message-ID
- <aP9CMa3dDuV_RGnG@pks.im>
- In-Reply-To
- <xmqqa51kszvc.fsf@gitster.g>
On Tue, Oct 21, 2025 at 02:34:31PM -0700, Junio C Hamano wrote:
Show 11 quoted lines
> Patrick Steinhardt <ps@pks.im> writes: > > @@ -9,6 +9,7 @@ SYNOPSIS > > -------- > > [synopsis] > > git history [<options>] > > +git history reword [<options>] <commit> > > It is curious that a command-less form is still listed here, > especially since this command "requires" a subcommand. I would have > expected that there will be a single line here after implementing a > single subcommand.
True. You can execute `git history` alone, but it doesn't do anything useful except for giving you an error and all the potential subcommands. That's in contrast to e.g. git-reflog(1), which supports a command-less mode that does something useful.
> > +`reword <commit> [--message=<message>]`:: > > That should be `reword [--message=<message>] <commit>` no?
Indeed.
Show 9 quoted lines
> > + Rewrite the commit message of the specified commit. All the other > > + details of this commit remain unchanged. If no commit message is > > + provided, then this command will spawn an editor with the current > > + message of that commit. > > As long as it takes more than one -m and concatenates them just like > "git commit -m <message1> -m <message2>" does, I would not complain > too much that a command line option to give message encourages sloppy > log messages.
I'll for now defer the discussion around "-m" completely, as the design isn't entirely clear yet in the first place. So I'll just drop the option in the next iteration.
Show 41 quoted lines
> > + if (!onto) {
> > + onto = commit;
> > + result.clean = 1;
> > + result.tree = repo_get_commit_tree(repo, commit);
> > + } else {
> > + onto = replay_pick_regular_commit(repo, commit, replayed_commits,
> > + onto, &merge_opts, &result);
> > + if (!onto)
> > + break;
> > + }
>
> Hmph, I would have expected that the overall flow of this command
> would be
>
> * find the commits above and including the <commit> in question,
> making sure there is no merge.
>
> * read metadata of <commit> like the parent (as we do not allow
> merges), tree, author ident & time.
>
> * create a new commit object that has the same metadata as <commit>
> on top of the parent of <commit>, but with the updated message
> and new committer ident & time.
>
> * initialize a variable Current to point at the rewritten <commit>
>
> * loop for each commit C in <commit>..HEAD range in reverse order
> (we know we have a single strand of pearls):
>
> - read metadata of C
>
> - create a new commit object C' that has the same metadata and
> message as C on top of the Current commit, with new committer
> ident & time.
>
> - make Current point at the resulting C'
>
> * Point the Current with HEAD.
>
> without having to touch any "pick" machinery. Why do we need to go
> down to the merge machinery for a mere "reword" operation?You're exactly right, we don't need the "pick" machinery at all right now. I think this is still a leftover from previous iterations, where I was also driving things like an "edit" command that _do_ require merges. But neither splitting a commit nor rewording it does require a merge at all.
Will simplify. We can reintroduce the heavier machinery at a later point in time as needed.
Thanks!
Patrick