Refactor the bookmark validation code

Now it's trivial for the test module to mock the bookmark validation
without introducing too much logic about this mocking to the actual class.

2 files changed, 13 insertions, 6 deletions

diff --git a/ranger/container/bookmarks.py b/ranger/container/bookmarks.py
index 25e81097..7b5e56b7 100644
--- a/ranger/container/bookmarks.py
+++ b/ranger/container/bookmarks.py
@@ -12,7 +12,7 @@ from ranger.core.shared import FileManagerAware
 ALLOWED_KEYS = string.ascii_letters + string.digits + "`'"
 
 
-class Bookmarks(FileManagerAware):  # pylint: disable=too-many-instance-attributes
+class Bookmarks(FileManagerAware):
     """Bookmarks is a container which associates keys with bookmarks.
 
     A key is a string with: len(key) == 1 and key in ALLOWED_KEYS.
@@ -29,7 +29,7 @@ class Bookmarks(FileManagerAware):  # pylint: disable=too-many-instance-attribut
     load_pattern = re.compile(r"^[\d\w']:.")
 
     def __init__(self, bookmarkfile, bookmarktype=str, autosave=False,
-                 validate=True, nonpersistent_bookmarks=()):
+                 nonpersistent_bookmarks=()):
         """Initializes Bookmarks.
 
         <bookmarkfile> specifies the path to the file where
@@ -41,7 +41,6 @@ class Bookmarks(FileManagerAware):  # pylint: disable=too-many-instance-attribut
         self.path = bookmarkfile
         self.bookmarktype = bookmarktype
         self.nonpersistent_bookmarks = set(nonpersistent_bookmarks)
-        self.validate = validate
 
     def load(self):
         """Load the bookmarks from path/bookmarks"""
@@ -90,7 +89,7 @@ class Bookmarks(FileManagerAware):  # pylint: disable=too-many-instance-attribut
             key = "'"
         if key in self.dct:
             value = self.dct[key]
-            if not self.validate or os.path.isdir(str(value)):
+            if self._validate(value):
                 return value
             else:
                 raise KeyError("Cannot open bookmark: `%s'!" % key)
@@ -258,3 +257,6 @@ class Bookmarks(FileManagerAware):  # pylint: disable=too-many-instance-attribut
 
     def _update_mtime(self):
         self.last_mtime = self._get_mtime()
+
+    def _validate(self, value):  # pylint: disable=no-self-use
+        return os.path.isdir(str(value))
diff --git a/tests/ranger/container/test_bookmarks.py b/tests/ranger/container/test_bookmarks.py
index 8c62bd23..84ac42c2 100644
--- a/tests/ranger/container/test_bookmarks.py
+++ b/tests/ranger/container/test_bookmarks.py
@@ -8,11 +8,16 @@ import pytest
 from ranger.container.bookmarks import Bookmarks
 
 
+class NotValidatedBookmarks(Bookmarks):
+    def _validate(self, value):
+        return True
+
+
 def testbookmarks(tmpdir):
     # Bookmarks point to directory location and allow fast access to
     # 'favorite' directories. They are persisted to a bookmark file, plain text.
     bookmarkfile = tmpdir.join("bookmarkfile")
-    bmstore = Bookmarks(str(bookmarkfile), validate=False)
+    bmstore = NotValidatedBookmarks(str(bookmarkfile))
 
     # loading an empty bookmark file doesnot crash
     bmstore.load()
@@ -33,7 +38,7 @@ def testbookmarks(tmpdir):
 
     # We can persist bookmarks to disk and restore them from disk
     bmstore.save()
-    secondstore = Bookmarks(str(bookmarkfile), validate=False)
+    secondstore = NotValidatedBookmarks(str(bookmarkfile))
     secondstore.load()
     assert "'" in secondstore
     assert secondstore["'"] == "the milk"