Change the bookmark interface

move the modulename to bookmark → boookmarks
add a bookmarklist class with
remove module-level variables
do a features check on startup before trying to fetch
    the bookmarks

6 files changed, 393 insertions, 362 deletions

diff --git a/src/bookmark.py b/src/bookmark.py
deleted file mode 100644
index afe38e5e..00000000
--- a/src/bookmark.py
+++ /dev/null
@@ -1,260 +0,0 @@
-"""
-Bookmarks module
-
-Therein the bookmark class is defined, representing one conference room.
-This object is used to generate elements for both local and remote
-bookmark storage. It can also parse xml Elements.
-
-This module also defines several functions for retrieving and updating
-bookmarks, both local and remote.
-"""
-
-import functools
-import logging
-
-from slixmpp.plugins.xep_0048 import Bookmarks, Conference, URL
-from common import safeJID
-from config import config
-
-log = logging.getLogger(__name__)
-
-preferred = config.get('use_bookmarks_method').lower()
-if preferred not in ('pep', 'privatexml'):
-    preferred = 'privatexml'
-not_preferred = 'privatexml' if preferred == 'pep' else 'pep'
-methods = ('local', preferred, not_preferred)
-
-
-class Bookmark(object):
-    possible_methods = methods
-
-    def __init__(self, jid, name=None, autojoin=False, nick=None, password=None, method='privatexml'):
-        self.jid = jid
-        self.name = name or jid
-        self.autojoin = autojoin
-        self.nick = nick
-        self.password = password
-        self._method = method
-
-    @property
-    def method(self):
-        return self._method
-
-    @method.setter
-    def method(self, value):
-        if value not in self.possible_methods:
-            log.debug('Could not set bookmark storing method: %s', value)
-            return
-        self._method = value
-
-    def __repr__(self):
-        return '<%s%s%s%s>' % (self.jid, ('/'+self.nick) if self.nick else '', '|autojoin' if self.autojoin else '', '|%s' % self.password if self.password else '')
-
-    def stanza(self):
-        """
-        Generate a <conference/> stanza from the instance
-        """
-        el = Conference()
-        el['name'] = self.name
-        el['jid'] = self.jid
-        el['autojoin'] = 'true' if self.autojoin else 'false'
-        if self.nick:
-            el['nick'] = self.nick
-        if self.password:
-            el['password'] = self.password
-        return el
-
-    def local(self):
-        """Generate a str for local storage"""
-        local = self.jid
-        if self.nick:
-            local += '/%s' % self.nick
-        local += ':'
-        if self.password:
-            config.set_and_save('password', self.password, section=self.jid)
-        return local
-
-    @functools.singledispatch
-    @staticmethod
-    def parse(el, method=None):
-        """
-        Generate a Bookmark object from a <conference/> element
-        (this is a fallback for raw XML Elements)
-        """
-        jid = el.get('jid')
-        name = el.get('name')
-        autojoin = True if el.get('autojoin', 'false').lower() in ('true', '1') else False
-        nick = None
-        for n in el.iter('nick'):
-            nick = n.text
-        password = None
-        for p in el.iter('password'):
-            password = p.text
-
-        return Bookmark(jid, name, autojoin, nick, password, method)
-
-    @staticmethod
-    @parse.register(Conference)
-    def parse_from_stanza(el, method=None):
-        """
-        Parse a Conference element into a Bookmark object
-        """
-        jid = el['jid']
-        autojoin = el['autojoin']
-        password = el['password']
-        nick = el['nick']
-        name = el['name']
-        return Bookmark(jid, name, autojoin, nick, password, method)
-
-bookmarks = []
-
-def get_by_jid(value):
-    """
-    Get a bookmark by bare jid
-    """
-    for item in bookmarks:
-        if item.jid == value:
-            return item
-
-def remove(value):
-    """
-    Remove a bookmark (with its jid or directly the Bookmark object).
-    """
-    if isinstance(value, str):
-        value = get_by_jid(value)
-    bookmarks.remove(value)
-
-def stanza_storage(method):
-    """Generate a <storage/> stanza with the conference elements."""
-    storage = Bookmarks()
-    for b in (b for b in bookmarks if b.method == method):
-        storage.append(b.stanza())
-    return storage
-
-def save_pep(xmpp):
-    """Save the remote bookmarks via PEP."""
-    xmpp.plugin['xep_0048'].set_bookmarks(stanza_storage('pep'),
-            method='xep_0223')
-
-def save_privatexml(xmpp):
-    """"Save the remote bookmarks with privatexml."""
-    xmpp.plugin['xep_0048'].set_bookmarks(stanza_storage('privatexml'),
-            method='xep_0049')
-
-def save_remote(xmpp, callback, method=preferred):
-    """Save the remote bookmarks."""
-    method = 'privatexml' if method != 'pep' else 'pep'
-
-    if method is 'privatexml':
-        xmpp.plugin['xep_0048'].set_bookmarks(stanza_storage('privatexml'),
-                                              method='xep_0049',
-                                              callback=callback)
-    else:
-        xmpp.plugin['xep_0048'].set_bookmarks(stanza_storage('pep'),
-                                              method='xep_0223',
-                                              callback=callback)
-
-def save_local():
-    """Save the local bookmarks."""
-    local = ''.join(bookmark.local() for bookmark in bookmarks if bookmark.method is 'local')
-    config.set_and_save('rooms', local)
-
-def save(xmpp, core=None):
-    """Save all the bookmarks."""
-    save_local()
-    def _cb(core, iq):
-        if iq["type"] == "error":
-            core.information('Could not save bookmarks.', 'Error')
-        elif core:
-            core.information('Bookmarks saved', 'Info')
-    if config.get('use_remote_bookmarks'):
-        preferred = config.get('use_bookmarks_method')
-        cb = functools.partial(_cb, core)
-        save_remote(xmpp, cb, method=preferred)
-
-def get_pep(xmpp, available_methods, callback):
-    """Add the remotely stored bookmarks via pep to the list."""
-    def _cb(iq):
-        if iq["type"] == "error":
-            available_methods["pep"] = False
-        else:
-            available_methods["pep"] = True
-            for conf in iq['pubsub']['items']['item']['bookmarks']['conferences']:
-                if isinstance(conf, URL):
-                    continue
-                b = Bookmark.parse(conf, method='pep')
-                if not get_by_jid(b.jid):
-                    bookmarks.append(b)
-        if callback:
-            callback()
-
-    xmpp.plugin['xep_0048'].get_bookmarks(method='xep_0223', callback=_cb)
-
-def get_privatexml(xmpp, available_methods, callback):
-    """Add the remotely stored bookmarks via privatexml to the list.
-    If both is True, we want to have the result of both methods (privatexml and pep) before calling pep"""
-    def _cb(iq):
-        if iq["type"] == "error":
-            available_methods["privatexml"] = False
-        else:
-            available_methods["privatexml"] = True
-            for conf in iq['private']['bookmarks']['conferences']:
-                b = Bookmark.parse(conf, method='privatexml')
-                if not get_by_jid(b.jid):
-                    bookmarks.append(b)
-        if callback:
-            callback()
-
-    xmpp.plugin['xep_0048'].get_bookmarks(method='xep_0049', callback=_cb)
-
-def get_remote(xmpp, callback):
-    """Add the remotely stored bookmarks to the list."""
-    if xmpp.anon:
-        return
-    method = config.get('use_bookmarks_method')
-    if not method:
-        available_methods = {}
-        def _save_and_call_callback():
-            # If both methods returned a result, we can now call the given callback
-            if callback and "privatexml" in available_methods and "pep" in available_methods:
-                save_bookmarks_method(available_methods)
-                if callback:
-                    callback()
-        for method in methods[1:]:
-            if method == 'pep':
-                get_pep(xmpp, available_methods, _save_and_call_callback)
-            else:
-                get_privatexml(xmpp, available_methods, _save_and_call_callback)
-    else:
-        if method == 'pep':
-            get_pep(xmpp, {}, callback)
-        else:
-            get_privatexml(xmpp, {}, callback)
-
-def save_bookmarks_method(available_methods):
-    pep, privatexml = available_methods["pep"], available_methods["privatexml"]
-    if pep and not privatexml:
-        config.set_and_save('use_bookmarks_method', 'pep')
-    elif privatexml and not pep:
-        config.set_and_save('use_bookmarks_method', 'privatexml')
-    elif not pep and not privatexml:
-        config.set_and_save('use_bookmarks_method', '')
-
-def get_local():
-    """Add the locally stored bookmarks to the list."""
-    rooms = config.get('rooms')
-    if not rooms:
-        return
-    rooms = rooms.split(':')
-    for room in rooms:
-        jid = safeJID(room)
-        if jid.bare == '':
-            continue
-        if jid.resource != '':
-            nick = jid.resource
-        else:
-            nick = None
-        passwd = config.get_by_tabname('password', jid.bare, fallback=False) or None
-        b = Bookmark(jid.bare, autojoin=True, nick=nick, password=passwd, method='local')
-        if not get_by_jid(b.jid):
-            bookmarks.append(b)
diff --git a/src/bookmarks.py b/src/bookmarks.py
new file mode 100644
index 00000000..47c40576
--- /dev/null
+++ b/src/bookmarks.py
@@ -0,0 +1,277 @@
+"""
+Bookmarks module
+
+Therein the bookmark class is defined, representing one conference room.
+This object is used to generate elements for both local and remote
+bookmark storage. It can also parse xml Elements.
+
+This module also defines several functions for retrieving and updating
+bookmarks, both local and remote.
+
+Poezio start scenario:
+
+- upon inital connection, poezio will disco#info the server
+- the available storage methods will be stored in the available_storage dict
+    (either 'pep' or 'privatexml')
+- if only one is available, poezio will set the use_bookmarks_method config option
+    to it. If both are, it will be set to 'privatexml' (or if it was previously set, the
+    value will be kept).
+- it will then query the preferred storages for bookmarks and cache them locally
+    (Bookmark objects with a method='remote' attribute)
+
+Adding a remote bookmark:
+
+- New Bookmark object added to the list with storage='remote'
+- All bookmarks are sent to the storage selected in use_bookmarks_method
+    if there was an error, the user is notified.
+
+
+"""
+
+import functools
+import logging
+from gettext import gettext as _
+
+from slixmpp.plugins.xep_0048 import Bookmarks, Conference, URL
+from slixmpp import JID
+from common import safeJID
+from config import config
+
+log = logging.getLogger(__name__)
+
+
+class Bookmark(object):
+
+    def __init__(self, jid, name=None, autojoin=False, nick=None, password=None, method='local'):
+        self.jid = jid
+        self.name = name or jid
+        self.autojoin = autojoin
+        self.nick = nick
+        self.password = password
+        self._method = method
+
+    @property
+    def method(self):
+        return self._method
+
+    @method.setter
+    def method(self, value):
+        if value not in ('local', 'remote'):
+            log.debug('Could not set bookmark storing method: %s', value)
+            return
+        self._method = value
+
+    def __repr__(self):
+        return '<%s%s|%s>' % (self.jid,
+                               ('/'+self.nick) if self.nick else '',
+                               self.method)
+
+    def stanza(self):
+        """
+        Generate a <conference/> stanza from the instance
+        """
+        el = Conference()
+        el['name'] = self.name
+        el['jid'] = self.jid
+        el['autojoin'] = 'true' if self.autojoin else 'false'
+        if self.nick:
+            el['nick'] = self.nick
+        if self.password:
+            el['password'] = self.password
+        return el
+
+    def local(self):
+        """Generate a str for local storage"""
+        local = self.jid
+        if self.nick:
+            local += '/%s' % self.nick
+        local += ':'
+        if self.password:
+            config.set_and_save('password', self.password, section=self.jid)
+        return local
+
+    @functools.singledispatch
+    @staticmethod
+    def parse(el):
+        """
+        Generate a Bookmark object from a <conference/> element
+        (this is a fallback for raw XML Elements)
+        """
+        jid = el.get('jid')
+        name = el.get('name')
+        autojoin = True if el.get('autojoin', 'false').lower() in ('true', '1') else False
+        nick = None
+        for n in el.iter('nick'):
+            nick = n.text
+        password = None
+        for p in el.iter('password'):
+            password = p.text
+
+        return Bookmark(jid, name, autojoin, nick, password, method='remote')
+
+    @staticmethod
+    @parse.register(Conference)
+    def parse_from_stanza(el):
+        """
+        Parse a Conference element into a Bookmark object
+        """
+        jid = el['jid']
+        autojoin = el['autojoin']
+        password = el['password']
+        nick = el['nick']
+        name = el['name']
+        return Bookmark(jid, name, autojoin, nick, password, method='remote')
+
+class BookmarkList(object):
+
+    def __init__(self):
+        self.bookmarks = []
+        preferred = config.get('use_bookmarks_method').lower()
+        if preferred not in ('pep', 'privatexml'):
+            preferred = 'privatexml'
+        self.preferred = preferred
+        self.available_storage = {
+            'privatexml': False,
+            'pep': False,
+        }
+
+    def __getitem__(self, key):
+        if isinstance(key, (str, JID)):
+            for i in self.bookmarks:
+                if key == i.jid:
+                    return i
+        else:
+            return self.bookmarks[key]
+
+    def __in__(self, key):
+        if isinstance(key, (str, JID)):
+            for bookmark in self.bookmarks:
+                if bookmark.jid == key:
+                    return True
+        else:
+            return key in self.bookmarks
+        return False
+
+    def remove(self, key):
+        if isinstance(key, (str, JID)):
+            for i in self.bookmarks[:]:
+                if i.jid == key:
+                    self.bookmarks.remove(i)
+        else:
+            self.bookmarks.remove(key)
+
+    def __iter__(self):
+        return iter(self.bookmarks)
+
+    def local(self):
+        return [bm for bm in self.bookmarks if bm.method == 'local']
+
+    def remote(self):
+        return [bm for bm in self.bookmarks if bm.method == 'remote']
+
+    def set(self, new):
+        self.bookmarks = new
+
+    def append(self, bookmark):
+        bookmark_exists = self[bookmark.jid]
+        if not bookmark_exists:
+            self.bookmarks.append(bookmark)
+        else:
+            self.bookmarks.remove(bookmark_exists)
+            self.bookmarks.append(bookmark)
+
+    def set_bookmarks_method(self, value):
+        if self.available_storage.get(value):
+            self.preferred = value
+            config.set_and_save('use_bookmarks_method', value)
+
+    def save_remote(self, xmpp, callback):
+        """Save the remote bookmarks."""
+        if not any(self.available_storage.values()):
+            return
+        method = 'xep_0049' if self.preferred == 'privatexml' else 'xep_0223'
+
+        if method:
+            xmpp.plugin['xep_0048'].set_bookmarks(stanza_storage(self.bookmarks),
+                                                  method=method,
+                                                  callback=callback)
+    def save_local(self):
+        """Save the local bookmarks."""
+        local = ''.join(bookmark.local() for bookmark in self if bookmark.method == 'local')
+        config.set_and_save('rooms', local)
+
+    def save(self, xmpp, core=None, callback=None):
+        """Save all the bookmarks."""
+        self.save_local()
+        def _cb(iq):
+            if callback:
+                callback(iq)
+            if iq["type"] == "error" and core:
+                core.information('Could not save remote bookmarks.', 'Error')
+            elif core:
+                core.information('Bookmarks saved', 'Info')
+        if config.get('use_remote_bookmarks'):
+            self.save_remote(xmpp, _cb)
+
+    def get_pep(self, xmpp, callback):
+        """Add the remotely stored bookmarks via pep to the list."""
+        def _cb(iq):
+            if iq['type'] == 'result':
+                for conf in iq['pubsub']['items']['item']['bookmarks']['conferences']:
+                    if isinstance(conf, URL):
+                        continue
+                    b = Bookmark.parse(conf)
+                    self.append(b)
+            if callback:
+                callback(iq)
+
+        xmpp.plugin['xep_0048'].get_bookmarks(method='xep_0223', callback=_cb)
+
+    def get_privatexml(self, xmpp, callback):
+        """
+        Fetch the remote bookmarks stored via privatexml.
+        """
+        def _cb(iq):
+            if iq['type'] == 'result':
+                for conf in iq['private']['bookmarks']['conferences']:
+                    b = Bookmark.parse(conf)
+                    self.append(b)
+            if callback:
+                callback(iq)
+
+        xmpp.plugin['xep_0048'].get_bookmarks(method='xep_0049', callback=_cb)
+
+    def get_remote(self, xmpp, information, callback):
+        """Add the remotely stored bookmarks to the list."""
+        if xmpp.anon or not any(self.available_storage.values()):
+            information(_('No remote bookmark storage available'), 'Warning')
+            return
+        if self.preferred == 'privatexml':
+            self.get_privatexml(xmpp, callback=callback)
+        else:
+            self.get_pep(xmpp, callback=callback)
+
+    def get_local(self):
+        """Add the locally stored bookmarks to the list."""
+        rooms = config.get('rooms')
+        if not rooms:
+            return
+        rooms = rooms.split(':')
+        for room in rooms:
+            jid = safeJID(room)
+            if jid.bare == '':
+                continue
+            if jid.resource != '':
+                nick = jid.resource
+            else:
+                nick = None
+            passwd = config.get_by_tabname('password', jid.bare, fallback=False) or None
+            b = Bookmark(jid.bare, autojoin=True, nick=nick, password=passwd, method='local')
+            self.append(b)
+
+def stanza_storage(bookmarks):
+    """Generate a <storage/> stanza with the conference elements."""
+    storage = Bookmarks()
+    for b in (b for b in bookmarks if b.method == 'remote'):
+        storage.append(b.stanza())
+    return storage
diff --git a/src/core/commands.py b/src/core/commands.py
index 5912abe2..497a53e6 100644
--- a/src/core/commands.py
+++ b/src/core/commands.py
@@ -6,9 +6,7 @@ import logging
 
 log = logging.getLogger(__name__)
 
