Subject: Re: bug#79408: 31.0.50; VC commands for cherry-picking

From: Sean Whitton <[email-masked]>
Date: Tue, 09 Sep 2025 12:08:40 +0100

Hello,

On Mon 08 Sep 2025 at 02:56pm -04, Spencer Baugh wrote:

> This reminded me of how we don't have a way to do format-patch using vc;
> that's somewhat related, and would be nice to support.

We do!  'M-x vc-prepare-patch' is the interactive entry point.

> In fact, I wonder if perhaps:
>
> - a new format-patch backend function
>
> - a new apply-patch backend function (to take advantage of better
>   backend-specific merging)
>
> - maybe a fancier checkin-patch backend function
>
> Could replace the need for these specialized backend functions?
>
> (I think git-cherry-pick used to be implemented as format-patch + apply
> under the hood, years ago)

So, the generic code would be responsible for invoking format-patch,
then apply-patch, then checkin-patch, in that order, right?

A couple of tricky things:

- We would need a way to extract authorship information and then pass
  that into the improved checkin-patch.  An author's name and e-mail
  address is generic enough, but the representation of dates might be
  something highly backend-specific.  It's another couple of API
  functions.

- There might be conflicts, in both the sense of ordinary diff
  application conflicts, and for Git, issues with the staging area.
  (There is a whole pile of logic in vc-git-checkin for dealing with the
  staging area with a comment summarising some of the issues, and there
  are still broken edge cases, I think.)
  I'm thinking that if the underlying VCS is responsible for handling
  the whole operation, it will manage to complete the cherry-pick anyway
  in more situations than our generic code will be able to do.
  It will also be faster, which might matter when cherry-picking a big
  chunk of a branch.

Thinking about these sorts of issues, I am inclined to prefer
cherry-pick-specific API functions.  They're not hard for a backend
author to implement, and I don't think they preclude adding an
apply-patch and a fancier checkin-patch too for other purposes.

(In particular, your apply-patch is very interesting, I agree that might
be a nice addition for diff-mode.  Then 'C-c RET a' would call into the
VCS, sometimes.  That's probably a separate bug though.)

>> These are my proposed commands:
>>
>> vc-cherry-pick:
>>  - Gets the log message for REV with get-change-comment API function.
>>  - Gets the string to append with cherry-pick-comment API function.
>>  - Puts these together, starts a Log Edit session for the user to amend
>>    the message (e.g. for Git, doing C-c C-s to add a sign-off).
>>  - On C-c C-c, calls the cherry-pick API function for the backend to do
>>    the cherry-pick.
>>    Examples: vc-git-cherry-pick would invoke 'git cherry-pick' or
>>    'git revert' depending on whether REVERT.
>>    vc-hg-cherry-pick would invoke 'hg graft' or 'hg backout'.
>>
>>  Will have vc-checkin's COMMENT, INITIAL-CONTENTS arguments so calling
>>  from Lisp can skip the Log Edit session.
>>
>>  A prefix argument might toggle whether to append the cherry-pick
>>  comment.
>
> That sounds like a bit of a waste of the prefix argument, since the user
> could always just delete that comment.  I think we'd be better off just
> not having the prefix argument do anything for now, and maybe we'll find
> a better use in the future.

That works fine when cherry-picking a single commit, but if
cherry-picking multiple commits, we're thinking there'll no opportunity
to edit the commit message.  In my experience cherry-picking, I want the
cherry-pick comment often, but definitely not always.

Git changed their default for this at one point; -x used to be implicit,
now you have to specify it.

So unfortunately I think we do have to give up the prefix arg to this.

>> vc-undo-revision (vc-revert is taken):
>>  Same as vc-cherry-pick, except passes REVERSE non-nil at the
>>  appropriate points.  No prefix argument.
>
> Maybe we could name it vc-revert-revision?
>
> Actually maybe we should name the commands something like:
>
> vc-revision-cherry-pick
> vc-revision-cherry-pick-apply
> vc-revision-revert
> vc-revision-revert-apply
>
> To display more directly how they're closely related and symmetric.

Yes, those are much better, thank you.

(I would just say vc-revision-cherry-apply over
vc-revision-cherry-pick-apply, though.)

>> vc-cherry-apply:
>>  The advantage of having a backend API function for this is that the
>>  backend can use its full merging logic.
>>  A generic vc-default-cherry-apply can be implementated similarly to
>>  vc-apply-to-other-working-tree.
>
> Right.  I assume that generic implementation would use the 'diff backend
> function.  And the sole disadvantage of such a generic implementation is
> that it would have less smart merging.

It would also be slower, probably, but for cherry-applying instead of
cherry-picking, that's less important, because you are probably applying
changes from fewer revisions.

>> vc-undo-apply:
>>  Like vc-cherry-apply except passes REVERSE non-nil at the appropriate
>>  point.  Similarly there can be a vc-default-reverse-cherry-apply.
>
> That sounds good, but...
>
> I wonder if we even need to support vc-cherry-apply and vc-undo-apply?
> At least right now.
>
> Two very different reasons why we might not want those:
>
> - Those seem primarily useful when you want to actually edit the diff
>   before committing.  But, it seems to me that using diff-mode for such
>   an operation is already pretty natural, and should be encouraged.
>
>   The way I do those two operations today in hg is:
>   - Open up vc-print-log starting at some revision
>   - Find the revision I want to revert or cherry-apply
>   - Hit d or D to open up the diff
>   - Use diff-mode to do the editing
>
>   Perhaps that is sufficient?

I think you're right.  Let's leave those two out for now.

> - In a completely different direction: what if we only supported
>   cherry-pick, but also added support for git reset --soft?  In git, I
>   often do cherry-apply by just cherry-picking and then resetting.  This
>   is a bit weird, but it might save us some complexity if we want to
>   support git reset --soft anyway?

There are a number of convenient workflows that I've used, and seen
people discuss, which rely on creative uses of 'git reset --soft'.
So I think it would be a very welcome addition.  It also means one could
immediately use these workflows and tricks with other VCS without
learning about what the equivalent operation is, because Emacs would
know.

In generic VC terms, we could have an operation that deletes revisions
from the end of the branch, and a version of that operation that deletes
the revisions without removing the changes.  The former would be
'git reset --hard' and the latter would be 'git reset --soft'.

We already have vc-allow-rewriting-published-history which we can use to
protect the user from getting themselves into an inconsistent state by
means of these operations.

I think we want to bake in the end-of-current-branch part.
Here is what I mean.  If we call it "vc-delete-revision", then it
invites the question as to whether you can delete revisions that are not
at the end of the branch.  But that's a completely different operation:
it's a rebase, not merely a reset.  It's much less VCS-generic how such
a thing could work.  So I propose to exclude it.

So I think we could call this something like

- vc-delete-back-to-revision
- vc-undo-checkins
- vc-undo-checkins-back-to
- vc-strip-revisions-back-to

?

-- 
Sean Whitton