[PATCH 2 of 2 RFCv2] commands: introduce stash command

Pierre-Yves David pierre-yves.david at ens-lyon.org
Sun Feb 17 18:03:18 CST 2013


On 15 févr. 2013, at 01:15, Idan Kamara wrote:

> # HG changeset patch
> # User Idan Kamara <idankk86 at gmail.com>
> # Date 1360886982 -7200
> # Node ID 06dd5eda17402c1e89a1aa212e340b146394fad8
> # Parent  d4c029076cf2213ad680a53dfd32b0886b2b7be0
> commands: introduce stash command

I'm very happy to see this happening. Having a sane and stable solution to shelve/unshelve is very important.

I not sure that putting that as a core command is the right things to do. I would expect the command to be introduced in an extension before getting into core.

> Stashes are unnamed (by default) and can be listed using --list and inspected
> with -s/--show.

Is there real usage for unnamed stash? don't we want to force the user to provide a minimal description ?

> Referring to stashes is done either by index (as shown in --list) or name (if
> one was given). If neither is specified, the most recent stash is chosen.

Most recent rules seems sane.

> A stash is saved as a regular secret commit in the repository

Note that once it is hidden, it does not needs to be secret. Its already excluded from exchanges.

> and is identified by a bookmark under a special namespace '.hg/stash/'.

Muh? what does stash as do do with bookmark? Can't you keep the label internally? (and expose them in revset if needed)

>  It is hidden from the user and isn't exchanged with other repositories.

> When a stash is popped or deleted, it is stripped from the repository or marked obsolete if obsolete is enabled.

If you use obsolescence markers, you have to ensure that people can recreate the "same stash" with a different commit hash.

In the same way it seems that you are storing stash commit as plain standard commit (including commit message content), this open the way to unfortunate collision between stash and standard changeset.

> It's currently not possible to stash when an mq patch is applied. Unless mq
> sometime in the future stops the use of strip (unlikely, mainly due to lack of
> interest in touching it), this won't change.

Your check is too weak. User can qimport stash parent and qpop them. You need stronger check in MQ

I'll do a more technical review later. Having smaller patches would help people wiling to review it.

Thanks again for working on this.

-- 
Pierre-Yves David



More information about the Mercurial-devel mailing list