-import functools
 import os
-import sys
 from datetime import datetime
 from gettext import gettext as _
 from xml.etree import cElementTree as ET
@@ -17,11 +15,11 @@ from slixmpp.xmlstream.stanzabase import StanzaBase
 from slixmpp.xmlstream.handler import Callback
 from slixmpp.xmlstream.matcher import StanzaPath
 
-import bookmark
 import common
 import fixes
 import pep
 import tabs
+from bookmarks import Bookmark
 from common import safeJID
 from config import config, DEFAULT_CONFIG, options as config_opts
 import multiuserchat as muc
@@ -428,20 +426,19 @@ def command_bookmark_local(self, args):
     elif args[0] == '*':
         new_bookmarks = []
         for tab in self.get_tabs(tabs.MucTab):
-            b = bookmark.get_by_jid(tab.name)
+            b = self.bookmarks[tab.name]
             if not b:
-                b = bookmark.Bookmark(tab.name,
-                                      autojoin=True,
-                                      method="local")
+                b = Bookmark(tab.name,
+                             autojoin=True,
+                             method="local")
                 new_bookmarks.append(b)
             else:
                 b.method = "local"
                 new_bookmarks.append(b)
-                bookmark.bookmarks.remove(b)
-        new_bookmarks.extend(bookmark.bookmarks)
-        bookmark.bookmarks = new_bookmarks
-        bookmark.save_local()
-        bookmark.save_remote(self.xmpp, None)
+                self.bookmarks.remove(b)
+        new_bookmarks.extend(self.bookmarks.bookmarks)
+        self.bookmarks.set(new_bookmarks)
+        self.bookmarks.save(self.xmpp)
         self.information('Bookmarks added and saved.', 'Info')
         return
     else:
@@ -456,10 +453,10 @@ def command_bookmark_local(self, args):
         if len(args) > 1:
             password = args[1]
 
-    bm = bookmark.get_by_jid(roomname)
+    bm = self.bookmarks[roomname]
     if not bm:
-        bm = bookmark.Bookmark(jid=roomname)
-        bookmark.bookmarks.append(bm)
+        bm = Bookmark(jid=roomname)
+        self.bookmarks.append(bm)
         self.information('Bookmark added.', 'Info')
     else:
         self.information('Bookmark updated.', 'Info')
@@ -468,9 +465,10 @@ def command_bookmark_local(self, args):
     bm.autojoin = True
     bm.password = password
     bm.method = "local"
-    bookmark.save_local()
+    self.bookmarks.save_local()
     self.information(_('Your local bookmarks are now: %s') %
-            [b for b in bookmark.bookmarks if b.method == 'local'], 'Info')
+                         self.bookmarks.local(),
+                     'Info')
 
 @command_args_parser.quoted(0, 3)
 def command_bookmark(self, args):
@@ -497,24 +495,24 @@ def command_bookmark(self, args):
             autojoin = True
         new_bookmarks = []
         for tab in self.get_tabs(tabs.MucTab):
-            b = bookmark.get_by_jid(tab.name)
+            b = self.bookmarks[tab.name]
             if not b:
-                b = bookmark.Bookmark(tab.name, autojoin=autojoin,
-                        method=bookmark.preferred)
+                b = Bookmark(tab.name, autojoin=autojoin,
+                             method='remote')
                 new_bookmarks.append(b)
             else:
-                b.method = bookmark.preferred
-                bookmark.bookmarks.remove(b)
+                b.method = 'remote'
+                self.bookmarks.remove(b)
                 new_bookmarks.append(b)
-        new_bookmarks.extend(bookmark.bookmarks)
-        bookmark.bookmarks = new_bookmarks
-        def _cb(self, iq):
+        new_bookmarks.extend(self.bookmarks.bookmarks)
+        self.bookmarks.set(new_bookmarks)
+        def _cb(iq):
             if iq["type"] != "error":
-                bookmark.save_local()
+                self.bookmarks.save_local()
                 self.information("Bookmarks added.", "Info")
             else:
                 self.information("Could not add the bookmarks.", "Info")
-        bookmark.save_remote(self.xmpp, functools.partial(_cb, self))
+        self.bookmarks.save_remote(self.xmpp, _cb)
         return
     else:
         info = safeJID(args[0])
@@ -533,64 +531,58 @@ def command_bookmark(self, args):
             password = args[2]
         else:
             password = None
-    bm = bookmark.get_by_jid(roomname)
+    bm = self.bookmarks[roomname]
     if not bm:
-        bm = bookmark.Bookmark(roomname)
-        bookmark.bookmarks.append(bm)
-    bm.method = config.get('use_bookmarks_method')
+        bm = Bookmark(roomname)
+        self.bookmarks.append(bm)
+    bm.method = 'remote'
     if nick:
         bm.nick = nick
     if password:
         bm.password = password
     bm.autojoin = autojoin
-    def _cb(self, iq):
+    def __cb(iq):
         if iq["type"] != "error":
             self.information('Bookmark added.', 'Info')
         else:
             self.information("Could not add the bookmarks.", "Info")
-    bookmark.save_remote(self.xmpp, functools.partial(_cb, self))
-    remote = []
-    for each in bookmark.bookmarks:
-        if each.method in ('pep', 'privatexml'):
-            remote.append(each)
+    self.bookmarks.save_remote(self.xmpp, __cb)
 
 @command_args_parser.ignored
 def command_bookmarks(self):
     """/bookmarks"""
-    local = []
-    remote = []
-    for each in bookmark.bookmarks:
-        if each.method in ('pep', 'privatexml'):
-            remote.append(each)
-        elif each.method == 'local':
-            local.append(each)
-
-    self.information(_('Your remote bookmarks are: %s') % remote,
-                     _('Info'))
-    self.information(_('Your local bookmarks are: %s') % local,
-                     _('Info'))
+    tab = self.get_tab_by_name('Bookmarks', tabs.BookmarksTab)
+    if tab:
+        self.current_tab_nb = tab.nb
+    else:
+        tab = tabs.BookmarksTab(self.bookmarks)
+        self.tabs.append(tab)
+        self.current_tab_nb = tab.nb
+    self.refresh_window()
 
 @command_args_parser.quoted(0, 1)
 def command_remove_bookmark(self, args):
     """/remove_bookmark [jid]"""
 
