Refactor command/key handling again

qutebrowser/app.py

@@ -33,12 +33,13 @@ class QuteBrowser(QApplication):
 
         self.mainwindow = MainWindow()
         self.commandparser = cmdutils.CommandParser()
-        self.keyparser = KeyParser(self.mainwindow)
+        self.keyparser = KeyParser(self.mainwindow, self.commandparser)
 
         self.aboutToQuit.connect(self.config.save)
         self.mainwindow.tabs.keypress.connect(self.keyparser.handle)
         self.keyparser.set_cmd_text.connect(self.mainwindow.status.cmd.set_cmd)
-        self.mainwindow.status.cmd.got_cmd.connect(self.commandparser.parse)
+        self.mainwindow.status.cmd.got_cmd.connect(
+            self.commandparser.parse_check_run)
         self.mainwindow.status.cmd.got_cmd.connect(
             self.mainwindow.tabs.setFocus)
         self.commandparser.error.connect(self.mainwindow.status.disp_error)

qutebrowser/commands/keys.py

@@ -3,7 +3,7 @@ import re
 
 from PyQt5.QtCore import QObject, pyqtSignal
 
-import qutebrowser.commands.utils as cmdutils
+from qutebrowser.commands.utils import ArgumentCountError
 
 class KeyParser(QObject):
     """Parser for vim-like key sequences"""
@@ -13,12 +13,17 @@ class KeyParser(QObject):
     # Signal emitted when the keystring is updated
     keystring_updated = pyqtSignal(str)
     # Keybindings
-    key_to_cmd = {}
+    bindings = {}
+    cmdparser = None
 
     MATCH_PARTIAL = 0
     MATCH_DEFINITIVE = 1
     MATCH_NONE = 2
 
+    def __init__(self, mainwindow, commandparser):
+        super().__init__(mainwindow)
+        self.cmdparser = commandparser
+
     def from_config_sect(self, sect):
         """Loads keybindings from a ConfigParser section, in the config format
         key = command, e.g.
@@ -26,7 +31,7 @@ class KeyParser(QObject):
         """
         for (key, cmd) in sect.items():
             logging.debug('registered key: {} -> {}'.format(key, cmd))
-            self.key_to_cmd[key] = cmdutils.cmd_dict[cmd]
+            self.bindings[key] = cmd
 
     def handle(self, e):
         """Wrapper for _handle to emit keystring_updated after _handle"""
@@ -51,9 +56,10 @@ class KeyParser(QObject):
             self.keystring = ''
             return
 
-        (countstr, cmdstr) = re.match('^(\d*)(.*)', self.keystring).groups()
+        (countstr, cmdstr_needle) = re.match('^(\d*)(.*)',
+                                             self.keystring).groups()
 
-        if not cmdstr:
+        if not cmdstr_needle:
             return
 
         # FIXME this doesn't handle ambigious keys correctly.
@@ -62,7 +68,7 @@ class KeyParser(QObject):
         # configurable timeout, which triggers cmd.run() when expiring. Then
         # when we enter _handle() again in time we stop the timer.
 
-        (match, cmd) = self._match_key(cmdstr)
+        (match, cmdstr_hay) = self._match_key(cmdstr_needle)
 
         if match == self.MATCH_DEFINITIVE:
             pass
@@ -79,28 +85,33 @@ class KeyParser(QObject):
         self.keystring = ''
         count = int(countstr) if countstr else None
 
-        if cmd.nargs and cmd.nargs != 0:
+        # If we get a ValueError (invalid cmd) here, something is very wrong,
-            logging.debug('Filling statusbar with partial command {}'.format(
+        # so we don't catch it
-                cmd.name))
-            self.set_cmd_text.emit(':{} '.format(cmd.name))
-        elif count is not None:
-            cmd.run(count=count)
-        else:
-            cmd.run()
 
-    def _match_key(self, cmdstr):
+        (cmd, args) = self.cmdparser.parse(cmdstr_hay)
+        try:
+            self.cmdparser.check(cmd, args)
+        except ArgumentCountError:
+            logging.debug('Filling statusbar with partial command {}'.format(
+                cmdstr_hay))
+            self.set_cmd_text.emit(':{} '.format(cmdstr_hay))
+            return
+        self.cmdparser.run(cmd, args, count=count)
+
+    def _match_key(self, cmdstr_needle):
         """Tries to match a given cmdstr with any defined command"""
         try:
-            cmd = self.key_to_cmd[cmdstr]
+            cmdstr_hay = self.bindings[cmdstr_needle]
-            return (self.MATCH_DEFINITIVE, cmd)
+            return (self.MATCH_DEFINITIVE, cmdstr_hay)
         except KeyError:
             # No definitive match, check if there's a chance of a partial match
-            for cmd in self.key_to_cmd:
+            for hay in self.bindings:
                 try:
-                    if cmdstr[-1] == cmd[len(cmdstr) - 1]:
+                    if cmdstr_needle[-1] == hay[len(cmdstr_needle) - 1]:
                         return (self.MATCH_PARTIAL, None)
                 except IndexError:
-                    # current cmd is shorter than our cmdstr, so it won't match
+                    # current cmd is shorter than our cmdstr_needle, so it
+                    # won't match
                     continue
             # no definitive and no partial matches if we arrived here
             return (self.MATCH_NONE, None)

qutebrowser/commands/utils.py

@@ -23,6 +23,10 @@ def register_all():
         cmd_dict[obj.name] = obj
 
 class CommandParser(QObject):
+    # FIXME
+    #
+    # this should work differently somehow, e.g. more than one instance, and
+    # remember args/cmd in between.
     """Parser for qutebrowser commandline commands"""
     error = pyqtSignal(str) # Emitted if there's an error
 
@@ -32,26 +36,41 @@ class CommandParser(QObject):
 
         # FIXME maybe we should handle unambigious shorthands for commands
         # here? Or at least we should add :q for :quit.
-        cmd = parts[0]
+        cmdstr = parts[0]
         try:
-            obj = cmd_dict[cmd]
+            cmd = cmd_dict[cmdstr]
         except KeyError:
-            self.error.emit("{}: no such command".format(cmd))
+            self.error.emit("{}: no such command".format(cmdstr))
-            return
+            raise ValueError
 
         if len(parts) == 1:
             args = []
-        elif obj.split_args:
+        elif cmd.split_args:
             args = shlex.split(parts[1])
         else:
             args = [parts[1]]
+        return (cmd, args)
 
+    def check(self, cmd, args):
         try:
-            obj.check(args)
+            cmd.check(args)
         except ArgumentCountError:
             self.error.emit("{}: invalid argument count".format(cmd))
+            raise
+
+    def parse_check_run(self, text, count=None):
+        try:
+            (cmd, args) = self.parse(text)
+            self.check(cmd, args)
+        except (ArgumentCountError, ValueError):
             return
-        obj.run(args)
+        self.run(cmd, args)
+
+    def run(self, cmd, args, count=None):
+        if count is not None:
+            cmd.run(args, count=count)
+        else:
+            cmd.run(args)
 
 class Command(QObject):
     """Base skeleton for a command. See the module help for
@@ -89,7 +108,7 @@ class Command(QObject):
         if args:
             dbgout += args
         if count is not None:
-            dbgout.append("* {}".format(count))
+            dbgout.append("(count={})".format(count))
         logging.debug(' '.join(dbgout))
 
         argv = [self.name]