Patchwork D6282: branch: abort if closing branch from a non-branchhead cset

login
register
mail settings
Submitter phabricator
Date April 19, 2019, 7:49 a.m.
Message ID <differential-rev-PHID-DREV-4lbhese3lkncnt5btjf6-req@phab.mercurial-scm.org>
Download mbox | patch
Permalink /patch/39761/
State Superseded
Headers show

Comments

phabricator - April 19, 2019, 7:49 a.m.
khanchi97 created this revision.
Herald added a subscriber: mercurial-devel.
Herald added a reviewer: hg-reviewers.

REVISION SUMMARY
  This patch make sure that we abort if the user is trying to
  close a branch from a cset which is not a branch head.
  Changes in test file reflect the fixed behaviour.

REPOSITORY
  rHG Mercurial

REVISION DETAIL
  https://phab.mercurial-scm.org/D6282

AFFECTED FILES
  mercurial/commands.py
  tests/test-branches.t

CHANGE DETAILS




To: khanchi97, #hg-reviewers
Cc: mercurial-devel
Pierre-Yves David - April 26, 2019, 2:56 p.m.
On 4/19/19 9:49 AM, khanchi97 (Sushil khanchi) wrote:
> khanchi97 created this revision.
> Herald added a subscriber: mercurial-devel.
> Herald added a reviewer: hg-reviewers.
> 
> REVISION SUMMARY
>    This patch make sure that we abort if the user is trying to
>    close a branch from a cset which is not a branch head.
>    Changes in test file reflect the fixed behaviour.

Is there a way to override this ? There can be situation where the 
changeset is not a local head but could be a remote head.

> 
> REPOSITORY
>    rHG Mercurial
> 
> REVISION DETAIL
>    https://phab.mercurial-scm.org/D6282
> 
> AFFECTED FILES
>    mercurial/commands.py
>    tests/test-branches.t
> 
> CHANGE DETAILS
> 
> diff --git a/tests/test-branches.t b/tests/test-branches.t
> --- a/tests/test-branches.t
> +++ b/tests/test-branches.t
> @@ -956,19 +956,16 @@
>   
>   trying to close branch from a cset which is not a branch head
>   it should abort:
> -XXX: it should have aborted here
>     $ hg ci -m "closing branch" --close-branch
> -  created new head
> +  abort: can only close branch heads
> +  [255]
>   
>     $ hg up 0
>     0 files updated, 0 files merged, 1 files removed, 0 files unresolved
>     $ hg log -GT "{rev}: {node|short} {desc|firstline}\n\t{branch}\n\n"
> -  _  3: 006876ddd20e closing branch
> +  o  2: 155349b645be added c
>     |  	default
>     |
> -  | o  2: 155349b645be added c
> -  |/   	default
> -  |
>     o  1: 5f6d8a4bf34a added b
>     |  	default
>     |
> diff --git a/mercurial/commands.py b/mercurial/commands.py
> --- a/mercurial/commands.py
> +++ b/mercurial/commands.py
> @@ -1675,6 +1675,8 @@
>   
>           if not bheads:
>               raise error.Abort(_('can only close branch heads'))
> +        elif branch == repo['.'].branch() and repo['.'].node() not in bheads:
> +            raise error.Abort(_('can only close branch heads'))
>           elif opts.get('amend'):
>               if (repo['.'].p1().branch() != branch and
>                   repo['.'].p2().branch() != branch):
> 
> 
> 
> To: khanchi97, #hg-reviewers
> Cc: mercurial-devel
> _______________________________________________
> Mercurial-devel mailing list
> Mercurial-devel@mercurial-scm.org
> https://www.mercurial-scm.org/mailman/listinfo/mercurial-devel
>
sushil khanchi - May 1, 2019, 11:14 a.m.
.

On Fri, Apr 26, 2019 at 8:29 PM Pierre-Yves David <
pierre-yves.david@ens-lyon.org> wrote:

>
>
> On 4/19/19 9:49 AM, khanchi97 (Sushil khanchi) wrote:
> > khanchi97 created this revision.
> > Herald added a subscriber: mercurial-devel.
> > Herald added a reviewer: hg-reviewers.
> >
> > REVISION SUMMARY
> >    This patch make sure that we abort if the user is trying to
> >    close a branch from a cset which is not a branch head.
> >    Changes in test file reflect the fixed behaviour.
>
> Is there a way to override this ? There can be situation where the
> changeset is not a local head but could be a remote head.
>
By closing remote head, you mean first close that locally (where it is not
a head) and push that to a remote (where it is a head)?


> >
> > REPOSITORY
> >    rHG Mercurial
> >
> > REVISION DETAIL
> >    https://phab.mercurial-scm.org/D6282
> >
> > AFFECTED FILES
> >    mercurial/commands.py
> >    tests/test-branches.t
> >
> > CHANGE DETAILS
> >
> > diff --git a/tests/test-branches.t b/tests/test-branches.t
> > --- a/tests/test-branches.t
> > +++ b/tests/test-branches.t
> > @@ -956,19 +956,16 @@
> >
> >   trying to close branch from a cset which is not a branch head
> >   it should abort:
> > -XXX: it should have aborted here
> >     $ hg ci -m "closing branch" --close-branch
> > -  created new head
> > +  abort: can only close branch heads
> > +  [255]
> >
> >     $ hg up 0
> >     0 files updated, 0 files merged, 1 files removed, 0 files unresolved
> >     $ hg log -GT "{rev}: {node|short} {desc|firstline}\n\t{branch}\n\n"
> > -  _  3: 006876ddd20e closing branch
> > +  o  2: 155349b645be added c
> >     |         default
> >     |
> > -  | o  2: 155349b645be added c
> > -  |/         default
> > -  |
> >     o  1: 5f6d8a4bf34a added b
> >     |         default
> >     |
> > diff --git a/mercurial/commands.py b/mercurial/commands.py
> > --- a/mercurial/commands.py
> > +++ b/mercurial/commands.py
> > @@ -1675,6 +1675,8 @@
> >
> >           if not bheads:
> >               raise error.Abort(_('can only close branch heads'))
> > +        elif branch == repo['.'].branch() and repo['.'].node() not in
> bheads:
> > +            raise error.Abort(_('can only close branch heads'))
> >           elif opts.get('amend'):
> >               if (repo['.'].p1().branch() != branch and
> >                   repo['.'].p2().branch() != branch):
> >
> >
> >
> > To: khanchi97, #hg-reviewers
> > Cc: mercurial-devel
> > _______________________________________________
> > Mercurial-devel mailing list
> > Mercurial-devel@mercurial-scm.org
> > https://www.mercurial-scm.org/mailman/listinfo/mercurial-devel
> >
>
> --
> Pierre-Yves David
> _______________________________________________
> Mercurial-devel mailing list
> Mercurial-devel@mercurial-scm.org
> https://www.mercurial-scm.org/mailman/listinfo/mercurial-devel
>
Pierre-Yves David - May 1, 2019, 11:44 a.m.
On 5/1/19 1:14 PM, Sushil Khanchi wrote:
> .
> 
> On Fri, Apr 26, 2019 at 8:29 PM Pierre-Yves David 
> <pierre-yves.david@ens-lyon.org <mailto:pierre-yves.david@ens-lyon.org>> 
> wrote:
> 
> 
> 
>     On 4/19/19 9:49 AM, khanchi97 (Sushil khanchi) wrote:
>      > khanchi97 created this revision.
>      > Herald added a subscriber: mercurial-devel.
>      > Herald added a reviewer: hg-reviewers.
>      >
>      > REVISION SUMMARY
>      >    This patch make sure that we abort if the user is trying to
>      >    close a branch from a cset which is not a branch head.
>      >    Changes in test file reflect the fixed behaviour.
> 
>     Is there a way to override this ? There can be situation where the
>     changeset is not a local head but could be a remote head.
> 
> By closing remote head, you mean first close that locally (where it is 
> not a head) and push that to a remote (where it is a head)?

Yes
sushil khanchi - May 1, 2019, 12:14 p.m.
.

On Wed, May 1, 2019 at 5:14 PM Pierre-Yves David <
pierre-yves.david@ens-lyon.org> wrote:

>
>
> On 5/1/19 1:14 PM, Sushil Khanchi wrote:
> > .
> >
> > On Fri, Apr 26, 2019 at 8:29 PM Pierre-Yves David
> > <pierre-yves.david@ens-lyon.org <mailto:pierre-yves.david@ens-lyon.org>>
>
> > wrote:
> >
> >
> >
> >     On 4/19/19 9:49 AM, khanchi97 (Sushil khanchi) wrote:
> >      > khanchi97 created this revision.
> >      > Herald added a subscriber: mercurial-devel.
> >      > Herald added a reviewer: hg-reviewers.
> >      >
> >      > REVISION SUMMARY
> >      >    This patch make sure that we abort if the user is trying to
> >      >    close a branch from a cset which is not a branch head.
> >      >    Changes in test file reflect the fixed behaviour.
> >
> >     Is there a way to override this ? There can be situation where the
> >     changeset is not a local head but could be a remote head.
> >
> > By closing remote head, you mean first close that locally (where it is
> > not a head) and push that to a remote (where it is a head)?
>
> Yes
>
I don't think in present code there is any way to override this. Do you
have any idea? Or any other place in mercurial you remember where we handle
similar situation?
And just for knowledge I would like to know how often developers use this
remote head closing thing (when locally it is not a head)?


> --
> Pierre-Yves David
>
Pierre-Yves David - May 1, 2019, 3:52 p.m.
.

On 5/1/19 2:14 PM, Sushil Khanchi wrote:
> .
> 
> On Wed, May 1, 2019 at 5:14 PM Pierre-Yves David 
> <pierre-yves.david@ens-lyon.org <mailto:pierre-yves.david@ens-lyon.org>> 
> wrote:
> 
> 
> 
>     On 5/1/19 1:14 PM, Sushil Khanchi wrote:
>      > .
>      >
>      > On Fri, Apr 26, 2019 at 8:29 PM Pierre-Yves David
>      > <pierre-yves.david@ens-lyon.org
>     <mailto:pierre-yves.david@ens-lyon.org>
>     <mailto:pierre-yves.david@ens-lyon.org
>     <mailto:pierre-yves.david@ens-lyon.org>>>
>      > wrote:
>      >
>      >
>      >
>      >     On 4/19/19 9:49 AM, khanchi97 (Sushil khanchi) wrote:
>      >      > khanchi97 created this revision.
>      >      > Herald added a subscriber: mercurial-devel.
>      >      > Herald added a reviewer: hg-reviewers.
>      >      >
>      >      > REVISION SUMMARY
>      >      >    This patch make sure that we abort if the user is trying to
>      >      >    close a branch from a cset which is not a branch head.
>      >      >    Changes in test file reflect the fixed behaviour.
>      >
>      >     Is there a way to override this ? There can be situation
>     where the
>      >     changeset is not a local head but could be a remote head.
>      >
>      > By closing remote head, you mean first close that locally (where
>     it is
>      > not a head) and push that to a remote (where it is a head)?
> 
>     Yes
> 
> I don't think in present code there is any way to override this. Do you 
> have any idea? Or any other place in mercurial you remember where we 
> handle similar situation?
> And just for knowledge I would like to know how often developers use 
> this remote head closing thing (when locally it is not a head)?

Something similar coming to mind is `hg tag`. It will -warn- when 
tagging a non-head but will still tag. Maybe we should align on this 
(warning instead of aborting)
sushil khanchi - May 1, 2019, 9:18 p.m.
.

On Wed, May 1, 2019 at 9:22 PM Pierre-Yves David <
pierre-yves.david@ens-lyon.org> wrote:

> .
>
> On 5/1/19 2:14 PM, Sushil Khanchi wrote:
> > .
> >
> > On Wed, May 1, 2019 at 5:14 PM Pierre-Yves David
> > <pierre-yves.david@ens-lyon.org <mailto:pierre-yves.david@ens-lyon.org>>
>
> > wrote:
> >
> >
> >
> >     On 5/1/19 1:14 PM, Sushil Khanchi wrote:
> >      > .
> >      >
> >      > On Fri, Apr 26, 2019 at 8:29 PM Pierre-Yves David
> >      > <pierre-yves.david@ens-lyon.org
> >     <mailto:pierre-yves.david@ens-lyon.org>
> >     <mailto:pierre-yves.david@ens-lyon.org
> >     <mailto:pierre-yves.david@ens-lyon.org>>>
> >      > wrote:
> >      >
> >      >
> >      >
> >      >     On 4/19/19 9:49 AM, khanchi97 (Sushil khanchi) wrote:
> >      >      > khanchi97 created this revision.
> >      >      > Herald added a subscriber: mercurial-devel.
> >      >      > Herald added a reviewer: hg-reviewers.
> >      >      >
> >      >      > REVISION SUMMARY
> >      >      >    This patch make sure that we abort if the user is
> trying to
> >      >      >    close a branch from a cset which is not a branch head.
> >      >      >    Changes in test file reflect the fixed behaviour.
> >      >
> >      >     Is there a way to override this ? There can be situation
> >     where the
> >      >     changeset is not a local head but could be a remote head.
> >      >
> >      > By closing remote head, you mean first close that locally (where
> >     it is
> >      > not a head) and push that to a remote (where it is a head)?
> >
> >     Yes
> >
> > I don't think in present code there is any way to override this. Do you
> > have any idea? Or any other place in mercurial you remember where we
> > handle similar situation?
> > And just for knowledge I would like to know how often developers use
> > this remote head closing thing (when locally it is not a head)?
>
> Something similar coming to mind is `hg tag`. It will -warn- when
> tagging a non-head but will still tag. Maybe we should align on this
> (warning instead of aborting)
>

How about prompting the user to confirm? One upside in prompting I see is
that user don't have to run `hg strip/prune`. Also the downside is it
requires user intervention.

>
> --
> Pierre-Yves David
>
Pierre-Yves David - May 5, 2019, 6:13 p.m.
On 5/1/19 11:18 PM, Sushil Khanchi wrote:
> .
> 
> On Wed, May 1, 2019 at 9:22 PM Pierre-Yves David 
> <pierre-yves.david@ens-lyon.org <mailto:pierre-yves.david@ens-lyon.org>> 
> wrote:
> 
>     .
> 
>     On 5/1/19 2:14 PM, Sushil Khanchi wrote:
>      > .
>      >
>      > On Wed, May 1, 2019 at 5:14 PM Pierre-Yves David
>      > <pierre-yves.david@ens-lyon.org
>     <mailto:pierre-yves.david@ens-lyon.org>
>     <mailto:pierre-yves.david@ens-lyon.org
>     <mailto:pierre-yves.david@ens-lyon.org>>>
>      > wrote:
>      >
>      >
>      >
>      >     On 5/1/19 1:14 PM, Sushil Khanchi wrote:
>      >      > .
>      >      >
>      >      > On Fri, Apr 26, 2019 at 8:29 PM Pierre-Yves David
>      >      > <pierre-yves.david@ens-lyon.org
>     <mailto:pierre-yves.david@ens-lyon.org>
>      >     <mailto:pierre-yves.david@ens-lyon.org
>     <mailto:pierre-yves.david@ens-lyon.org>>
>      >     <mailto:pierre-yves.david@ens-lyon.org
>     <mailto:pierre-yves.david@ens-lyon.org>
>      >     <mailto:pierre-yves.david@ens-lyon.org
>     <mailto:pierre-yves.david@ens-lyon.org>>>>
>      >      > wrote:
>      >      >
>      >      >
>      >      >
>      >      >     On 4/19/19 9:49 AM, khanchi97 (Sushil khanchi) wrote:
>      >      >      > khanchi97 created this revision.
>      >      >      > Herald added a subscriber: mercurial-devel.
>      >      >      > Herald added a reviewer: hg-reviewers.
>      >      >      >
>      >      >      > REVISION SUMMARY
>      >      >      >    This patch make sure that we abort if the user
>     is trying to
>      >      >      >    close a branch from a cset which is not a branch
>     head.
>      >      >      >    Changes in test file reflect the fixed behaviour.
>      >      >
>      >      >     Is there a way to override this ? There can be situation
>      >     where the
>      >      >     changeset is not a local head but could be a remote head.
>      >      >
>      >      > By closing remote head, you mean first close that locally
>     (where
>      >     it is
>      >      > not a head) and push that to a remote (where it is a head)?
>      >
>      >     Yes
>      >
>      > I don't think in present code there is any way to override this.
>     Do you
>      > have any idea? Or any other place in mercurial you remember where we
>      > handle similar situation?
>      > And just for knowledge I would like to know how often developers use
>      > this remote head closing thing (when locally it is not a head)?
> 
>     Something similar coming to mind is `hg tag`. It will -warn- when
>     tagging a non-head but will still tag. Maybe we should align on this
>     (warning instead of aborting)
> 
> 
> How about prompting the user to confirm? One upside in prompting I see 
> is that user don't have to run `hg strip/prune`. Also the downside is it 
> requires user intervention.

