Working on tests.

rtv/exceptions.py

@@ -10,6 +10,14 @@ class RTVError(Exception):
     "Base RTV error class"
 
 
+class KeystringError(RTVError):
+    "Unable to parse key string"
+
+
+class ConfigError(RTVError):
+    "There was a problem with the configuration"
+
+
 class AccountError(RTVError):
     "Could not access user account"
 

rtv/objects.py

@@ -12,7 +12,6 @@ import logging
 import threading
 from curses import ascii
 from contextlib import contextmanager
-from collections import defaultdict
 
 import six
 import praw
@@ -530,14 +529,23 @@ class Controller(object):
         # to check if any parent controllers have registered the keypress
         self.parents = inspect.getmro(type(self))[:-1]
 
-        # Update the character map with the bindings defined in the keymap
         if not keymap:
-            for controller in [self] + self.parents:
+            return
-                for binding, func in controller.character_map.items():
+
-                    if isinstance(binding, KeyBinding):
+        # Go through the controller and all of it's parents and look for
-                        # TODO: Raise error if trying to overwrite a char
+        # Command objects in the character map. Use the keymap the lookup the
-                        mappings = {char: func for char in keymap.get(binding)}
+        # keys associated with those command objects and add them to the
-                        self.character_map.update(mappings)
+        # character map.
+        for controller in self.parents:
+            for command, func in controller.character_map.copy().items():
+                if isinstance(command, Command):
+                    for key in keymap.get(command):
+                        if key in controller.character_map:
+                            raise exceptions.ConfigError(
+                                'Invalid configuration, cannot bind the `%s`'
+                                ' key to two different commands in the `%s`'
+                                ' context' % (key, self.__class__.__name__))
+                        controller.character_map[key] = func
 
     def trigger(self, char, *args, **kwargs):
 
@@ -571,32 +579,57 @@ class Controller(object):
         return inner
 
 
-class KeyBinding(object):
+class Command(object):
+    """
+    Minimal class that should be used to wrap abstract commands that may be
+    implemented as one or more physical keystrokes.
+
+    E.g. Command("REFRESH") can be represented by the KeyMap to be triggered
+         by either `r` or `F5`
+    """
 
     def __init__(self, val):
         self.val = val.upper()
 
+    def __repr__(self):
+        return 'Command(%s)' % self.val
 
-class KeyMapMeta(type):
+    def __eq__(self, other):
+        return repr(self) == repr(other)
 
-    def __getattr__(cls, key):
+    def __ne__(self, other):
-        return KeyBinding(key)
+        return not self == other
+
+    def __hash__(self):
+        return hash(repr(self))
 
 
-@six.add_metaclass(KeyMapMeta)
 class KeyMap(object):
+    """
+    Mapping between commands and the keys that they represent.
+    """
 
     def __init__(self, bindings):
-        self._bindings = defaultdict(list)
+        self.set_bindings(bindings)
-        self.update(bindings)
 
-    def update(self, **bindings):
+    def set_bindings(self, bindings):
-        for command, keys in bindings:
+        # Clear the keymap before applying the bindings to avoid confusion.
-            binding = KeyBinding(command)
+        # If a user defines custom bindings in their config file, they must
-            self._bindings[binding] = [self._parse_key(key) for key in keys]
+        # explicitly define ALL of the bindings.
+        self._keymap = {}
+        for command, keys in bindings.items():
+            if not isinstance(command, Command):
+                command = Command(command)
+            self._keymap[command] = [self._parse_key(key) for key in keys]
 
-    def get(self, binding):
+    def get(self, command):
-        return self._bindings[binding]
+        if not isinstance(command, Command):
+            command = Command(command)
+        try:
+            return self._keymap[command]
+        except KeyError:
+            raise exceptions.ConfigError(
+                'Invalid configuration, `%s` key undefined' % command.val)
 
     @staticmethod
     def _parse_key(key):
