Patchwork [5,of,6,RFC] destutil: show number of other branch heads, even if on a closed head

login
register
mail settings
Submitter Katsunori FUJIWARA
Date Feb. 24, 2016, 2:17 p.m.
Message ID <1f9b3d38e380b018cbe1.1456323433@feefifofum>
Download mbox | patch
Permalink /patch/13345/
State Accepted
Delegated to: Martin von Zweigbergk
Headers show

Comments

Katsunori FUJIWARA - Feb. 24, 2016, 2:17 p.m.
# HG changeset patch
# User FUJIWARA Katsunori <foozy@lares.dti.ne.jp>
# Date 1456322433 -32400
#      Wed Feb 24 23:00:33 2016 +0900
# Node ID 1f9b3d38e380b018cbe10ed1acee882c014e3763
# Parent  54683cfda6e09c5502ad133d5a023929531a6b8b
destutil: show number of other branch heads, even if on a closed head

Before this patch, bare "hg update" doesn't show number of other
non-closed branch heads, if working parent is on a closed branch head
at the end of updating. This might mislead user into overlooking other
non-closed branch heads.

This patch gets a list of all branch heads regardless of closed-ness
of it, and uses it to examine whether working parent is on a branch
heads or not.
Pierre-Yves David - Feb. 24, 2016, 11:16 p.m.
On 02/24/2016 03:17 PM, FUJIWARA Katsunori wrote:
> # HG changeset patch
> # User FUJIWARA Katsunori <foozy@lares.dti.ne.jp>
> # Date 1456322433 -32400
> #      Wed Feb 24 23:00:33 2016 +0900
> # Node ID 1f9b3d38e380b018cbe10ed1acee882c014e3763
> # Parent  54683cfda6e09c5502ad133d5a023929531a6b8b
> destutil: show number of other branch heads, even if on a closed head
>
> Before this patch, bare "hg update" doesn't show number of other
> non-closed branch heads, if working parent is on a closed branch head
> at the end of updating. This might mislead user into overlooking other
> non-closed branch heads.
>
> This patch gets a list of all branch heads regardless of closed-ness
> of it, and uses it to examine whether working parent is on a branch
> heads or not.

closed head should mostly disapear from the UI. I personnaly don't think 
this is a move in the right direction to include them in this warning again.

I'm unclear about your sentence:

 > if working parent is on a closed branch head
 > at the end of updating. This might mislead user into overlooking other
 > non-closed branch heads.

Can you elaborate on this scenario. What's happening, what is printed 
and why is this confusing for the user?
Katsunori FUJIWARA - Feb. 25, 2016, 6:48 p.m.
At Thu, 25 Feb 2016 00:16:04 +0100,
Pierre-Yves David wrote:
> 
> On 02/24/2016 03:17 PM, FUJIWARA Katsunori wrote:
> > # HG changeset patch
> > # User FUJIWARA Katsunori <foozy@lares.dti.ne.jp>
> > # Date 1456322433 -32400
> > #      Wed Feb 24 23:00:33 2016 +0900
> > # Node ID 1f9b3d38e380b018cbe10ed1acee882c014e3763
> > # Parent  54683cfda6e09c5502ad133d5a023929531a6b8b
> > destutil: show number of other branch heads, even if on a closed head
> >
> > Before this patch, bare "hg update" doesn't show number of other
> > non-closed branch heads, if working parent is on a closed branch head
> > at the end of updating. This might mislead user into overlooking other
> > non-closed branch heads.
> >
> > This patch gets a list of all branch heads regardless of closed-ness
> > of it, and uses it to examine whether working parent is on a branch
> > heads or not.
> 
> closed head should mostly disapear from the UI. I personnaly don't think 
> this is a move in the right direction to include them in this warning again.
> 
> I'm unclear about your sentence:
> 
>  > if working parent is on a closed branch head
>  > at the end of updating. This might mislead user into overlooking other
>  > non-closed branch heads.
> 
> Can you elaborate on this scenario. What's happening, what is printed 
> and why is this confusing for the user?

Main concern of this patch is not "count also closed heads as other
heads", but "show message about other branch heads even if parent is
on a closed head".

What about revised patch description below ?

====================
destutil: show message about other branch heads, even if on a closed head

Before this patch, bare "hg update" displays message below, if there
is at least one non-closed branch head other than current parent:

  1. 'XX other heads for branch "BRANCH"' message, if current parent is
     on a non-closed branch head

     This suggests user to invoke "hg heads" or so for merging them.

  2. no message, if current parent is on a closed branch head

     At this patch, bare "hg update" might choose closed branch head
     as update destination, and it causes this situation easily.

  3. no message, otherwise (= current parent isn't on any branch head)

'XX other heads for branch "BRANCH"' should be displayed also in #2
case above, because user might overlook other non-closed branch heads.

This patch gets a list of all branch heads regardless of closed-ness
of it, and uses it (= 'allheads') to distinguish #1/#2 from #3 above.
====================


> -- 
> Pierre-Yves David
> 

----------------------------------------------------------------------
[FUJIWARA Katsunori]                             foozy@lares.dti.ne.jp
Pierre-Yves David - Feb. 26, 2016, 9:55 p.m.
On 02/25/2016 07:48 PM, FUJIWARA Katsunori wrote:
>
> At Thu, 25 Feb 2016 00:16:04 +0100,
> Pierre-Yves David wrote:
>>
>> On 02/24/2016 03:17 PM, FUJIWARA Katsunori wrote:
>>> # HG changeset patch
>>> # User FUJIWARA Katsunori <foozy@lares.dti.ne.jp>
>>> # Date 1456322433 -32400
>>> #      Wed Feb 24 23:00:33 2016 +0900
>>> # Node ID 1f9b3d38e380b018cbe10ed1acee882c014e3763
>>> # Parent  54683cfda6e09c5502ad133d5a023929531a6b8b
>>> destutil: show number of other branch heads, even if on a closed head
>>>
>>> Before this patch, bare "hg update" doesn't show number of other
>>> non-closed branch heads, if working parent is on a closed branch head
>>> at the end of updating. This might mislead user into overlooking other
>>> non-closed branch heads.
>>>
>>> This patch gets a list of all branch heads regardless of closed-ness
>>> of it, and uses it to examine whether working parent is on a branch
>>> heads or not.
>>
>> closed head should mostly disapear from the UI. I personnaly don't think
>> this is a move in the right direction to include them in this warning again.
>>
>> I'm unclear about your sentence:
>>
>>   > if working parent is on a closed branch head
>>   > at the end of updating. This might mislead user into overlooking other
>>   > non-closed branch heads.
>>
>> Can you elaborate on this scenario. What's happening, what is printed
>> and why is this confusing for the user?
>
> Main concern of this patch is not "count also closed heads as other
> heads", but "show message about other branch heads even if parent is
> on a closed head".
>
> What about revised patch description below ?

I'm either much more awake or this is much clearer. I've updated the 
description and pushed the result to the clowncopter.

Thanks.

Lets chat about patch 6 now ;-)

Patch

diff --git a/mercurial/destutil.py b/mercurial/destutil.py
--- a/mercurial/destutil.py
+++ b/mercurial/destutil.py
@@ -358,9 +358,10 @@  def _statusotherbook(ui, repo):
 
 def _statusotherbranchheads(ui, repo):
     currentbranch = repo.dirstate.branch()
+    allheads = repo.branchheads(currentbranch, closed=True)
     heads = repo.branchheads(currentbranch)
-    if repo.revs('%ln and parents()', heads):
-        # we are on a head
+    if repo.revs('%ln and parents()', allheads):
+        # we are on a head, even though it might be closed
         otherheads = repo.revs('%ln - parents()', heads)
         if otherheads:
             ui.status(_('%i other heads for branch "%s"\n') %
diff --git a/tests/test-update-branches.t b/tests/test-update-branches.t
--- a/tests/test-update-branches.t
+++ b/tests/test-update-branches.t
@@ -175,6 +175,41 @@  Cases are run as shown in that table, ro
   parent=1
   M foo
 
+  $ cd ..
+
+Test updating with closed head
+---------------------------------------------------------------------
+
+  $ hg clone -U -q b1 closed-heads
+  $ cd closed-heads
+
+Test updating if at least one non-closed branch head exists
+
+if on the closed branch head:
+- updating is no-op
+- "N other heads for ...." message is displayed
+
+  $ hg update -q -C 3
+  $ hg commit --close-branch -m 6
+  $ norevtest "on closed branch head" clean 6
+  0 files updated, 0 files merged, 0 files removed, 0 files unresolved
+  1 other heads for branch "default"
+  parent=6
+
+Test updating if all branch heads are closed
+
+if on the closed branch head:
+- updating is no-op
+- "N other heads for ...." message isn't displayed
+
+  $ hg update -q -C 2
+  $ hg commit --close-branch -m 7
+  $ norevtest "all heads of branch default are closed" clean 6
+  0 files updated, 0 files merged, 0 files removed, 0 files unresolved
+  parent=6
+
+  $ cd ../b1
+
 Test obsolescence behavior
 ---------------------------------------------------------------------