Re: support for git commit --amend/--signoff

[Dan Nicolaescu]

Stefan Monnier <address@hidden> writes:

>>>> We could have a log-edit-extra-flags function that computes a set of
>>>> extra flags, and pass those flags to vc-git-checkin.
>>> If amend is triggered from a new command (like vc-amend), then there's no
>>> need for any special support in log-edit.  We could easily store the
>>> relevant info in log-edit-callback or in vc-specific
>>> buffer-local variables.
>> The problem with that is that vc-amend does not integrate very well
>> with C-x v v
>
> That doesn't bother me too much.  I wish we could move further away from
> C-x v v since I find it doesn't really work for modern VCSes.

Not sure why you think so, but please don't try to take it away, it
has been in use for so long, and it's still very useful.

>> and it is very Git specific,
>
> Not sure about the "very", but in any case I don't see why that would
> argue for integration inside "commit" rather than for the addition of
> a separate command.

You can do C-x v v to commit one file, then C-x v v and switch ammend
on to add another file to the same commit.

Also --amend it's still a property of commit, it's not a different
command, why make it a different command in emacs?


>> and given the reluctance of some other VCs to change history, it does
>> not seem that it will be generalized.
>
> DaRCS already supports a more general form of Git's amend.  Bazaar is

No idea, we don't support DaRCS at all...

> unlikely to go there, tho, indeed (although they are not opposed to
> adding a more limited command to modify commit-comments).  Don't know
> about others.


>
>> More, it does not solve the problem of --signoff (which might be
>> adopted by other VCs).
>
> What's the problem with sign-off?  It seems this one fits perfectly well
> inside a "Signed-Off-By:" header.

Signed-Off-By: as an empty header does not work.  --signoff does not have any 
arguments.


>> Here's a solution that takes care of all problems and it is extensible.
>> I adds one line to log-edit.el and two to vc.el.
>> vc-git-log-edit-get-extra-flags-function does the work in vc-git.el
>
> Why add a var "log-edit-get-extra-flags-function" that's never used by
> log-edit?

It's used by vc-checkin, the main user of log-edit and by the modes derived 
from log-edit.
Do you have a better proposal?

> BTW, I think you can get what you want already without touching any of
> vc.el and log-edit.el by making vc-git-log-edit-toggle-amend tweak the
> log-edit-callback variable.

That sounds fragile and much harder to understand for someone trying
to develop vc.el 5 years from now ...


> Maybe adding an `extra-flags' arg to `checkin' is the better approach,
> but adding it everywhere just for the sake of Git doesn't sound too
> good, which is why I'm resisting it.

Why do you think that adding a top level vc-amend command that will
only be used by git better than adding an unused argument (for now, it
can become useful in the future) to other backends?

> Also as a user I'd *really* like to have a clear visual feedback about
> the fact that I'm amending something, which is another reason why having
> it inside the headers is an attractive direction.

For that in my local tree I have a minor mode instead of
vc-git-log-edit-toggle-amend, it shows "amend" in the modeline.