prompt are usually not a great user experience. I lean toward saying 
"lets have a user experience similar to `hg tag`". Alternatively, we 
could have a warning early. For example, if the commit message editor 
had a line about adding a new heads/closing non-head, this would be a 
clear win.
Matt Harbison - May 9, 2019, 3:23 a.m.
On Sun, 05 May 2019 14:13:16 -0400, Pierre-Yves David  
<pierre-yves.david@ens-lyon.org> wrote:

>
>
> On 5/1/19 11:18 PM, Sushil Khanchi wrote:
>> .
>>  On Wed, May 1, 2019 at 9:22 PM Pierre-Yves David  
>> <pierre-yves.david@ens-lyon.org  
>> <mailto:pierre-yves.david@ens-lyon.org>> wrote:
>>      .
>>      On 5/1/19 2:14 PM, Sushil Khanchi wrote:
>>      > .
>>      >
>>      > On Wed, May 1, 2019 at 5:14 PM Pierre-Yves David
>>      > <pierre-yves.david@ens-lyon.org
>>     <mailto:pierre-yves.david@ens-lyon.org>
>>     <mailto:pierre-yves.david@ens-lyon.org
>>     <mailto:pierre-yves.david@ens-lyon.org>>>
>>      > wrote:
>>      >
>>      >
>>      >
>>      >     On 5/1/19 1:14 PM, Sushil Khanchi wrote:
>>      >      > .
>>      >      >
>>      >      > On Fri, Apr 26, 2019 at 8:29 PM Pierre-Yves David
>>      >      > <pierre-yves.david@ens-lyon.org
>>     <mailto:pierre-yves.david@ens-lyon.org>
>>      >     <mailto:pierre-yves.david@ens-lyon.org
>>     <mailto:pierre-yves.david@ens-lyon.org>>
>>      >     <mailto:pierre-yves.david@ens-lyon.org
>>     <mailto:pierre-yves.david@ens-lyon.org>
>>      >     <mailto:pierre-yves.david@ens-lyon.org
>>     <mailto:pierre-yves.david@ens-lyon.org>>>>
>>      >      > wrote:
>>      >      >
>>      >      >
>>      >      >
>>      >      >     On 4/19/19 9:49 AM, khanchi97 (Sushil khanchi) wrote:
>>      >      >      > khanchi97 created this revision.
>>      >      >      > Herald added a subscriber: mercurial-devel.
>>      >      >      > Herald added a reviewer: hg-reviewers.
>>      >      >      >
>>      >      >      > REVISION SUMMARY
>>      >      >      >    This patch make sure that we abort if the user
>>     is trying to
>>      >      >      >    close a branch from a cset which is not a branch
>>     head.
>>      >      >      >    Changes in test file reflect the fixed  
>> behaviour.
>>      >      >
>>      >      >     Is there a way to override this ? There can be  
>> situation
>>      >     where the
>>      >      >     changeset is not a local head but could be a remote  
>> head.
>>      >      >
>>      >      > By closing remote head, you mean first close that locally
>>     (where
>>      >     it is
>>      >      > not a head) and push that to a remote (where it is a  
>> head)?
>>      >
>>      >     Yes
>>      >
>>      > I don't think in present code there is any way to override this.
>>     Do you
>>      > have any idea? Or any other place in mercurial you remember  
>> where we
>>      > handle similar situation?
>>      > And just for knowledge I would like to know how often developers  
>> use
>>      > this remote head closing thing (when locally it is not a head)?
>>      Something similar coming to mind is `hg tag`. It will -warn- when
>>     tagging a non-head but will still tag. Maybe we should align on this
>>     (warning instead of aborting)
>>   How about prompting the user to confirm? One upside in prompting I  
>> see is that user don't have to run `hg strip/prune`. Also the downside  
>> is it requires user intervention.
>
> prompt are usually not a great user experience.

