[PATCH] bookmarks: deactivate current bookmark if no name is given

David M. Carr david at carrclan.us
Mon Oct 8 07:30:14 CDT 2012


On Mon, Oct 8, 2012 at 5:48 AM, Thomas Arendsen Hein
<thomas at intevation.de> wrote:
> * Pierre-Yves David <pierre-yves.david at logilab.fr> [20121008 10:58]:
>> On Mon, Oct 08, 2012 at 12:24:08AM +0200, Idan Kamara wrote:
>> > # HG changeset patch
>> > # User Idan Kamara <idankk86 at gmail.com>
>> > # Date 1349648370 -7200
>> > # Branch stable
>> > # Node ID 67f7906491d804423d16cb151e7add017ca1138d
>> > # Parent  6647ac9b9044023b4947e890b07d6dfef30ea9b3
>> > bookmarks: deactivate current bookmark if no name is given
>> >
>> > f57f891eb88e added this help text to hg bookmark:
>> >
>> >   If no NAME is given, the current active bookmark will be marked inactive.
>> >
>> > But that never actually seemed to be the case.
>> >
>> > diff --git a/mercurial/commands.py b/mercurial/commands.py
>> > --- a/mercurial/commands.py
>> > +++ b/mercurial/commands.py
>> > @@ -847,6 +847,12 @@
>> >          if len(marks) == 0:
>> >              ui.status(_("no bookmarks set\n"))
>> >          else:
>> > +            if inactive:
>> > +                if not repo._bookmarkcurrent:
>> > +                    raise util.Abort(_("no active bookmark to deactivate"))
>> > +                bookmarks.setcurrent(repo, None)
>> > +                return
>> > +
>>
>> I feel like the use of Abort here is a bit abusive.
>> What about a simple ui.write_err call + a return <some-error> code.
>
> Good point, I would even suggest ui.status and no error code, so
> scripts can use "hg bookmark -i -q" to deactivate a possibly active
> bookmark without printing a message before doing things.
>
> The result is that no bookmark is active, so no need to abort or set
> an error code.

That makes good sense to me.  There are definitely cases in scripts
where it may not be clear whether a bookmark is currently set.  When
requesting that the current bookmark is cleared, the current bookmark
already being clear doesn't sound like an error condition to me.

>
>> Rest semantic of the patches seems good.
>
> ditto.
>
> Regards,
> Thomas
>
> --
> thomas at intevation.de - http://intevation.de/~thomas/ - OpenPGP key: 0x5816791A
> Intevation GmbH, Neuer Graben 17, 49074 Osnabrueck - AG Osnabrueck, HR B 18998
> Geschaeftsfuehrer: Frank Koormann, Bernhard Reiter, Dr. Jan-Oliver Wagner
>
> _______________________________________________
> Mercurial-devel mailing list
> Mercurial-devel at selenic.com
> http://selenic.com/mailman/listinfo/mercurial-devel
>



-- 
David M. Carr
david at carrclan.us


More information about the Mercurial-devel mailing list