[PATCH] shelve: adds restoring newly created branch (issue5048)

[Yuya Nishihara]

On Wed, 24 Feb 2016 13:01:50 +0900, FUJIWARA Katsunori wrote:
> At Mon, 22 Feb 2016 23:59:25 +0900,
> Yuya Nishihara wrote:
> > 
> > On Wed, 17 Feb 2016 14:17:11 +0100, liscju wrote:
> > > # HG changeset patch
> > > # User liscju <piotr.listkiewicz at gmail.com>
> > > # Date 1455067407 -3600
> > > #      Wed Feb 10 02:23:27 2016 +0100
> > > # Node ID da5e9a27089d31988dfd2d8cf62fc63805aa13d7
> > > # Parent  a036e1ae1fbe88ab99cb861ebfc2e4da7a3912ca
> > > shelve: adds restoring newly created branch (issue5048)
> > > 
> > > Before this patch shelve never preserved branch information,
> > > so after applying unshelve branch was the same as it was
> > > on working copy no matter in which branch shelve took place.
> > > 
> > > This patch makes shelve remember branch in which shelve takes
> > > place and restoring it in unshelve if shelve takes place
> > > in newly created branch. In other words, restoring information
> > > takes place when shelve is made on working copy that is
> > > prepared for a branch change.
> > 
> > New behavior makes sense. I did nitpick. ;)
> > 
> > > diff -r a036e1ae1fbe -r da5e9a27089d hgext/shelve.py
> > > --- a/hgext/shelve.py	Sun Feb 07 00:49:31 2016 -0600
> > > +++ b/hgext/shelve.py	Wed Feb 10 02:23:27 2016 +0100
> > > @@ -148,6 +148,8 @@
> > >              pendingctx = fp.readline().strip()
> > >              parents = [bin(h) for h in fp.readline().split()]
> > >              stripnodes = [bin(h) for h in fp.readline().split()]
> > > +            restorebranch = fp.readline().strip() == 'True'
> > > +            origshelvebranch = fp.readline().strip()
> > 
> > Can it be a single "branch-name-should-be-set" field? I feel the string 'True'
> > is a bit ugly.
> > 
> > > +        origshelvebranch = shelvectx.branch()
> > > +        restorebranch = shelvectx.branch() != shelvectx.parents()[0].branch()
> > 
> > We have a shortcut, shelvectx.p1()
> > 
> > > +        if restorebranch:
> > > +            repo.dirstate.setbranch(origshelvebranch)
> > 
> > Perhaps we'd better to say "marked working directory as branch %s" again.

I had to say that "marked working directory" should be warned only if the
working branch was actually changed.

> If user shelve part of changes (e.g. "hg shelve -i" or "hg shelve
> FILEPATTERN") with preparation for a branch change, which of behaviors
> below is reasonable for users ?
> 
>     branch  branch
>     after   after
>   # shelve  unshelve  which shelve belongs to B
>   - ------- --------- ---------------------------------
>   0   P      P        none (= current behavior)
>   1   P      B        only the first one (= this patch)
>   2   B(*1)  P(*2)    only the last one
>   3   B      B        all
>   - ------- --------- ---------------------------------
>     
>   - P: branch of the 1st parent of the working directory
>   - B: branch to be newly created at next commit
> 
>   *1: P, only if shelve saves all changes
>   *2: B, only if the last shelve is unshelved
> 
> If we choose #1 or #2, "marked working directory as branch %s" would
> be needed also at (or near) the end of "hg shelve", which clears
> preparation for a branch change. In this case, "reset branch to
> ORIGINAL" or so might be better.
> 
> BTW, #2 and #3 above are listed up just for discussion. Now, I don't
> have strong opinion on keeping branch change after shelving :-)

Ugh, good point. I have no concrete idea how partial shelve should handle
branch change, but it would make some sense to keep the branch change if
working directory is not clean after shelving.