Maybe require `--force` to override the abort?  I don't necessarily care  
for prompts either, but the tag behavior always struck me as weird- it  
warns, but it makes changes to the repo anyway and the stock hg  
configuration doesn't let you edit history to fix it.

> I lean toward saying "lets have a user experience similar to `hg tag`".  
> Alternatively, we could have a warning early. For example, if the commit  
> message editor had a line about adding a new heads/closing non-head,  
> this would be a clear win.

I typically don't notice what else is in the commit message editor, unless  
while writing the commit, I start to doubt what files I named.
sushil khanchi - May 17, 2019, 8:33 a.m.
I am +1 with adding the `--force` option to override the abort as it seems
the better option than
1) warning message (as it would change the repo anyway)
2) adding a message in commit editor. (hard to notice)

I will send a patch for it. Let me know if you have a different opinion on
this :-)

On Thu, May 9, 2019 at 8:53 AM Matt Harbison <mharbison72@gmail.com> wrote:

> On Sun, 05 May 2019 14:13:16 -0400, Pierre-Yves David
> <pierre-yves.david@ens-lyon.org> wrote:
>
> >
> >
> > On 5/1/19 11:18 PM, Sushil Khanchi wrote:
> >> .
> >>  On Wed, May 1, 2019 at 9:22 PM Pierre-Yves David
> >> <pierre-yves.david@ens-lyon.org
> >> <mailto:pierre-yves.david@ens-lyon.org>> wrote:
> >>      .
> >>      On 5/1/19 2:14 PM, Sushil Khanchi wrote:
> >>      > .
> >>      >
> >>      > On Wed, May 1, 2019 at 5:14 PM Pierre-Yves David
> >>      > <pierre-yves.david@ens-lyon.org
> >>     <mailto:pierre-yves.david@ens-lyon.org>
> >>     <mailto:pierre-yves.david@ens-lyon.org
> >>     <mailto:pierre-yves.david@ens-lyon.org>>>
> >>      > wrote:
> >>      >
> >>      >
> >>      >
> >>      >     On 5/1/19 1:14 PM, Sushil Khanchi wrote:
> >>      >      > .
> >>      >      >
> >>      >      > On Fri, Apr 26, 2019 at 8:29 PM Pierre-Yves David
> >>      >      > <pierre-yves.david@ens-lyon.org
> >>     <mailto:pierre-yves.david@ens-lyon.org>
> >>      >     <mailto:pierre-yves.david@ens-lyon.org
> >>     <mailto:pierre-yves.david@ens-lyon.org>>
> >>      >     <mailto:pierre-yves.david@ens-lyon.org
> >>     <mailto:pierre-yves.david@ens-lyon.org>
> >>      >     <mailto:pierre-yves.david@ens-lyon.org
> >>     <mailto:pierre-yves.david@ens-lyon.org>>>>
> >>      >      > wrote:
> >>      >      >
> >>      >      >
> >>      >      >
> >>      >      >     On 4/19/19 9:49 AM, khanchi97 (Sushil khanchi) wrote:
> >>      >      >      > khanchi97 created this revision.
> >>      >      >      > Herald added a subscriber: mercurial-devel.
> >>      >      >      > Herald added a reviewer: hg-reviewers.
> >>      >      >      >
> >>      >      >      > REVISION SUMMARY
> >>      >      >      >    This patch make sure that we abort if the user
> >>     is trying to
> >>      >      >      >    close a branch from a cset which is not a branch
> >>     head.
> >>      >      >      >    Changes in test file reflect the fixed
> >> behaviour.
> >>      >      >
> >>      >      >     Is there a way to override this ? There can be
> >> situation
> >>      >     where the
> >>      >      >     changeset is not a local head but could be a remote
> >> head.
> >>      >      >
> >>      >      > By closing remote head, you mean first close that locally
> >>     (where
> >>      >     it is
> >>      >      > not a head) and push that to a remote (where it is a
> >> head)?
> >>      >
> >>      >     Yes
> >>      >
> >>      > I don't think in present code there is any way to override this.
> >>     Do you
> >>      > have any idea? Or any other place in mercurial you remember
> >> where we
> >>      > handle similar situation?
> >>      > And just for knowledge I would like to know how often
> developers
> >> use
> >>      > this remote head closing thing (when locally it is not a head)?
> >>      Something similar coming to mind is `hg tag`. It will -warn- when
> >>     tagging a non-head but will still tag. Maybe we should align on this
> >>     (warning instead of aborting)
> >>   How about prompting the user to confirm? One upside in prompting I
> >> see is that user don't have to run `hg strip/prune`. Also the downside
> >> is it requires user intervention.
> >
> > prompt are usually not a great user experience.
>
> Maybe require `--force` to override the abort?  I don't necessarily care
> for prompts either, but the tag behavior always struck me as weird- it
> warns, but it makes changes to the repo anyway and the stock hg
> configuration doesn't let you edit history to fix it.
>
> > I lean toward saying "lets have a user experience similar to `hg tag`".
> > Alternatively, we could have a warning early. For example, if the
> commit
> > message editor had a line about adding a new heads/closing non-head,
> > this would be a clear win.
>
> I typically don't notice what else is in the commit message editor,
> unless
> while writing the commit, I start to doubt what files I named.
>

