[PATCH 1 of 7 bm-refactor V2] bookmarks: factor out delete logic from commands

Martin von Zweigbergk martinvonz at google.com
Thu Jun 22 01:48:16 EDT 2017


On Wed, Jun 21, 2017 at 1:45 PM, Sean Farley <sean at farley.io> wrote:
> # HG changeset patch
> # User Sean Farley <sean at farley.io>
> # Date 1497333768 25200
> #      Mon Jun 12 23:02:48 2017 -0700
> # Branch bm-refactor
> # Node ID afad435bfa048b562cd4e302f01d0456e7ad33c5
> # Parent  41b081ac2145d88ba1fe494630ce69652f016078
> bookmarks: factor out delete logic from commands
>
> While we're here, let's use fancy context managers. I believe this
> should still work since our locks are re-entrant.
>
> diff --git a/mercurial/bookmarks.py b/mercurial/bookmarks.py
> index 451c557..0371a52 100644
> --- a/mercurial/bookmarks.py
> +++ b/mercurial/bookmarks.py
> @@ -689,5 +689,20 @@ def checkformat(repo, mark):
>      if not mark:
>          raise error.Abort(_("bookmark names cannot consist entirely of "
>                              "whitespace"))
>      scmutil.checknewlabel(repo, mark, 'bookmark')
>      return mark
> +
> +def delete(repo, tr, names):
> +    """remove a mark from the bookmark store
> +
> +    Raises an abort error if mark does not exist.
> +    """
> +    marks = repo._bookmarks
> +    for mark in names:
> +        if mark not in marks:
> +            raise error.Abort(_("bookmark '%s' does not exist") %
> +                              mark)
> +        if mark == repo._activebookmark:
> +            deactivate(repo)

Should we put delete() on bmstore instead? Then you wouldn't need to
pass the repo. Or is the longer-term direction to make users
(call-sites) unaware of bmstore?

> +        del marks[mark]
> +    marks.recordchange(tr)
> diff --git a/mercurial/commands.py b/mercurial/commands.py
> index 4dd2521..876fb3e 100644
> --- a/mercurial/commands.py
> +++ b/mercurial/commands.py
> @@ -973,18 +973,11 @@ def bookmark(ui, repo, *names, **opts):
>              lock = repo.lock()
>              cur = repo.changectx('.').node()
>              marks = repo._bookmarks
>              if delete:
>                  tr = repo.transaction('bookmark')
> -                for mark in names:
> -                    if mark not in marks:
> -                        raise error.Abort(_("bookmark '%s' does not exist") %
> -                                         mark)
> -                    if mark == repo._activebookmark:
> -                        bookmarks.deactivate(repo)
> -                    del marks[mark]
> -
> +                bookmarks.delete(repo, tr, names)
>              elif rename:
>                  tr = repo.transaction('bookmark')
>                  if not names:
>                      raise error.Abort(_("new bookmark name required"))
>                  elif len(names) > 1:
> _______________________________________________
> Mercurial-devel mailing list
> Mercurial-devel at mercurial-scm.org
> https://www.mercurial-scm.org/mailman/listinfo/mercurial-devel


More information about the Mercurial-devel mailing list