+    def cb(success):
+        if success:
+            self.information(_('Bookmark deleted'), 'Info')
+        else:
+            self.information(_('Error while deleting the bookmark'), 'Error')
+
     if not args:
         tab = self.current_tab()
-        if isinstance(tab, tabs.MucTab) and bookmark.get_by_jid(tab.name):
-            bookmark.remove(tab.name)
-            bookmark.save(self.xmpp)
-            if bookmark.save(self.xmpp):
-                self.information('Bookmark deleted', 'Info')
+        if isinstance(tab, tabs.MucTab) and self.bookmarks[tab.name]:
+            self.bookmarks.remove(tab.name)
+            self.bookmarks.save(self.xmpp, callback=cb)
         else:
-            self.information('No bookmark to remove', 'Info')
+            self.information(_('No bookmark to remove'), 'Info')
     else:
-        if bookmark.get_by_jid(args[0]):
-            bookmark.remove(args[0])
-            if bookmark.save(self.xmpp):
-                self.information('Bookmark deleted', 'Info')
-
+        if self.bookmarks[args[0]]:
+            self.bookmarks.remove(args[0])
+            self.bookmarks.save(self.xmpp, callback=cb)
         else:
-            self.information('No bookmark to remove', 'Info')
+            self.information(_('No bookmark to remove'), 'Info')
 
 @command_args_parser.quoted(1, 2)
 def command_set(self, args):
diff --git a/src/core/completions.py b/src/core/completions.py
index c793fa52..68cbb24a 100644
--- a/src/core/completions.py
+++ b/src/core/completions.py
@@ -8,7 +8,6 @@ log = logging.getLogger(__name__)
 import os
 from functools import reduce
 
-import bookmark
 import common
 import pep
 import tabs
@@ -96,7 +95,7 @@ def completion_join(self, the_input):
 
     relevant_rooms = []
     relevant_rooms.extend(sorted(self.pending_invites.keys()))
-    bookmarks = {str(elem.jid): False for elem in bookmark.bookmarks}
+    bookmarks = {str(elem.jid): False for elem in self.bookmarks}
     for tab in self.get_tabs(tabs.MucTab):
         name = tab.name
         if name in bookmarks and not tab.joined:
@@ -192,7 +191,7 @@ def completion_bookmark(self, the_input):
 
 def completion_remove_bookmark(self, the_input):
     """Completion for /remove_bookmark"""
-    return the_input.new_completion([bm.jid for bm in bookmark.bookmarks], 1, quotify=False)
+    return the_input.new_completion([bm.jid for bm in self.bookmarks], 1, quotify=False)
 
 
 def completion_decline(self, the_input):
diff --git a/src/core/core.py b/src/core/core.py
index 807ca0cc..34d733ba 100644
--- a/src/core/core.py
+++ b/src/core/core.py
@@ -10,7 +10,6 @@ import logging
 log = logging.getLogger(__name__)
 
 import asyncio
-import collections
 import shutil
 import curses
 import os
@@ -18,22 +17,20 @@ import pipes
 import sys
 import time
 from threading import Event
-from datetime import datetime
 from gettext import gettext as _
 
 from slixmpp.xmlstream.handler import Callback
 
-import bookmark
 import connection
 import decorators
 import events
-import fixes
 import singleton
 import tabs
 import theming
 import timed_events
 import windows
 
+from bookmarks import BookmarkList
 from common import safeJID
 from config import config, firstrun
 from contact import Contact, Resource
@@ -75,6 +72,7 @@ class Core(object):
         self.keyboard = keyboard.Keyboard()
         roster.set_node(self.xmpp.client_roster)
         decorators.refresh_wrapper.core = self
+        self.bookmarks = BookmarkList()
         self.paused = False
         self.event = Event()
         self.debug = False
@@ -311,6 +309,8 @@ class Core(object):
                                        theming.update_themes_dir)
         self.add_configuration_handler("theme",
                                        self.on_theme_config_change)
