Make bindings win over mappings

Fixes #2995

doc/help/settings.asciidoc

@@ -627,6 +627,7 @@ Default:
 This setting can be used to map keys to other keys.
 When the key used as dictionary-key is pressed, the binding for the key used as dictionary-value is invoked instead.
 This is useful for global remappings of keys, for example to map Ctrl-[ to Escape.
+Note that when a key is bound (via `bindings.default` or `bindings.commands`), the mapping is ignored.
 
 Type: <<types,Dict>>
 

qutebrowser/config/configdata.yml

@@ -1903,6 +1903,9 @@ bindings.key_mappings:
     This is useful for global remappings of keys, for example to map Ctrl-[ to
     Escape.
 
+    Note that when a key is bound (via `bindings.default` or
+    `bindings.commands`), the mapping is ignored.
+
 bindings.default:
   default:
     normal:

qutebrowser/keyinput/basekeyparser.py

@@ -122,6 +122,7 @@ class BaseKeyParser(QObject):
             self._debug_log("Ignoring only-modifier keyeevent.")
             return False
 
+        if binding not in self.special_bindings:
             key_mappings = config.val.bindings.key_mappings
             try:
                 binding = key_mappings['<{}>'.format(binding)][1:-1]
@@ -133,26 +134,28 @@ class BaseKeyParser(QObject):
         except KeyError:
             self._debug_log("No special binding found for {}.".format(binding))
             return False
-        count, _command = self._split_count()
+        count, _command = self._split_count(self._keystring)
         self.execute(cmdstr, self.Type.special, count)
         self.clear_keystring()
         return True
 
-    def _split_count(self):
+    def _split_count(self, keystring):
         """Get count and command from the current keystring.
 
+        Args:
+            keystring: The key string to split.
+
         Return:
             A (count, command) tuple.
         """
         if self._supports_count:
-            (countstr, cmd_input) = re.match(r'^(\d*)(.*)',
+            (countstr, cmd_input) = re.match(r'^(\d*)(.*)', keystring).groups()
-                                             self._keystring).groups()
             count = int(countstr) if countstr else None
             if count == 0 and not cmd_input:
-                cmd_input = self._keystring
+                cmd_input = keystring
                 count = None
         else:
-            cmd_input = self._keystring
+            cmd_input = keystring
             count = None
         return count, cmd_input
 
@@ -183,18 +186,17 @@ class BaseKeyParser(QObject):
             self._debug_log("Ignoring, no text char")
             return self.Match.none
 
-        key_mappings = config.val.bindings.key_mappings
+        count, cmd_input = self._split_count(self._keystring + txt)
-        txt = key_mappings.get(txt, txt)
+        match, binding = self._match_key(cmd_input)
-        self._keystring += txt
+        if match == self.Match.none:
-
+            mappings = config.val.bindings.key_mappings
-        count, cmd_input = self._split_count()
+            mapped = mappings.get(txt, None)
-
+            if mapped is not None:
-        if not cmd_input:
+                txt = mapped
-            # Only a count, no command yet, but we handled it
+                count, cmd_input = self._split_count(self._keystring + txt)
-            return self.Match.other
-
                 match, binding = self._match_key(cmd_input)
 
+        self._keystring += txt
         if match == self.Match.definitive:
             self._debug_log("Definitive match for '{}'.".format(
                 self._keystring))
@@ -207,6 +209,8 @@ class BaseKeyParser(QObject):
             self._debug_log("Giving up with '{}', no matches".format(
                 self._keystring))
             self.clear_keystring()
+        elif match == self.Match.other:
+            pass
         else:
             raise AssertionError("Invalid match value {!r}".format(match))
         return match
@@ -223,6 +227,9 @@ class BaseKeyParser(QObject):
                 binding: - None with Match.partial/Match.none.
                          - The found binding with Match.definitive.
         """
+        if not cmd_input:
+            # Only a count, no command yet, but we handled it
+            return (self.Match.other, None)
         # A (cmd_input, binding) tuple (k, v of bindings) or None.
         definitive_match = None
         partial_match = False

tests/unit/keyinput/conftest.py

@@ -31,6 +31,12 @@ BINDINGS = {'prompt': {'<Ctrl-a>': 'message-info ctrla',
             'command': {'foo': 'message-info bar',
                         '<Ctrl+X>': 'message-info ctrlx'},
             'normal': {'a': 'message-info a', 'ba': 'message-info ba'}}
+MAPPINGS = {
+    '<Ctrl+a>': 'a',
+    '<Ctrl+b>': '<Ctrl+a>',
+    'x': 'a',
+    'b': 'a',
+}
 
 
 @pytest.fixture
@@ -38,3 +44,4 @@ def keyinput_bindings(config_stub, key_config_stub):
     """Register some test bindings."""
     config_stub.val.bindings.default = {}
     config_stub.val.bindings.commands = dict(BINDINGS)
+    config_stub.val.bindings.key_mappings = dict(MAPPINGS)

tests/unit/keyinput/test_basekeyparser.py

@@ -91,8 +91,7 @@ class TestDebugLog:
 ])
 def test_split_count(config_stub, input_key, supports_count, expected):
     kp = basekeyparser.BaseKeyParser(0, supports_count=supports_count)
-    kp._keystring = input_key
+    assert kp._split_count(input_key) == expected
-    assert kp._split_count() == expected
 
 
 @pytest.mark.usefixtures('keyinput_bindings')
@@ -165,20 +164,14 @@ class TestSpecialKeys:
         keyparser._read_config('prompt')
 
     def test_valid_key(self, fake_keyevent_factory, keyparser):
-        if utils.is_mac:
+        modifier = Qt.MetaModifier if utils.is_mac else Qt.ControlModifier
-            modifier = Qt.MetaModifier
-        else:
-            modifier = Qt.ControlModifier
         keyparser.handle(fake_keyevent_factory(Qt.Key_A, modifier))
         keyparser.handle(fake_keyevent_factory(Qt.Key_X, modifier))
         keyparser.execute.assert_called_once_with(
             'message-info ctrla', keyparser.Type.special, None)
 
     def test_valid_key_count(self, fake_keyevent_factory, keyparser):
-        if utils.is_mac:
+        modifier = Qt.MetaModifier if utils.is_mac else Qt.ControlModifier
-            modifier = Qt.MetaModifier
-        else:
-            modifier = Qt.ControlModifier
         keyparser.handle(fake_keyevent_factory(5, text='5'))
         keyparser.handle(fake_keyevent_factory(Qt.Key_A, modifier, text='A'))
         keyparser.execute.assert_called_once_with(
@@ -199,6 +192,22 @@ class TestSpecialKeys:
         keyparser.handle(fake_keyevent_factory(Qt.Key_A, Qt.NoModifier))
         assert not keyparser.execute.called
 
+    def test_mapping(self, config_stub, fake_keyevent_factory, keyparser):
+        modifier = Qt.MetaModifier if utils.is_mac else Qt.ControlModifier
+
+        keyparser.handle(fake_keyevent_factory(Qt.Key_B, modifier))
+        keyparser.execute.assert_called_once_with(
+            'message-info ctrla', keyparser.Type.special, None)
+
+    def test_binding_and_mapping(self, config_stub, fake_keyevent_factory,
+                                 keyparser):
+        """with a conflicting binding/mapping, the binding should win."""
+        modifier = Qt.MetaModifier if utils.is_mac else Qt.ControlModifier
+
+        keyparser.handle(fake_keyevent_factory(Qt.Key_A, modifier))
+        keyparser.execute.assert_called_once_with(
+            'message-info ctrla', keyparser.Type.special, None)
+
 
 class TestKeyChain:
 
@@ -230,7 +239,7 @@ class TestKeyChain:
         handle_text((Qt.Key_X, 'x'),
                     # Then start the real chain
                     (Qt.Key_B, 'b'), (Qt.Key_A, 'a'))
-        keyparser.execute.assert_called_once_with(
+        keyparser.execute.assert_called_with(
             'message-info ba', keyparser.Type.chain, None)
         assert keyparser._keystring == ''
 
@@ -249,6 +258,16 @@ class TestKeyChain:
         handle_text((Qt.Key_C, 'c'))
         assert keyparser._keystring == ''
 
+    def test_mapping(self, config_stub, handle_text, keyparser):
+        handle_text((Qt.Key_X, 'x'))
+        keyparser.execute.assert_called_once_with(
+            'message-info a', keyparser.Type.chain, None)
+
+    def test_binding_and_mapping(self, config_stub, handle_text, keyparser):
+        """with a conflicting binding/mapping, the binding should win."""
+        handle_text((Qt.Key_B, 'b'))
+        assert not keyparser.execute.called
+
 
 class TestCount: