[PATCH RFC] commit: add --amend option to amend the parent changeset

[Jason Harris]

On Feb 13, 2012, at 7:44 PM, Pierre-Yves David wrote:

> Last week discussion on IRC and recent email convince me to argue event
> stronger to use a dedicated command.

(Sorry for the late reply (I just had knee surgery after tearing my ACL after skiing))

> The main argument to add an --amend switch on commit is "This is a simple
> solution to a simple problem". But I don't feel we can stop there. I strongly
> believe a big part of mercurial success account for:
> 
>    1) we provide simple solution to simple problem,
>    2) we provide powerful to complex problem (and as simple as possible),
>    3) we provide the two options in a consistent way.
> 
> Of course iterative work is a good idea. We should do "(1) simple solution"
> before doing "(2) powerful solution" but to ensure "(3) consistency" we need to
> Think about (2) before freezing (1).


But what is inconsistent about having an '--amend' option?

> Let's go back to our amend usecase:
> 
> (1) We want a simple and clean alternative to:
> 
>    $ hg commit
>    $ touch <file>
>    $ hg rollback
>    $ hg commit -m "the same message I preserved somehow"
> 
>    You suggest to introduce.
> 
>    $ hg commit
>    $ touch <file>
>    $ hg commit --amend
> 
>    MISSION ACCOMPLISHED. We have a simple solution a simple problem.

Exactly. Or
   'hg commit --amend --date ...'
 or
   'hg commit --amend --message "..."'
etc...

>    But what about the futur?
> 
> (2) But this new feature is a small step in the direction of a critical history
>    rewriting operation: "update a changeset". In other words you are moving
>    toward a full featured emplacement to mq's qrefresh.

Maybe, but it is still a very useful step, and one I use every day. (There is an
amend check box in the MacHg commit sheet)

>    To easily solve more situation (and send qrefesh back in the abyss)

I don't think we can deprecate qrefresh in any case. I think it will still be around
for a long time to come.

>  you'll
>    probably just need an handful of additional switches. And you **will** have to
>    add them as some point. Plug the amend machinery on commit mean that
>    those additional switches need to be added to commit too. This is bad idea and
>    several useful switches will probably be rejected not to clutter commit.
> 
>    Here is some-example of switches.
> 
>    /!\ You do not need to read them if you are already convinced that a new
>    /!\ behavior need new variant.
> 
> 
>    --edit      Update the commit message.

Wait, commit already has the --message option. 

>                Spawning an editor every time you want to refresh a changeset
>                content seems wrong to me. I'm using qrefresh and hg amend far
>                very often. This need to be a fast a cheap operation. It should
>                not requires you to fight with your editor everytime.
> 
>                Even if spawning an editor is picked as the default. I expect a
>                --keep-message switch to appear.

Well the default would be to keep the same message. Often one doesn't really
want to have "Opps, correct the loop termination". "Opps, include the right
file", "Opps, fix increment." sorts of messages and you just want the main
message the commit already had so commit --amend would not spawn a new editor
and just keep the old one. If you explicitly give it a message then it would use
that.

>                Such flag are -required- for amend  but do not make sense for
>                commit.
> 
>                (git have this on commit mainly for amend purpose)
> 
>    -D and -U   qrefresh have option to update the date of a commit to the
>                current one and to get the ownership a refreshed commit. People
>                will want to keep them when moving away from mq. But I'm pretty
>                sure they won't make it as commit switch.
> 
>                (git have a --reset-author switch on commit for this purpose)

If you really want you can move to mq if you want this, but the default should
almost always be to update the time stamp and keep the same author.

How many times would you not want this for the "Opps, fix variable scope."
sort of message?

>    --rebase    Once amending a changeset with children will be valid (when
>                obsolete is done), people will want a "all in one" operation
>                that both amend the changeset and resync descendant changesets
>                (think about pull --rebase).
> 
>                Such switch don't make sense on commit (or sense I do not want
>                to encourage)