+        self.add_configuration_handler("use_bookmarks_method",
+                                       self.on_bookmarks_method_config_change)
         self.add_configuration_handler("password",
                                        self.on_password_change)
         self.add_configuration_handler("enable_vertical_tab_list",
@@ -351,6 +351,14 @@ class Core(object):
         for callback in self.configuration_change_handlers[option]:
             callback(option, value)
 
+    def on_bookmarks_method_config_change(self, option, value):
+        """
+        Called when the use_bookmarks_method option changes
+        """
+        if 'value' not in ('pep', 'privatexml'):
+            return
+        self.bookmarks.preferred = value
+
     def on_gaps_config_change(self, option, value):
         """
         Called when the option create_gaps is changed.
@@ -840,7 +848,7 @@ class Core(object):
         Returns the nickname associated with a bookmark
         or the default nickname
         """
-        bm = bookmark.get_by_jid(room_name)
+        bm = self.bookmarks[room_name]
         if bm:
             return bm.nick
         return self.own_nick
@@ -1980,6 +1988,7 @@ class Core(object):
     on_receipt = handlers.on_receipt
     on_attention = handlers.on_attention
     room_error = handlers.room_error
+    check_bookmark_storage = handlers.check_bookmark_storage
     outgoing_stanza = handlers.outgoing_stanza
     incoming_stanza = handlers.incoming_stanza
     validate_ssl = handlers.validate_ssl
diff --git a/src/core/handlers.py b/src/core/handlers.py
index b16b8c0f..0bca6cd8 100644
--- a/src/core/handlers.py
+++ b/src/core/handlers.py
@@ -16,11 +16,9 @@ from gettext import gettext as _
 from os import path
 
 from slixmpp import InvalidJID
-from slixmpp.stanza import Message
 from slixmpp.xmlstream.stanzabase import StanzaBase, ElementBase
 from xml.etree import ElementTree as ET
 
-import bookmark
 import common
 import fixes
 import pep
@@ -48,6 +46,49 @@ try:
 except ImportError:
     PYGMENTS = False
 
+def _join_initial_rooms(self, bookmarks):
+    """Join all rooms given in the iterator `bookmarks`"""
+    for bm in bookmarks:
+        if not (bm.autojoin or config.get('open_all_bookmarks')):
+            continue
+        tab = self.get_tab_by_name(bm.jid, tabs.MucTab)
+        nick = bm.nick if bm.nick else self.own_nick
+        if not tab:
+            self.open_new_room(bm.jid, nick, False)
+        self.initial_joins.append(bm.jid)
+        histo_length = config.get('muc_history_length')
+        if histo_length == -1:
+            histo_length = None
+        if histo_length is not None:
+            histo_length = str(histo_length)
+        # do not join rooms that do not have autojoin
+        # but display them anyway
+        if bm.autojoin:
+            muc.join_groupchat(self, bm.jid, nick,
+                    passwd=bm.password,
+                    maxhistory=histo_length,
+                    status=self.status.message,
+                    show=self.status.show)
+
+def check_bookmark_storage(self, features):
+    private = 'jabber:iq:private' in features
+    pep_ = 'http://jabber.org/protocol/pubsub#publish' in features
+    self.bookmarks.available_storage['private'] = private
+    self.bookmarks.available_storage['pep'] = pep_
+    def _join_remote_only(iq):
+        if iq['type'] == 'error':
+            type_ = iq['error']['type']
+            condition = iq['error']['condition']
+            if not (type_ == 'cancel' and condition == 'item-not-found'):
+                self.information(_('Unable to fetch the remote'
+                                   ' bookmarks; %s: %s') % (type_, condition),
+                                 'Error')
+            return
+        remote_bookmarks = self.bookmarks.remote()
+        _join_initial_rooms(self, remote_bookmarks)
+    if not self.xmpp.anon and config.get('use_remote_bookmarks'):
+        self.bookmarks.get_remote(self.xmpp, self.information, _join_remote_only)
+
 def on_session_start_features(self, _):
     """
     Enable carbons & blocking on session start if wanted and possible
@@ -64,6 +105,7 @@ def on_session_start_features(self, _):
             self.xmpp.plugin['xep_0280'].enable()
             self.xmpp.add_event_handler('carbon_received', self.on_carbon_received)
             self.xmpp.add_event_handler('carbon_sent', self.on_carbon_sent)
+        self.check_bookmark_storage(features)
 
     self.xmpp.plugin['xep_0030'].get_info(jid=self.xmpp.boundjid.domain,
                                           callback=callback)
@@ -926,37 +968,9 @@ def on_session_start(self, event):
             pres['status'] = self.status.message
             self.events.trigger('send_normal_presence', pres)
             pres.send()
-    bookmark.get_local()
-    def _join_initial_rooms(bookmarks):
-        """Join all rooms given in the iterator `bookmarks`"""
-        for bm in bookmarks:
-            if bm.autojoin or config.get('open_all_bookmarks'):
-                tab = self.get_tab_by_name(bm.jid, tabs.MucTab)
-                nick = bm.nick if bm.nick else self.own_nick
-                if not tab:
-                    self.open_new_room(bm.jid, nick, False)
-                self.initial_joins.append(bm.jid)
-                histo_length = config.get('muc_history_length')
-                if histo_length == -1:
-                    histo_length = None
-                if histo_length is not None:
-                    histo_length = str(histo_length)
-                # do not join rooms that do not have autojoin
-                # but display them anyway
-                if bm.autojoin:
-                    muc.join_groupchat(self, bm.jid, nick,
-                            passwd=bm.password,
-                            maxhistory=histo_length,
-                            status=self.status.message,
-                            show=self.status.show)
-    def _join_remote_only():
-        remote_bookmarks = (bm for bm in bookmark.bookmarks if (bm.method in ("pep", "privatexml")))
-        _join_initial_rooms(remote_bookmarks)
-    if not self.xmpp.anon and config.get('use_remote_bookmarks'):
-        bookmark.get_remote(self.xmpp, _join_remote_only)
-    # join all the available bookmarks. As of yet, this is just the local
-    # ones
-    _join_initial_rooms(bookmark.bookmarks)
+    self.bookmarks.get_local()
+    # join all the available bookmarks. As of yet, this is just the local ones
+    _join_initial_rooms(self, self.bookmarks)
 
     if config.get('enable_user_nick'):
         self.xmpp.plugin['xep_0172'].publish_nick(nick=self.own_nick, callback=dumb_callback)