Re: [PATCH RFC 11/11] builtin/history: implement "split" subcommand
- From
Patrick Steinhardt <ps@pks.im>
- Date
- Aug 22, 2025, 12:22 UTC
- Message-ID
- <aKhg8q-AAlsGDvFS@pks.im>
- In-Reply-To
- <CALnO6CBuwDVMZ-QTay+PUiXKsWMsABJcs1pAB=uUXf7-DJ4Mnw@mail.gmail.com>
On Wed, Aug 20, 2025 at 05:27:32PM -0400, D. Ben Knoble wrote:
Show 27 quoted lines
> On Wed, Aug 20, 2025 at 5:05 AM Patrick Steinhardt <ps@pks.im> wrote: > > diff --git a/Documentation/git-history.adoc b/Documentation/git-history.adoc > > index 6e8b4e1326..f0f1f2a093 100644 > > --- a/Documentation/git-history.adoc > > +++ b/Documentation/git-history.adoc > > @@ -47,6 +48,26 @@ reorder <revision> (--before=<revision>|--after=<revision>):: > > commit. The commits must be related to one another and must be > > reachable from the current `HEAD` commit. > > > > +split <revision> [--message=<message>] [--] [<pathspec>...]:: > > + Interactively split up the commit into two commits by choosing > > + hunks introduced by it that will be moved into the new split-out > > + commit. These hunks will then be written into a new commit that > > + becomes the parent of the previous commit. The original commit > > + stays intact, except that its parent will be the newly split-out > > + commit. > > ++ > > +The commit message of the new commit will be asked for by launching the > > +configured editor. Authorship of the commit will be the same as for the > > +original commit. > > ++ > > +If passed, _<pathspec>_ can be used to limit which changes shall be split out > > +of the original commit. Files not matching any of the pathspecs will remain > > +part of the original commit. For more details about the _<pathspec>_ syntax, > > +see the 'pathspec' entry. > > Glossary entry?
Yup.
Show 16 quoted lines
> > + /* > > + * But we do ask the user for a new commit message. This is in contrast > > + * to the second commit, where we'll retain the original commit > > + * message. > > + */ > > Interesting. I can see using the original as the template for _both_, > or the first instead of the second. jj's split works a little > differently (especially with their notion of descriptions), so I can't > use them as a reference for the behavior. > > I suppose this is one of those "everybody has their preference" > things, but I think giving the message in both new commits as the > template gives splitters the most information available when writing > the message. (Of course, in my editor, I can presumably do something > like ":Git show -s <split-commit-ish>" if I want.)
I think giving only the split-out changes is a reasonable default, but I can totally see that we might eventually want to add a command line option to change the behaviour.
Show 14 quoted lines
> > + if (!commit_message) {
> > + split_message_path = repo_git_path(repo, "SPLIT_MSG");
> > + strbuf_addch(&split_message, '\n');
> > + strbuf_commented_addf(&split_message, comment_line_str,
> > + _("Please enter a commit message for the split-out changes."));
> > + write_file_buf(split_message_path, split_message.buf, split_message.len);
>
> I also noticed the commented template differs substantially from the
> regular commit template, and my editor doesn't recognize "SPLIT_MSG"
> as a commit message file.
>
> The latter can be fixed elsewhere, but for the former: perhaps it's
> worth using the usual template with the wording here prepended?
> Respecting commit.verbose / commit.status, too.Yeah, that's something I wanted to get around to, but haven't yet. I also noticed that it's not exactly easy to figure out what you're currently editing without that lack of context.
I'll include that in v2.
Show 7 quoted lines
> BTW, if I quit the editor with an error here, I'm left back where I > started. So I'd have to re-stage changes if I wanted to split again, > which is a bit different from how interactive rebase will leave me > with the partially staged changes. Obviously that's harder to do with > the in-memory index + automatic re-application of remaining patch when > finished, so maybe a note in the docs about this being "all or > nothing"?
Yeah, fair. I guess adding a note for now is the best way to go about it, but this is certainly something we can and should iterate on in the future.
Patrick