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

[David M. Carr]

On Sun, Oct 7, 2012 at 6:24 PM, Idan Kamara <idankk86 at gmail.com> 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
> +
>              for bmark, n in sorted(marks.iteritems()):
>                  current = repo._bookmarkcurrent
>                  if bmark == current and n == cur:
> diff --git a/tests/test-bookmarks-current.t b/tests/test-bookmarks-current.t
> --- a/tests/test-bookmarks-current.t
> +++ b/tests/test-bookmarks-current.t
> @@ -115,6 +115,14 @@
>       Z                         0:719295282060
>
>    $ hg up -q Y
> +  $ hg bookmark -i
> +  $ hg bookmarks
> +     Y                         0:719295282060
> +     Z                         0:719295282060
> +  $ hg bookmark -i
> +  abort: no active bookmark to deactivate
> +  [255]
> +  $ hg up -q Y
>    $ hg bookmarks
>     * Y                         0:719295282060
>       Z                         0:719295282060
> _______________________________________________
> Mercurial-devel mailing list
> Mercurial-devel at selenic.com
> http://selenic.com/mailman/listinfo/mercurial-devel

Looks good to me for stable, for what it's worth.  This looks like a
valid case where the current behavior doesn't match the documented
behavior, and the chosen fix (to implemented the documented behavior)
is what I would have expected to be the behavior in the first place.

I don't see an issue in the bug tracker for this, but I think I may
have run into it myself at one point (and probably should have created
an issue back then).

The additional test coverage looks appropriate to the change, and the
remainder of the test corpus run cleanly for me.
-- 
David M. Carr
david at carrclan.us