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

4 files changed, 116 insertions, 102 deletions

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)