Patchwork [2,of,6] bookmarks: hoist getbkfile out of bmstore class

login
register
mail settings
Submitter Augie Fackler
Date Dec. 2, 2015, 4:19 p.m.
Message ID <b1772fde2b3225ed9007.1449073148@imladris.local>
Download mbox | patch
Permalink /patch/11749/
State Superseded
Headers show

Comments

Augie Fackler - Dec. 2, 2015, 4:19 p.m.
# HG changeset patch
# User Augie Fackler <augie@google.com>
# Date 1447292738 18000
#      Wed Nov 11 20:45:38 2015 -0500
# Node ID b1772fde2b3225ed90075dfdcb661f8e821fb92a
# Parent  a3c94c05b06a6288079dbeb99719774b9ac6e47b
bookmarks: hoist getbkfile out of bmstore class

It's totally fine that this hook exists, but I don't see a need for it
to live inside the bmstore class.

Patch

diff --git a/hgext/share.py b/hgext/share.py
--- a/hgext/share.py
+++ b/hgext/share.py
@@ -121,7 +121,7 @@  def clone(orig, ui, source, *args, **opt
     return orig(ui, source, *args, **opts)
 
 def extsetup(ui):
-    extensions.wrapfunction(bookmarks.bmstore, 'getbkfile', getbkfile)
+    extensions.wrapfunction(bookmarks, '_getbkfile', getbkfile)
     extensions.wrapfunction(bookmarks.bmstore, 'recordchange', recordchange)
     extensions.wrapfunction(bookmarks.bmstore, '_writerepo', writerepo)
     extensions.wrapcommand(commands.table, 'clone', clone)
@@ -149,12 +149,12 @@  def _getsrcrepo(repo):
     srcurl, branches = parseurl(source)
     return repository(repo.ui, srcurl)
 
-def getbkfile(orig, self, repo):
+def getbkfile(orig, repo):
     if _hassharedbookmarks(repo):
         srcrepo = _getsrcrepo(repo)
         if srcrepo is not None:
             repo = srcrepo
-    return orig(self, repo)
+    return orig(repo)
 
 def recordchange(orig, self, tr):
     # Continue with write to local bookmarks file as usual
diff --git a/mercurial/bookmarks.py b/mercurial/bookmarks.py
--- a/mercurial/bookmarks.py
+++ b/mercurial/bookmarks.py
@@ -22,6 +22,25 @@  from . import (
     util,
 )
 
+def _getbkfile(repo):
+    """Hook so that extensions that mess with the store can hook bm storage.
+
+    For core, this just handles wether we should see pending
+    bookmarks or the committed ones. Other extensions (like share)
+    may need to tweak this behavior further.
+    """
+    bkfile = None
+    if 'HG_PENDING' in os.environ:
+        try:
+            bkfile = repo.vfs('bookmarks.pending')
+        except IOError as inst:
+            if inst.errno != errno.ENOENT:
+                raise
+    if bkfile is None:
+        bkfile = repo.vfs('bookmarks')
+    return bkfile
+
+
 class bmstore(dict):
     """Storage for bookmarks.
 
@@ -41,7 +60,7 @@  class bmstore(dict):
         dict.__init__(self)
         self._repo = repo
         try:
-            bkfile = self.getbkfile(repo)
+            bkfile = _getbkfile(repo)
             for line in bkfile:
                 line = line.strip()
                 if not line:
@@ -60,24 +79,6 @@  class bmstore(dict):
             if inst.errno != errno.ENOENT:
                 raise
 
-    def getbkfile(self, repo):
-        """Hook so that extensions that mess with the store can hook bm storage.
-
-        For core, this just handles wether we should see pending
-        bookmarks or the committed ones. Other extensions (like share)
-        may need to tweak this behavior further.
-        """
-        bkfile = None
-        if 'HG_PENDING' in os.environ:
-            try:
-                bkfile = repo.vfs('bookmarks.pending')
-            except IOError as inst:
-                if inst.errno != errno.ENOENT:
-                    raise
-        if bkfile is None:
-            bkfile = repo.vfs('bookmarks')
-        return bkfile
-
     def recordchange(self, tr):
         """record that bookmarks have been changed in a transaction