From 3a11a24be022cbc9115b49f3f63707a65a425b19 Mon Sep 17 00:00:00 2001
From: Florian Bruhin <git@the-compiler.org>
Date: Thu, 1 Mar 2018 23:04:00 +0100
Subject: [PATCH] Fix modifier handling

We don't want to show <Shift-Shift>, but <Ctrl-Shift> should still work
correctly.
---
 qutebrowser/keyinput/keyutils.py     | 22 +++++++++++-----------
 tests/unit/keyinput/test_keyutils.py | 26 ++++++++++++--------------
 2 files changed, 23 insertions(+), 25 deletions(-)

diff --git a/qutebrowser/keyinput/keyutils.py b/qutebrowser/keyinput/keyutils.py
index 85c35895a..1cb941ac7 100644
--- a/qutebrowser/keyinput/keyutils.py
+++ b/qutebrowser/keyinput/keyutils.py
@@ -33,14 +33,6 @@ def is_printable(key):
     return key <= 0xff
 
 
-def is_modifier_key(key):
-    # FIXME docs
-    return key in (Qt.Key_Control, Qt.Key_Alt, Qt.Key_Shift, Qt.Key_Meta,
-                   Qt.Key_AltGr, Qt.Key_Super_L, Qt.Key_Super_R,
-                   Qt.Key_Hyper_L, Qt.Key_Hyper_R, Qt.Key_Direction_L,
-                   Qt.Key_Direction_R)
-
-
 def _key_to_string(key):
     """Convert a Qt::Key member to a meaningful name.
 
@@ -203,10 +195,18 @@ class KeyInfo:
             A name of the key (combination) as a string.
         """
         key_string = _key_to_string(self.key)
+        modifier_map = {
+            Qt.Key_Shift: Qt.ShiftModifier,
+            Qt.Key_Control: Qt.ControlModifier,
+            Qt.Key_Alt: Qt.AltModifier,
+            Qt.Key_Meta: Qt.MetaModifier,
+            Qt.Key_Mode_switch: Qt.GroupSwitchModifier,
+        }
+        modifiers = int(self.modifiers)
 
-        if is_modifier_key(self.key):
+        if self.key in modifier_map:
             # Don't return e.g. <Shift+Shift>
-            return '<{}>'.format(key_string)
+            modifiers &= ~modifier_map[self.key]
         elif is_printable(self.key):
             # "normal" binding
             # FIXME Add a test to make sure Tab doesn't become TAB
@@ -220,7 +220,7 @@ class KeyInfo:
                 key_string = key_string.lower()
 
         # "special" binding
-        modifier_string = QKeySequence(self.modifiers).toString()
+        modifier_string = QKeySequence(modifiers).toString()
         return '<{}{}>'.format(modifier_string, key_string)
 
     def text(self):
diff --git a/tests/unit/keyinput/test_keyutils.py b/tests/unit/keyinput/test_keyutils.py
index bf24de401..25fe8e50e 100644
--- a/tests/unit/keyinput/test_keyutils.py
+++ b/tests/unit/keyinput/test_keyutils.py
@@ -115,12 +115,6 @@ class TestKeyEventToString:
                                     modifiers=Qt.ControlModifier)
         assert str(keyutils.KeyInfo.from_event(evt)) == '<Control>'
 
-    def test_only_hyper_l(self, fake_keyevent_factory):
-        """Test keyeevent when only Hyper_L is pressed."""
-        evt = fake_keyevent_factory(key=Qt.Key_Hyper_L,
-                                    modifiers=Qt.MetaModifier)
-        assert str(keyutils.KeyInfo.from_event(evt)) == '<Hyper L>'
-
     def test_only_key(self, fake_keyevent_factory):
         """Test with a simple key pressed."""
         evt = fake_keyevent_factory(key=Qt.Key_A)
@@ -140,6 +134,18 @@ class TestKeyEventToString:
         s = str(keyutils.KeyInfo.from_event(evt))
         assert s == '<Meta+Ctrl+Alt+Shift+a>'
 
+    def test_modifier_key(self, fake_keyevent_factory):
+        evt = fake_keyevent_factory(key=Qt.Key_Shift,
+                                    modifiers=Qt.ShiftModifier)
+        s = str(keyutils.KeyInfo.from_event(evt))
+        assert s == '<Shift>'
+
+    def test_modifier_key_with_modifiers(self, fake_keyevent_factory):
+        evt = fake_keyevent_factory(key=Qt.Key_Shift,
+                                    modifiers=(Qt.ShiftModifier |
+                                               Qt.ControlModifier))
+        s = str(keyutils.KeyInfo.from_event(evt))
+        assert s == '<Ctrl+Shift>'
 
 @pytest.mark.parametrize('keystr, expected', [
     ('<Control-x>', keyutils.KeySequence(Qt.ControlModifier | Qt.Key_X)),
@@ -184,11 +190,3 @@ def test_normalize_keystr(orig, normalized):
 ])
 def test_is_printable(key, printable):
     assert keyutils.is_printable(key) == printable
-
-
-@pytest.mark.parametrize('key, ismodifier', [
-    (Qt.Key_Control, True),
-    (Qt.Key_X, False)
-])
-def test_is_modifier_key(key, ismodifier):
-    assert keyutils.is_modifier_key(key) == ismodifier