Although this could be very nice, once you do this you have to have merge
machinery. So now there would be 4 commands (I might be missing others) that
need merge machinery : merge, rebase, graft, amend... Hmmm I think keeping this
list of commands as short as possible would be a good thing, it centralizes the
merging machinery which can be tricky to a single command.

For this, fairly uncommon case when you want to amend something which is not a
head you can very easily already do this with a rebase, followed by a collapse.

(Collapse would need to know about merge changesets which I don't think it does
at the moment, but that should be added to collapse in any case.)

>                (note: amending a changeset with children may motivate a
>                --force switch.)
> 
>                (git is unable to handle such detached commit situation)
> 
>    --force     amend changeset with descendant (see above)
> 
>    --???       option to exclude content already present in previous commit version

Can be already done by lots of other means.

>    --note      This is expected to be introduce by obsolete for every history
>                rewriting operation. It will be a comment of why a changeset is
>                rewritten. The data is stored in obsolete internal and available
>                for people who tries to understand what is happening.
> 
>                Commit is not a rewriting operation an will not get this option.
> 
>                (git does not have concept near the obsolete one)

I wouldn't use this for a "Opps, fix scope of variable i". It would be stupid to
do so. What would the note say: "uhhmmm, I changed the history so I could fix
the scope of the variable i". The whole point of rewriting the history is to
take "stuff out of the historical record" / group things in such a way the the
history is changed, since the user has decided it is not worth cluttering the
historical record with this information. (Or explicitly wants to remove the
nuclear launch codes and doesn't really want people to see that they included
the nuclear launch codes in the first place...)

(Having this part of an "obsolete" concept where you are altering changes which
have already been "published" is a different matter.)


>    I'll stop there. I think this example corpus is convincing enough that
>    amend operation will eventually need dedicated flags that does not belong to
>    commit.

At the worst case we can add an amend command which does all this at a later
stage but it would just seem to duplicate rebase followed by collapse, for the
complicated cases and commit --amend for the simple cases right?


> (3) As explained explained above, the amend concept **will** most certainly need
>    additional option and flagging. If amend operation is available through a
>    commit flag we have two solution to extend it:
> 
>    A) Add flag to commit which are not related to what commit does.

For the normal standard cases, no extra flags are needed.

>    B) Introduce a new command that finish the job.

I don't think we need the amend command as a separate command at all. The task
can easily be done with rebase followed by collapse, or if you need to get
tricky in rare cases with lots of other commands, rebase, collapse, mq,
import / export, qrecord, etc.


>   Both solutions leads to inconsistent UI.
> 
>    I do not think we can elude the issue now as this **will** happen. That is
>    why I advocate a "do it right the first time" approach: Adding a new command
>    for amend from the beginning.

As for being inconsistent, having more than one way to do things doesn't always
make things inconsistent at all. Eg rebase, can be done through export / import, or
mq. Does that make rebase inconsistent? Not at all. Rebase is incredibly
convenient for doing what it does. The same would be true of commit --amend to
add the "Opps, fix small stupid goof here." which programmers are doing tens to
hundreds of times a day. There are lots of other commands which are convenience
ones, hg summary instead of log, status, and branches, and tags. Having a
convenience option is a fine thing if it fit's in with the common workflow, and
the --amend perfectly fits the extremely common case of the "oops, fix the XYZ"
sort of commit.

From the users, perspective they really won't care at all that it changed
"history". They just want to be able to add the quick fix and get on with
whatever they were doing... Thus i don't think we should have the interface
strictly grouped around the type of internal operation that is occurring. The
user doesn't care.

Adding the --amend option now to commit will not preclude anything whatsoever...
I have had the option in MacHg to amend commits from the commit sheet for over a
year now (amend was introduced in MacHg 0.9.10 (November 2010), cf the current
released MacHg is 0.9.25). Anyway having the amend option has worked out really
well. (I have of course had to implement it through mq) but from the user
interface it simply amends the last commit. 

Cheers,
   Jason