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

[Pierre-Yves David]

On 02/24/2016 05:23 PM, Yuya Nishihara wrote:
> 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.

We should either shelve it (put it in the shelve and remove it from the 
working copy) or not (not putting it in the shelve, not removing it from 
the working copy).

I'm not sure what to do about partial shelving. I don't thing the 
interactive UI have a way to select that yet (wink wink nudge nudge).
It probably make sense to not shelve the branch change unless no 
pattern/selection is used. So only on "bare" shelve.

Cheers,

-- 
Pierre-Yves David