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

[Idan Kamara]

On Mon, Feb 6, 2012 at 8:15 PM, Pierre-Yves David
<pierre-yves.david at logilab.fr> wrote:
>
> On Mon, Feb 06, 2012 at 04:08:39PM +0200, Idan Kamara wrote:
> > On Mon, Feb 6, 2012 at 2:41 PM, Pierre-Yves David <
> > pierre-yves.david at logilab.fr> wrote:
> >
> > >
> > > Before commenting on the RFC itself, I would like to point that we also
> > > have an implementation of an amend command here:
> > >
> > >
> > > http://hg-dev.octopoid.net/hgwebdir.cgi/mutable-history/file/64d16f07d67f/hgext/evolution.py#l277
> > >
> > > This implemenentation handle several cases including "commit message
> > > rewritting" (including when no change found) and "altering branch". You
> > > probably want to have a look at it.
> > >
> > >
> > > Moreover, I do feel we better work on having obsolete done before putting
> > > new history rewriting operation in core. History rewriting operation
> > > usually have to work around common issue that obsolete could handle in a
> > > common and graceful way (eg: amend changeset with children). Having
> > > obsolete documented and done will help us to shape a consistent UI for
> > > history rewriting operation too.
> > >
> >
> > I look forward to discussions on this concept but I don't think it is
> > mutually exclusive with this patch (and by the looks of it, they even share
> > some logic).
>
> I agree their are not mutally exclusive. But they better be integrated.
>
> > Like I said in the commit message, this new option to 'hg commit' is trying
> > to solve a simple problem with a simple solution. From the (little)
> > documentation I've read about 'obsolete', using it for this particular
> > problem feels like killing a bug with an Ion Cannon.
>
> Obsolete relation, is more about getting a solid ground than building a Ion
> Cannon. (Ui to evolve changeset might be seen as a Ion Cannon)
>
> One important part of my replay was
>
>    """help us to shape a consistent UI for history rewriting operation"""
>
> Solving the simple problem with simple solution is fine, making sure we build a
> consistent set of solutions that provide simple solution to simple issue and
> simplest as possible solution to more complexe issue is important too.

I'm not sure what you're talking about here.

>
> I'm pretty sure we will have a way to "amend" changeset at the end. I just want
> to make sure the details of the "amend" are consistent with the rest of
> mercurial. As I do not feel able to get it right at first pass and while
> solving partial probleme, I advocate to move cautiously.
>
> > > Anyway, I won't bame people working on that front but I'll frown to any
> > > "best effort solution" that will get into the way of graceful solution
> > > using obsolete changeset.
> > >
> >
> > What's not graceful about this?
>
> Here, I target solution related to using amend on changeset with children. In
> general we need to be careful not to lay down backward compatilibity trap for
> later improvement. This does not means I againts any movement forward. This means
> I think we should be careful and not rush.
>
> > > I made minor update to the wiki page related to the obsolete concept for
> > > those who missed previous discussion on the subject[1]. I'll write more
> > > details down in the comming weeks.  I expects the obsolete concepts to be
> > > discussed this cycle (2.2) and implemented the next (2.3)
> > >
> > > [1] http://mercurial.selenic.com/wiki/ChangesetEvolution
> > >
> > >
> > > On Thu, Feb 02, 2012 at 02:35:03PM +0200, Idan Kamara wrote:
> > > > # HG changeset patch # User Idan Kamara <idankk86 at gmail.com> # Date
> > > > 1328185747 -7200 # Branch stable # Node ID
> > > > cc7dec00d5016f03acf789c35ce6ef50b204f0cb # Parent
> > > > 0620421044a2bcaafd054a6ee454614888699de8 commit: add --amend option to
> > > > amend the parent changeset
> > >
> > > I do not think this should be a flag to commit. "hg commit" is about adding
> > > new changeset to the history. Your RFC is about rewriting a changeset. I
> > > would use a dedicated command "hg amend" for this purpose.
> > >
> >
> > I actually like it being on commit, it's the obvious place. It doesn't
> > require "learning" a new command, which in this case has the exact same flags
> > too.
>
> I'm not very convinced by the "exact same flag" argument. There are multiple
> commands with commit option:
>
>    * commit

Yes, commit has the same flags as commit :).

>    * backout
>    * graft
>    * import
>    * tag
>
>    * qnew
>    * qrecord

They might have some of the same flags, but none of them
share anything with commit as amend does.

>
> Moreover can think about several flag that are amend only:
>
> Will exist:
>    --edit    edit commit message.

Should happen automatically when a message isn't specified on
the command line.

>    -U        update commit with current user
>    -D        update date with current date

These are kind of useful.

>    --<??>    option to exclude content already present in previous commit version

What is the use case for this?

>
> Probably useful
>    -c --change REV          specifies the changeset to amend (allow to amend multiple commit in one, afterward)

Doesn't really fit the 'simple problem, simple solution'.

>    -n --note VALUE          use text as commit message for this update (audit purpose)

Not sure what this is meant to do.

>
>
> commit and amend are different actions and should be different command;
>
> * This avoids newbies findind the --amend switch in "hg help commit"
>  misunderstand it and shooting themself in the foot.
>
> * This avoids command with a soups of 25 options and borked semantics (git rebase -i vs hg histedit)

I haven't added any flags to commit other than --amend.

>
> * This helps wrapping and aliasing around the amend concept.

There was a bullet in my todo list to refactor the amend logic to a
cmdutil.amend() function
(or other suitable place).

>
>
>
> > > > Commits a new changeset incorporating both the changes to the given files
> > > > and all the changes from the current parent changeset into the repository.
> > > >
> > > > You cannot amend public changesets. Amending a changeset with children
> > > > results in a warning (we might want to forbid this).
> > > >
> > > > Behind the scenes, first commit the update as a regular child of the current
> > > > parent. Then create a new commit on the parent's parents with the updated
> > > > contents. Then change the working copy parent to this new combined changeset.
> > > > Finally, strip the intermediate commit created in the beginning (might
> > > > want to also strip the amended commit if it has no children).
> > >
> > > I do not think we should make this part of core yet. Core is currently free
> > > of history rewriting stuff and this look like a good idea. I would put that
> > > in an extension until we have alternative to strip in core. In a general
> > > manner I will be overcautious about setting history rewriting operation
> > > into core store until we have more hindsight.
> > >
> >
> > I like extensions. I think some of them are great. But if the average user
> > has 5-10 extensions enabled, then that's bad. This feels like something the
> > average user would want in his toolbox.
> >
> > If X out of Y users (where X is say Y/2) who come to IRC ask "how do I do Z
> > in Mercurial?" and the answer is "use extension foo", then foo shouldn't be
> > an extension.
>
> I'm not saying this should never be in core. But I think this should
> start being and extension. In particular until we have safer mechanism than
> strip to mark replaced changeset.
>
> Experimenting outside core is a good way to make thing available while keeping
> space to tune the UI with feedback.
>
> I believe we can wait a few months before making it part of core.

I don't mind if others think this is necessary.

Anyway, to sum this RFC up, it looks like there's no real support for this,
so I'm not going to finish this until others voice their opinion.