@@ -604,17 +637,27 @@ class KeyMap(object):
         Parse a key represented by a string and return its character code.
         """
 
-        if isinstance(key, int):
+        try:
-            return key
+            if isinstance(key, int):
-        elif re.match('[<]KEY_.*[>]', key):
+                return key
-            # Curses control character
+            elif re.match('[<]KEY_.*[>]', key):
-            return getattr(curses, key[1:-1])
+                # Curses control character
-        elif re.match('[<].*[>]', key):
+                return getattr(curses, key[1:-1])
-            # Ascii control character
+            elif re.match('[<].*[>]', key):
-            return getattr(ascii, key[1:-1])
+                # Ascii control character
-        elif key.startswith('0x'):
+                return getattr(ascii, key[1:-1])
-            # Ascii hex code
+            elif key.startswith('0x'):
-            return int(key, 16)
+                # Ascii hex code
-        else:
+                return int(key, 16)
-            # Ascii character
+            else:
-            return ord(key)
+                # Ascii character
+                code = ord(key)
+                if 0 <= code <= 255:
+                    return code
+                # Python 3.3 has a curses.get_wch() function that we can use
+                # for unicode keys, but Python 2.7 is limited to ascii.
+                raise exceptions.ConfigError(
+                    'Invalid key `%s`, must be in the ascii range' % key)
+
+        except (AttributeError, ValueError, TypeError):
+            raise exceptions.ConfigError('Invalid key string `%s`' % key)

rtv/page.py

@@ -9,7 +9,7 @@ from functools import wraps
 from kitchen.text.display import textual_width
 
 from . import docs
-from .objects import Controller, Color, KeyMap
+from .objects import Controller, Color, Command
 
 
 def logged_in(f):
@@ -68,60 +68,60 @@ class Page(object):
             ch = self.term.stdscr.getch()
             self.controller.trigger(ch)
 
-    @PageController.register(KeyMap.EXIT)
+    @PageController.register(Command('EXIT'))
     def exit(self):
         if self.term.prompt_y_or_n('Do you really want to quit? (y/n): '):
             sys.exit()
 
-    @PageController.register(KeyMap.FORCE_EXIT)
+    @PageController.register(Command('FORCE_EXIT'))
     def force_exit(self):
         sys.exit()
 
-    @PageController.register(KeyMap.HELP)
+    @PageController.register(Command('HELP'))
     def show_help(self):
         self.term.show_notification(docs.HELP.strip().splitlines())
 
-    @PageController.register(KeyMap.SORT_HOT)
+    @PageController.register(Command('SORT_HOT'))
     def sort_content_hot(self):
         self.refresh_content(order='hot')
 
-    @PageController.register(KeyMap.SORT_TOP)
+    @PageController.register(Command('SORT_TOP'))
     def sort_content_top(self):
         self.refresh_content(order='top')
 
-    @PageController.register(KeyMap.SORT_RISING)
+    @PageController.register(Command('SORT_RISING'))
     def sort_content_rising(self):
         self.refresh_content(order='rising')
 
-    @PageController.register(KeyMap.SORT_NEW)
+    @PageController.register(Command('SORT_NEW'))
     def sort_content_new(self):
         self.refresh_content(order='new')
 
-    @PageController.register(KeyMap.SORT_CONTROVERSIAL)
+    @PageController.register(Command('SORT_CONTROVERSIAL'))
     def sort_content_controversial(self):
         self.refresh_content(order='controversial')
 
-    @PageController.register(KeyMap.MOVE_UP)
+    @PageController.register(Command('MOVE_UP'))
     def move_cursor_up(self):
         self._move_cursor(-1)
         self.clear_input_queue()
 
-    @PageController.register(KeyMap.MOVE_DOWN)
+    @PageController.register(Command('MOVE_DOWN'))
     def move_cursor_down(self):
         self._move_cursor(1)
         self.clear_input_queue()
 
-    @PageController.register(KeyMap.PAGE_UP)
+    @PageController.register(Command('PAGE_UP'))
     def move_page_up(self):
         self._move_page(-1)
         self.clear_input_queue()
 
-    @PageController.register(KeyMap.PAGE_DOWN)
+    @PageController.register(Command('PAGE_DOWN'))
     def move_page_down(self):
         self._move_page(1)
         self.clear_input_queue()
 
-    @PageController.register(KeyMap.UPVOTE)
+    @PageController.register(Command('UPVOTE'))
     @logged_in
     def upvote(self):
         data = self.content.get(self.nav.absolute_index)
@@ -138,7 +138,7 @@ class Page(object):
             if not self.term.loader.exception:
                 data['likes'] = True
 
-    @PageController.register(KeyMap.DOWNVOTE)
+    @PageController.register(Command('DOWNVOTE'))
     @logged_in
     def downvote(self):
         data = self.content.get(self.nav.absolute_index)
@@ -155,7 +155,7 @@ class Page(object):
             if not self.term.loader.exception:
                 data['likes'] = None
 
-    @PageController.register(KeyMap.LOGIN)
+    @PageController.register(Command('LOGIN'))
     def login(self):
         """
         Prompt to log into the user's account, or log out of the current
@@ -169,7 +169,7 @@ class Page(object):
         else:
             self.oauth.authorize()
 
-    @PageController.register(KeyMap.DELETE)
+    @PageController.register(Command('DELETE'))
     @logged_in
     def delete_item(self):
         """
@@ -193,7 +193,7 @@ class Page(object):
         if self.term.loader.exception is None:
             self.refresh_content()
 
-    @PageController.register(KeyMap.EDIT)
+    @PageController.register(Command('EDIT'))
     @logged_in
     def edit(self):
         """
@@ -228,7 +228,7 @@ class Page(object):
         if self.term.loader.exception is None:
             self.refresh_content()
 
-    @PageController.register(KeyMap.INBOX)
+    @PageController.register(Command('INBOX'))
     @logged_in
     def get_inbox(self):
         """

rtv/submission.py

@@ -7,7 +7,7 @@ import curses
 from . import docs
 from .content import SubmissionContent
 from .page import Page, PageController, logged_in
-from .objects import Navigator, Color, KeyMap
+from .objects import Navigator, Color, Command
 
 
 class SubmissionController(PageController):
@@ -24,11 +24,11 @@ class SubmissionPage(Page):
         else:
             self.content = SubmissionContent(submission, term.loader)
 
-        self.controller = SubmissionController(self)
+        self.controller = SubmissionController(self, keymap=config.keymap)
         # Start at the submission post, which is indexed as -1
         self.nav = Navigator(self.content.get, page_index=-1)
 
-    @SubmissionController.register(KeyMap.SUBMISSION_TOGGLE_COMMENT)
+    @SubmissionController.register(Command('SUBMISSION_TOGGLE_COMMENT'))
     def toggle_comment(self):
         "Toggle the selected comment tree between visible and hidden"
 
@@ -40,13 +40,13 @@ class SubmissionPage(Page):
             # causes the cursor index to go out of bounds.
             self.nav.page_index, self.nav.cursor_index = current_index, 0
 
-    @SubmissionController.register(KeyMap.SUBMISSION_EXIT)
+    @SubmissionController.register(Command('SUBMISSION_EXIT'))
     def exit_submission(self):
         "Close the submission and return to the subreddit page"
 
         self.active = False
 
-    @SubmissionController.register(KeyMap.REFRESH)
+    @SubmissionController.register(Command('REFRESH'))
     def refresh_content(self, order=None, name=None):
         "Re-download comments and reset the page index"
 
@@ -59,7 +59,7 @@ class SubmissionPage(Page):
         if not self.term.loader.exception:
             self.nav = Navigator(self.content.get, page_index=-1)
 
-    @SubmissionController.register(KeyMap.SUBMISSION_OPEN_IN_BROWSER)
+    @SubmissionController.register(Command('SUBMISSION_OPEN_IN_BROWSER'))
     def open_link(self):
         "Open the selected item with the webbrowser"
 
@@ -70,7 +70,7 @@ class SubmissionPage(Page):
         else:
             self.term.flash()
 
-    @SubmissionController.register(KeyMap.SUBMISSION_POST)
+    @SubmissionController.register(Command('SUBMISSION_POST'))
     @logged_in
     def add_comment(self):
         """
@@ -113,7 +113,7 @@ class SubmissionPage(Page):
         if not self.term.loader.exception:
             self.refresh_content()
 
-    @SubmissionController.register(KeyMap.DELETE)
+    @SubmissionController.register(Command('DELETE'))
     @logged_in
     def delete_comment(self):
         "Delete a comment as long as it is not the current submission"

rtv/subreddit.py

@@ -7,7 +7,7 @@ import curses
 from . import docs
 from .content import SubredditContent
 from .page import Page, PageController, logged_in
-from .objects import Navigator, Color, KeyMap
+from .objects import Navigator, Color, Command
 from .submission import SubmissionPage
 from .subscription import SubscriptionPage
 from .terminal import Terminal
@@ -28,10 +28,10 @@ class SubredditPage(Page):
         super(SubredditPage, self).__init__(reddit, term, config, oauth)
 
         self.content = SubredditContent.from_name(reddit, name, term.loader)
-        self.controller = SubredditController(self)
+        self.controller = SubredditController(self, keymap=config.keymap)
         self.nav = Navigator(self.content.get)
 
-    @SubredditController.register(KeyMap.REFRESH)
+    @SubredditController.register(Command('REFRESH'))
     def refresh_content(self, order=None, name=None):
         "Re-download all submissions and reset the page index"
 
@@ -49,7 +49,7 @@ class SubredditPage(Page):
         if not self.term.loader.exception:
             self.nav = Navigator(self.content.get)
 
-    @SubredditController.register(KeyMap.SUBREDDIT_SEARCH)
+    @SubredditController.register(Command('SUBREDDIT_SEARCH'))
     def search_subreddit(self, name=None):
         "Open a prompt to search the given subreddit"
 
@@ -65,7 +65,7 @@ class SubredditPage(Page):
         if not self.term.loader.exception:
             self.nav = Navigator(self.content.get)
 
-    @SubredditController.register(KeyMap.SUBREDDIT_PROMPT)
+    @SubredditController.register(Command('SUBREDDIT_PROMPT'))
     def prompt_subreddit(self):
         "Open a prompt to navigate to a different subreddit"
 
@@ -73,7 +73,7 @@ class SubredditPage(Page):
         if name is not None:
             self.refresh_content(order='ignore', name=name)
 
-    @SubredditController.register(KeyMap.SUBREDDIT_OPEN)
+    @SubredditController.register(Command('SUBREDDIT_OPEN'))
     def open_submission(self, url=None):
         "Select the current submission to view posts"
 
@@ -93,7 +93,7 @@ class SubredditPage(Page):
         if data.get('url_type') == 'selfpost':
             self.config.history.add(data['url_full'])
 
-    @SubredditController.register(KeyMap.SUBREDDIT_OPEN_IN_BROWSER)
+    @SubredditController.register(Command('SUBREDDIT_OPEN_IN_BROWSER'))
     def open_link(self):
         "Open a link with the webbrowser"
 
@@ -107,7 +107,7 @@ class SubredditPage(Page):
             self.term.open_browser(data['url_full'])
             self.config.history.add(data['url_full'])
 
-    @SubredditController.register(KeyMap.SUBREDDIT_POST)
+    @SubredditController.register(Command('SUBREDDIT_POST'))
     @logged_in
     def post_submission(self):
         "Post a new submission to the given subreddit"
@@ -145,7 +145,7 @@ class SubredditPage(Page):
 
         self.refresh_content()
 
-    @SubredditController.register(KeyMap.SUBREDDIT_OPEN_SUBSCRIPTIONS)
+    @SubredditController.register(Command('SUBREDDIT_OPEN_SUBSCRIPTIONS'))
     @logged_in
     def open_subscriptions(self):
         "Open user subscriptions page"

rtv/subscription.py

@@ -5,7 +5,7 @@ import curses
 
 from .page import Page, PageController
 from .content import SubscriptionContent
-from .objects import Color, Navigator, KeyMap
+from .objects import Color, Navigator, Command
 
 
 class SubscriptionController(PageController):
@@ -18,11 +18,11 @@ class SubscriptionPage(Page):
         super(SubscriptionPage, self).__init__(reddit, term, config, oauth)
 
         self.content = SubscriptionContent.from_user(reddit, term.loader)
-        self.controller = SubscriptionController(self)
+        self.controller = SubscriptionController(self, keymap=config.keymap)
         self.nav = Navigator(self.content.get)
         self.subreddit_data = None
 
-    @SubscriptionController.register(KeyMap.REFRESH)
+    @SubscriptionController.register(Command('REFRESH'))
     def refresh_content(self, order=None, name=None):
         "Re-download all subscriptions and reset the page index"
 
@@ -37,14 +37,14 @@ class SubscriptionPage(Page):
         if not self.term.loader.exception:
             self.nav = Navigator(self.content.get)
 
-    @SubscriptionController.register(KeyMap.SUBSCRIPTION_SELECT)
+    @SubscriptionController.register(Command('SUBSCRIPTION_SELECT'))
     def select_subreddit(self):
         "Store the selected subreddit and return to the subreddit page"
 
         self.subreddit_data = self.content.get(self.nav.absolute_index)
         self.active = False
 
-    @SubscriptionController.register(KeyMap.SUBSCRIPTION_CLOSE)
+    @SubscriptionController.register(Command('SUBSCRIPTION_CLOSE'))
     def close_subscriptions(self):
         "Close subscriptions and return to the subreddit page"
 

tests/test_config.py

@@ -88,25 +88,35 @@ def test_config_from_file():
         'link': 'https://reddit.com/permalink •',
         'subreddit': 'cfb'}
 
+    bindings = {
+        'REFRESH': 'r, <KEY_F5>',
+        'UPVOTE': ''}
+
     with NamedTemporaryFile(suffix='.cfg') as fp:
 
-        fargs = Config.get_file(filename=fp.name)
+        fargs, fbindings = Config.get_file(filename=fp.name)
         config = Config(**fargs)
+        config.keymap.set_bindings(fbindings)
         assert config.config == {}
