Git development
 help / color / mirror / Atom feed
From: Patrick Steinhardt <ps@pks.im>
To: "D. Ben Knoble" <ben.knoble@gmail.com>
Cc: git@vger.kernel.org, Elijah Newren <newren@gmail.com>
Subject: Re: [PATCH 2/2] builtin/history: introduce "fixup" subcommand
Date: Fri, 24 Apr 2026 08:53:45 +0200	[thread overview]
Message-ID: <aesTeWQqMTFd4gy8@pks.im> (raw)
In-Reply-To: <CALnO6CAZQxvqEqDhahFs7NcjENrU=Dg=cbFDkEeAE3+h_3R+8g@mail.gmail.com>

On Thu, Apr 23, 2026 at 05:18:50PM -0400, D. Ben Knoble wrote:
> On Thu, Apr 23, 2026 at 2:55 AM Patrick Steinhardt <ps@pks.im> wrote:
> > On Wed, Apr 22, 2026 at 03:06:12PM -0400, D. Ben Knoble wrote:
> > > On Wed, Apr 22, 2026 at 6:30 AM Patrick Steinhardt <ps@pks.im> wrote:
> > > > diff --git a/Documentation/git-history.adoc b/Documentation/git-history.adoc
> > > > index 24dc907033..3cdfc8ba02 100644
> > > > --- a/Documentation/git-history.adoc
> > > > +++ b/Documentation/git-history.adoc
> > > > @@ -53,6 +55,19 @@ COMMANDS
> > > >
> > > >  The following commands are available to rewrite history in different ways:
> > > >
> > > > +`fixup <commit>`::
> > > > +       Apply the currently staged changes to the specified commit. The staged
> > > > +       changes are incorporated into the target commit's tree via a three-way
> > > > +       merge, using HEAD's tree as the merge base, which is equivalent to
> > > > +       linkgit:git-cherry-pick[1].
> > >
> > > I'm not quite sure what, as a user of "git history fixup," I'm
> > > supposed to take from this. Does it make conflicts less likely when
> > > creating the new fixup? I imagine it doesn't help with conflicts
> > > between <commit> and HEAD that newly arise.
> > >
> > > Anyway, I'd think the mechanics are less relevant than the end-user
> > > behavior at this point in the doc, unless the equivalence with
> > > cherry-pick is supposed to tell me something about that behavior.
> >
> > There's at least two more or less obvious variants to do this:
> >
> >   - You generate the diff between HEAD and index and then try to reapply
> >     the patch on top of the target commit.
> >
> >   - You perform the three-way merge.
> >
> > The second item is definitely more robust compared to generating the
> > diff and reapplying it, and we use the exact same strategy to perform
> > cherry-picks nowadays.
> >
> > > > diff --git a/builtin/history.c b/builtin/history.c
> > > > index 549e352c74..6299f0dfa9 100644
> > > > --- a/builtin/history.c
> > > > +++ b/builtin/history.c
> > [snip]
> > > > +       /*
> > > > +        * Perform the three-way merge to reapply changes in the index onto the
> > > > +        * target commit. This is using basically the same logic as a
> > > > +        * cherry-pick, where the base commit is our HEAD, ours is the original
> > > > +        * tree and theirs is the index tree.
> > > > +        */
> > >
> > > OTOH, this explanation helps quite a bit here :)
> >
> > Hm, okay. I felt that this explanation here is even more technical. How
> > about:
> >
> >     `fixup <commit>`::
> >         Apply the currently staged changes to the specified commit. This
> >         is done by performing a three-way merge between the HEAD commit,
> >         the target commit and the tree generated from staged changes.
> >         This is using the same logic as linkgit:git-cherry-pick[1].
> >
> > Not sure that this is an improvement? Happy to hear other suggestions.
> >
> > Thanks!
> >
> > Patrick
> 
> Hm. I think what I meant is that the in-code comment makes sense to
> describe internals; for users, I'm not sure what I should get out of
> that description of fixup.
> 
> What I (think I) really care about is that it behaves a bit like `git
> rebase -i` with a "fixup" command (modulo conflicts). Especially since
> this is quite a bit more porcelain than plumbing, no?
> 
> Idk. If the 3-way merge is valuable to keep, maybe it belongs in a
> second paragraph just to push it out of the way of the primary
> description ("Apply the currently staged changes to the specified
> commit")?

Ah, that's what you're getting at! I totally misunderstood what you
wanted to say, this makes a lot more sense. How about this:

    `fixup <commit>`::
        Apply the currently staged changes to the specified commit. This
        is similar in nature to `git commit --fixup=<commit>` followed
        by `git rebase --autosquash <commit>~`. Changes are applied to
        the target commit by performing a three-way merge between the
        HEAD commit, the target commit and the tree generated from
        staged changes.

Maybe there should be a new paragraph before we start talking about the
technical details?

Thanks!

Patrick

  reply	other threads:[~2026-04-24  6:53 UTC|newest]

Thread overview: 21+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2026-04-22 10:28 [PATCH 0/2] builtin/history: introduce "fixup" subcommand Patrick Steinhardt
2026-04-22 10:28 ` [PATCH 1/2] builtin/history: generalize function to commit trees Patrick Steinhardt
2026-04-22 10:28 ` [PATCH 2/2] builtin/history: introduce "fixup" subcommand Patrick Steinhardt
2026-04-22 19:06   ` D. Ben Knoble
2026-04-23  6:55     ` Patrick Steinhardt
2026-04-23 21:18       ` D. Ben Knoble
2026-04-24  6:53         ` Patrick Steinhardt [this message]
2026-04-24 14:43           ` D. Ben Knoble
2026-04-22 18:18 ` [PATCH 0/2] " Tian Yuchen
2026-04-23  6:55   ` Patrick Steinhardt
2026-04-23 14:21 ` [PATCH v2 0/3] " Patrick Steinhardt
2026-04-23 14:21   ` [PATCH v2 1/3] replay: allow callers to control what happens with empty commits Patrick Steinhardt
2026-04-23 14:21   ` [PATCH v2 2/3] builtin/history: generalize function to commit trees Patrick Steinhardt
2026-04-23 14:21   ` [PATCH v2 3/3] builtin/history: introduce "fixup" subcommand Patrick Steinhardt
2026-04-27  5:53 ` [PATCH v3 0/3] " Patrick Steinhardt
2026-04-27  5:53   ` [PATCH v3 1/3] replay: allow callers to control what happens with empty commits Patrick Steinhardt
2026-05-12  4:51     ` Junio C Hamano
2026-04-27  5:53   ` [PATCH v3 2/3] builtin/history: generalize function to commit trees Patrick Steinhardt
2026-04-27  5:53   ` [PATCH v3 3/3] builtin/history: introduce "fixup" subcommand Patrick Steinhardt
2026-05-12  5:47   ` [PATCH v3 0/3] " Junio C Hamano
2026-05-12  6:41     ` Patrick Steinhardt

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=aesTeWQqMTFd4gy8@pks.im \
    --to=ps@pks.im \
    --cc=ben.knoble@gmail.com \
    --cc=git@vger.kernel.org \
    --cc=newren@gmail.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox