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

[Pierre-Yves David]

On Mon, Feb 06, 2012 at 04:36:15PM +0100, Jason Harris wrote:
> > 
> > 
> > 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 agree with Idan here. '--amend' on commit to me is the obvious place for the
> simple amend operation. (For comparison it's in the commit dialog for MacHg
> where I added this check box), also in git it's a straight forward '--amend'
> argument to git commit.
> 
> (Also, you objecting to this is somewhat surprising since you have just finished
> adding a single command "phase" which does two radically different things
> sets-the-phase and reports-the-phase depending soley on options. So here the
> small semantic tweak of amend versus commit being an option is comparable
> trivial.)

The phase command "read" or "write" a value

`commit` "append" history, `amend` "alter" history. You can not really compare
the two.

More argument in my reply to idank.


> > > 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.
> 
> As for wether "amend" is an extension or not, I would be tempted to say most
> things even say your recent "phases" should likely be extensions until all the
> kinks have been worked out. Once something is an extension is really the first
> time that lots of people are going to use it / experiment with it. Then once the
> kinks are worked out move it into core. So having --amend as an extension for
> say 3 months, would be fine. It also might make sense to have it as an extension
> until such time as your concept / proposal of "obsolete" is fully integrated...

It was not viable to have the "phases" concept optional because all client need
to collaborate on the matter. There is enough trouble with old
client//new-client interaction about phases not to add some with
new-client//new-client interaction.

The same argument will apply for obsolete relations. (But not for history
rewriting issue)

Argument for keeping amend out of core for now are given in my replay to idank.


-- 
Pierre-Yves David

http://www.logilab.fr/

-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 198 bytes
Desc: Digital signature
URL: <http://selenic.com/pipermail/mercurial-devel/attachments/20120206/218b424c/attachment.pgp>