+        assert config.keymap._keymap == {}
 
+        # [rtv]
         rows = ['{0}={1}'.format(key, val) for key, val in args.items()]
         data = '\n'.join(['[rtv]'] + rows)
         fp.write(codecs.encode(data, 'utf-8'))
+
+        # [bindings]
+        rows = ['{0}={1}'.format(key, val) for key, val in bindings.items()]
+        data = '\n'.join(['', '', '[bindings]'] + rows)
+        fp.write(codecs.encode(data, 'utf-8'))
+
         fp.flush()
-        fargs = Config.get_file(filename=fp.name)
+        fargs, fbindings = Config.get_file(filename=fp.name)
         config.update(**fargs)
+        config.keymap.set_bindings(fbindings)
         assert config.config == args
-
+        assert config.keymap.get('REFRESH') == [ord('r'), 269]
-
+        assert config.keymap.get('UPVOTE') == []
-def test_config_keys():
-
-    config = Config()
-    pass
 
 
 def test_config_refresh_token():

tests/test_objects.py

@@ -4,10 +4,13 @@ from __future__ import unicode_literals
 import time
 import curses
 
+import six
 import pytest
 import requests
 
-from rtv.objects import Color, Controller, Navigator, curses_session
+from rtv import exceptions
+from rtv.objects import Color, Controller, Navigator, Command, KeyMap, \
+    curses_session
 
 try:
     from unittest import mock
@@ -246,6 +249,88 @@ def test_objects_controller():
     assert controller_c.trigger('3') is None
 
 
+def test_objects_controller_command():
+
+    class ControllerA(Controller):
+        character_map = {}
+
+    class ControllerB(ControllerA):
+        character_map = {}
+
+    @ControllerA.register(Command('REFRESH'))
+    def call_page(_):
+        return 'a1'
+
+    @ControllerA.register(Command('UPVOTE'))
+    def call_page(_):
+        return 'a2'
+
+    @ControllerB.register(Command('REFRESH'))
+    def call_page(_):
+        return 'b1'
+
+    # Two commands aren't allowed to share keys
+    keymap = KeyMap({'REFRESH': [0x10, 0x11], 'UPVOTE': [0x11, 0x12]})
+    with pytest.raises(exceptions.ConfigError) as e:
+        ControllerA(None, keymap=keymap)
+    assert 'ControllerA' in six.text_type(e)
+
+    # Reset the character map
+    ControllerA.character_map = {Command('REFRESH'): int, Command('UPVOTE'):int}
+
+    # All commands must be defined in the keymap
+    keymap = KeyMap({'REFRESH': [0x10]})
+    with pytest.raises(exceptions.ConfigError) as e:
+        ControllerB(None, keymap=keymap)
+    assert 'UPVOTE' in six.text_type(e)
+
+
+def test_objects_command():
+
+    c1 = Command("REFRESH")
+    c2 = Command("refresh")
+    c3 = Command("EXIT")
+
+    assert c1 == c2
+    assert c1 != c3
+
+    keymap = {c1: None, c2: None, c3: None}
+    assert len(keymap) == 2
+    assert c1 in keymap
+    assert c2 in keymap
+    assert c3 in keymap
+
+
+def test_objects_keymap():
+
+    bindings = {
+        'refresh': ['a', 0x12, '<LF>', '<KEY_UP>'],
+        'exit': [],
+        Command('UPVOTE'): ['b', '<KEY_F5>']
+    }
+
+    keymap = KeyMap(bindings)
+    assert keymap.get(Command('REFRESH')) == [97, 18, 10, 259]
+    assert keymap.get(Command('exit')) == []
+    assert keymap.get('upvote') == [98, 269]
+    with pytest.raises(exceptions.ConfigError) as e:
+        keymap.get('downvote')
+    assert 'Command(DOWNVOTE)' in six.text_type(e)
+
+    # Updating the bindings wipes out the old ones
+    bindings = {'refresh': ['a', 0x12, '<LF>', '<KEY_UP>']}
+    keymap.set_bindings(bindings)
+    assert keymap.get('refresh')
+    with pytest.raises(exceptions.ConfigError) as e:
+        keymap.get('upvote')
+    assert 'Command(UPVOTE)' in six.text_type(e)
+
+    for key in ('', None, '<lf>', '<DNS>', '<KEY_UD>', '♬'):
+        with pytest.raises(exceptions.ConfigError) as e:
+            keymap.set_bindings({'refresh': [key]})
+        assert six.text_type(key) in six.text_type(e)
+
+
 def test_objects_navigator_properties():
 
     def valid_page_cb(_):