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

Sean Farley sean at farley.io
Thu Jun 22 16:33:17 EDT 2017


Martin von Zweigbergk <martinvonz at google.com> writes:

> 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?

That's a good question. I brought this up on IRC and no one really
responded. It *seems* that the design of bookmarks.py is to hide away
the existence of repo._bookmarks (which kinda makes sense). Then again,
a lot of these method would benefit from living in a bmstore.

I don't mind either way (we could do both, actually).
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 800 bytes
Desc: not available
URL: <http://www.mercurial-scm.org/pipermail/mercurial-devel/attachments/20170622/faa6939b/attachment.sig>


More information about the Mercurial-devel mailing list