Patch

diff --git a/tests/test-branches.t b/tests/test-branches.t
--- a/tests/test-branches.t
+++ b/tests/test-branches.t
@@ -956,19 +956,16 @@ 
 
 trying to close branch from a cset which is not a branch head
 it should abort:
-XXX: it should have aborted here
   $ hg ci -m "closing branch" --close-branch
-  created new head
+  abort: can only close branch heads
+  [255]
 
   $ hg up 0
   0 files updated, 0 files merged, 1 files removed, 0 files unresolved
   $ hg log -GT "{rev}: {node|short} {desc|firstline}\n\t{branch}\n\n"
-  _  3: 006876ddd20e closing branch
+  o  2: 155349b645be added c
   |  	default
   |
-  | o  2: 155349b645be added c
-  |/   	default
-  |
   o  1: 5f6d8a4bf34a added b
   |  	default
   |
diff --git a/mercurial/commands.py b/mercurial/commands.py
--- a/mercurial/commands.py
+++ b/mercurial/commands.py
@@ -1675,6 +1675,8 @@ 
 
         if not bheads:
             raise error.Abort(_('can only close branch heads'))
+        elif branch == repo['.'].branch() and repo['.'].node() not in bheads:
+            raise error.Abort(_('can only close branch heads'))
         elif opts.get('amend'):
             if (repo['.'].p1().branch() != branch and
                 repo['.'].p2().branch() != branch):