From dd9470af94afffd177fe19375d756af2d854a1ce Mon Sep 17 00:00:00 2001
From: Ryan Roden-Corrent <ryan@rcorre.net>
Date: Mon, 1 Aug 2016 12:11:26 -0400
Subject: [PATCH 1/7] Show hidden commands in help completion.

Hidden commands are not shown in command completion as they typically
would not be run directly. However, a user might still might like to see
help for them if, for example, they are writing a script or creating a
binding.

Addresses #1707.
---
 qutebrowser/completion/models/miscmodels.py | 4 ++--
 tests/unit/completion/test_models.py        | 5 +++--
 2 files changed, 5 insertions(+), 4 deletions(-)

diff --git a/qutebrowser/completion/models/miscmodels.py b/qutebrowser/completion/models/miscmodels.py
index bbbaa8a9d..56d436079 100644
--- a/qutebrowser/completion/models/miscmodels.py
+++ b/qutebrowser/completion/models/miscmodels.py
@@ -75,8 +75,8 @@ class HelpCompletionModel(base.BaseCompletionModel):
         assert cmdutils.cmd_dict
         cmdlist = []
         for obj in set(cmdutils.cmd_dict.values()):
-            if (obj.hide or (obj.debug and not objreg.get('args').debug) or
-                    obj.deprecated):
+            if ((obj.debug and not objreg.get('args').debug) or
+                obj.deprecated):
                 pass
             else:
                 cmdlist.append((':' + obj.name, obj.desc))
diff --git a/tests/unit/completion/test_models.py b/tests/unit/completion/test_models.py
index 6772e1201..3f85c0eff 100644
--- a/tests/unit/completion/test_models.py
+++ b/tests/unit/completion/test_models.py
@@ -193,7 +193,7 @@ def test_help_completion(qtmodeltester, monkeypatch, stubs):
     """Test the results of command completion.
 
     Validates that:
-        - only non-hidden and non-deprecated commands are included
+        - only non-deprecated commands are included
         - commands are sorted by name
         - the command description is shown in the desc column
         - the binding (if any) is shown in the misc column
@@ -211,8 +211,9 @@ def test_help_completion(qtmodeltester, monkeypatch, stubs):
     assert actual == [
         ("Commands", [
             (':drop', 'drop all user data', ''),
+            (':hide', '', ''),
             (':roll', 'never gonna give you up', ''),
-            (':stop', 'stop qutebrowser', '')
+            (':stop', 'stop qutebrowser', ''),
         ]),
         ("Settings", [
             ('general->time', 'Is an illusion.', ''),

From 0ea61d5f15b3e78fff74aded7ebafa4e2951b555 Mon Sep 17 00:00:00 2001
From: Ryan Roden-Corrent <ryan@rcorre.net>
Date: Mon, 1 Aug 2016 12:30:12 -0400
Subject: [PATCH 2/7] Show hidden commands in completion for :bind.

There is a fair amount of duplicate code around command completion that
could probably be refactored.

Resolves #1707.
---
 qutebrowser/completion/models/instances.py  | 10 +++++++
 qutebrowser/completion/models/miscmodels.py | 32 ++++++++++++++++++++-
 qutebrowser/config/parsers/keyconf.py       |  2 +-
 qutebrowser/utils/usertypes.py              |  3 +-
 tests/unit/completion/test_models.py        | 31 ++++++++++++++++++++
 5 files changed, 75 insertions(+), 3 deletions(-)

diff --git a/qutebrowser/completion/models/instances.py b/qutebrowser/completion/models/instances.py
index d3b49c0ec..6359d3771 100644
--- a/qutebrowser/completion/models/instances.py
+++ b/qutebrowser/completion/models/instances.py
@@ -114,6 +114,13 @@ def init_session_completion():
     _instances[usertypes.Completion.sessions] = model
 
 
+def _init_bind_completion():
+    """Initialize the command completion model."""
+    log.completion.debug("Initializing bind completion.")
+    model = miscmodels.BindCompletionModel()
+    _instances[usertypes.Completion.bind] = model
+
+
 INITIALIZERS = {
     usertypes.Completion.command: _init_command_completion,
     usertypes.Completion.helptopic: _init_helptopic_completion,
@@ -125,6 +132,7 @@ INITIALIZERS = {
     usertypes.Completion.quickmark_by_name: init_quickmark_completions,
     usertypes.Completion.bookmark_by_url: init_bookmark_completions,
     usertypes.Completion.sessions: init_session_completion,
+    usertypes.Completion.bind: _init_bind_completion,
 }
 
 
@@ -182,5 +190,7 @@ def init():
     keyconf = objreg.get('key-config')
     keyconf.changed.connect(
         functools.partial(update, [usertypes.Completion.command]))
+    keyconf.changed.connect(
+        functools.partial(update, [usertypes.Completion.bind]))
 
     objreg.get('config').changed.connect(_update_aliases)
diff --git a/qutebrowser/completion/models/miscmodels.py b/qutebrowser/completion/models/miscmodels.py
index 56d436079..68ba40839 100644
--- a/qutebrowser/completion/models/miscmodels.py
+++ b/qutebrowser/completion/models/miscmodels.py
@@ -30,7 +30,7 @@ from qutebrowser.completion.models import base
 
 class CommandCompletionModel(base.BaseCompletionModel):
 
-    """A CompletionModel filled with all commands and descriptions."""
+    """A CompletionModel filled with non-hidden commands and descriptions."""
 
     # https://github.com/The-Compiler/qutebrowser/issues/545
     # pylint: disable=abstract-method
@@ -259,3 +259,33 @@ class TabCompletionModel(base.BaseCompletionModel):
         tabbed_browser = objreg.get('tabbed-browser', scope='window',
                                     window=int(win_id))
         tabbed_browser.on_tab_close_requested(int(tab_index) - 1)
+
+
+class BindCompletionModel(base.BaseCompletionModel):
+
+    """A CompletionModel filled with all bindable commands and descriptions."""
+
+    # https://github.com/The-Compiler/qutebrowser/issues/545
+    # pylint: disable=abstract-method
+
+    COLUMN_WIDTHS = (20, 60, 20)
+
+    def __init__(self, parent=None):
+        super().__init__(parent)
+        assert cmdutils.cmd_dict
+        cmdlist = []
+        for obj in set(cmdutils.cmd_dict.values()):
+            if ((obj.debug and not objreg.get('args').debug) or
+                obj.deprecated):
+                pass
+            else:
+                cmdlist.append((obj.name, obj.desc))
+        for name, cmd in config.section('aliases').items():
+            cmdlist.append((name, "Alias for '{}'".format(cmd)))
+        cat = self.new_category("Commands")
+
+        # map each command to its bound keys and show these in the misc column
+        key_config = objreg.get('key-config')
+        cmd_to_keys = key_config.get_reverse_bindings_for('normal')
+        for (name, desc) in sorted(cmdlist):
+            self.new_item(cat, name, desc, ', '.join(cmd_to_keys[name]))
diff --git a/qutebrowser/config/parsers/keyconf.py b/qutebrowser/config/parsers/keyconf.py
index 2eebd6bd9..0a88a45bb 100644
--- a/qutebrowser/config/parsers/keyconf.py
+++ b/qutebrowser/config/parsers/keyconf.py
@@ -153,7 +153,7 @@ class KeyConfigParser(QObject):
     @cmdutils.register(instance='key-config', maxsplit=1, no_cmd_split=True,
                        no_replace_variables=True)
     @cmdutils.argument('win_id', win_id=True)
-    @cmdutils.argument('command', completion=usertypes.Completion.command)
+    @cmdutils.argument('command', completion=usertypes.Completion.bind)
     def bind(self, key, win_id, command=None, *, mode='normal', force=False):
         """Bind a key to a command.
 
diff --git a/qutebrowser/utils/usertypes.py b/qutebrowser/utils/usertypes.py
index 1cd1f0385..ea7662e41 100644
--- a/qutebrowser/utils/usertypes.py
+++ b/qutebrowser/utils/usertypes.py
@@ -238,7 +238,8 @@ KeyMode = enum('KeyMode', ['normal', 'hint', 'command', 'yesno', 'prompt',
 # Available command completions
 Completion = enum('Completion', ['command', 'section', 'option', 'value',
                                  'helptopic', 'quickmark_by_name',
-                                 'bookmark_by_url', 'url', 'tab', 'sessions'])
+                                 'bookmark_by_url', 'url', 'tab', 'sessions',
+                                 'bind'])
 
 
 # Exit statuses for errors. Needs to be an int for sys.exit.
diff --git a/tests/unit/completion/test_models.py b/tests/unit/completion/test_models.py
index 3f85c0eff..c4fc82830 100644
--- a/tests/unit/completion/test_models.py
+++ b/tests/unit/completion/test_models.py
@@ -448,3 +448,34 @@ def test_setting_value_completion(qtmodeltester, monkeypatch, stubs,
             ('11', '', ''),
         ])
     ]
+
+
+def test_bind_completion(qtmodeltester, monkeypatch, stubs, config_stub,
+                         key_config_stub):
+    """Test the results of command completion.
+
+    Validates that:
+        - only non-hidden and non-deprecated commands are included
+        - commands are sorted by name
+        - the command description is shown in the desc column
+        - the binding (if any) is shown in the misc column
+        - aliases are included
+    """
+    _patch_cmdutils(monkeypatch, stubs,
+                    'qutebrowser.completion.models.miscmodels.cmdutils')
+    config_stub.data['aliases'] = {'rock': 'roll'}
+    key_config_stub.set_bindings_for('normal', {'s': 'stop', 'rr': 'roll'})
+    model = miscmodels.BindCompletionModel()
+    qtmodeltester.data_display_may_return_none = True
+    qtmodeltester.check(model)
+
+    actual = _get_completions(model)
+    assert actual == [
+        ("Commands", [
+            ('drop', 'drop all user data', ''),
+            ('hide', '', ''),
+            ('rock', "Alias for 'roll'", ''),
+            ('roll', 'never gonna give you up', 'rr'),
+            ('stop', 'stop qutebrowser', 's')
+        ])
+    ]

From 8dbc368d085e6979bec8cc105e249540fada01dc Mon Sep 17 00:00:00 2001
From: Ryan Roden-Corrent <ryan@rcorre.net>
Date: Mon, 1 Aug 2016 17:06:49 -0400
Subject: [PATCH 3/7] Refactor duplicate code for command completion.

Consolidate the logic used to generate the command completion category
into one place. This is shared by CommandCompletionModel,
HelpCompletionModel, and BindCompletionModel.
---
 qutebrowser/completion/models/miscmodels.py | 79 ++++++++++-----------
 tests/unit/completion/test_models.py        |  7 +-
 2 files changed, 42 insertions(+), 44 deletions(-)

diff --git a/qutebrowser/completion/models/miscmodels.py b/qutebrowser/completion/models/miscmodels.py
index 68ba40839..591d58b3c 100644
--- a/qutebrowser/completion/models/miscmodels.py
+++ b/qutebrowser/completion/models/miscmodels.py
@@ -39,23 +39,11 @@ class CommandCompletionModel(base.BaseCompletionModel):
 
     def __init__(self, parent=None):
         super().__init__(parent)
-        assert cmdutils.cmd_dict
-        cmdlist = []
-        for obj in set(cmdutils.cmd_dict.values()):
-            if (obj.hide or (obj.debug and not objreg.get('args').debug) or
-                    obj.deprecated):
-                pass
-            else:
-                cmdlist.append((obj.name, obj.desc))
-        for name, cmd in config.section('aliases').items():
-            cmdlist.append((name, "Alias for '{}'".format(cmd)))
+        cmdlist = _get_cmd_completions(include_aliases=True,
+                                       include_hidden=False)
         cat = self.new_category("Commands")
-
-        # map each command to its bound keys and show these in the misc column
-        key_config = objreg.get('key-config')
-        cmd_to_keys = key_config.get_reverse_bindings_for('normal')
-        for (name, desc) in sorted(cmdlist):
-            self.new_item(cat, name, desc, ', '.join(cmd_to_keys[name]))
+        for (name, desc, misc) in cmdlist:
+            self.new_item(cat, name, desc, misc, )
 
 
 class HelpCompletionModel(base.BaseCompletionModel):
@@ -72,17 +60,11 @@ class HelpCompletionModel(base.BaseCompletionModel):
 
     def _init_commands(self):
         """Fill completion with :command entries."""
-        assert cmdutils.cmd_dict
-        cmdlist = []
-        for obj in set(cmdutils.cmd_dict.values()):
-            if ((obj.debug and not objreg.get('args').debug) or
-                obj.deprecated):
-                pass
-            else:
-                cmdlist.append((':' + obj.name, obj.desc))
+        cmdlist = _get_cmd_completions(include_aliases=False,
+                                       include_hidden=True, prefix=':')
         cat = self.new_category("Commands")
-        for (name, desc) in sorted(cmdlist):
-            self.new_item(cat, name, desc)
+        for (name, desc, misc) in cmdlist:
+            self.new_item(cat, name, desc, misc)
 
     def _init_settings(self):
         """Fill completion with section->option entries."""
@@ -272,20 +254,35 @@ class BindCompletionModel(base.BaseCompletionModel):
 
     def __init__(self, parent=None):
         super().__init__(parent)
-        assert cmdutils.cmd_dict
-        cmdlist = []
-        for obj in set(cmdutils.cmd_dict.values()):
-            if ((obj.debug and not objreg.get('args').debug) or
-                obj.deprecated):
-                pass
-            else:
-                cmdlist.append((obj.name, obj.desc))
-        for name, cmd in config.section('aliases').items():
-            cmdlist.append((name, "Alias for '{}'".format(cmd)))
+        cmdlist = _get_cmd_completions(include_hidden=True,
+                                       include_aliases=True)
         cat = self.new_category("Commands")
+        for (name, desc, misc) in cmdlist:
+            self.new_item(cat, name, desc, misc)
 
-        # map each command to its bound keys and show these in the misc column
-        key_config = objreg.get('key-config')
-        cmd_to_keys = key_config.get_reverse_bindings_for('normal')
-        for (name, desc) in sorted(cmdlist):
-            self.new_item(cat, name, desc, ', '.join(cmd_to_keys[name]))
+
+def _get_cmd_completions(include_hidden, include_aliases, prefix=''):
+    """Get a list of completions info for commands, sorted by name.
+
+    Args:
+        include_hidden: True to include commands annotated with hide=True.
+        include_aliases: True to include command aliases.
+        prefix: String to append to the command name.
+
+    Return: A list of tuples of form (name, description, bindings).
+    """
+    assert cmdutils.cmd_dict
+    cmdlist = []
+    cmd_to_keys = objreg.get('key-config').get_reverse_bindings_for('normal')
+    for obj in set(cmdutils.cmd_dict.values()):
+        if ((not obj.debug or objreg.get('args').debug) and
+             not obj.deprecated and
+             (include_hidden or not obj.hide)):
+            bindings = ', '.join(cmd_to_keys[obj.name])
+            cmdlist.append((prefix + obj.name, obj.desc, bindings))
+
+    if include_aliases:
+        for name, cmd in config.section('aliases').items():
+            cmdlist.append((name, "Alias for '{}'".format(cmd), ''))
+
+    return sorted(cmdlist)
diff --git a/tests/unit/completion/test_models.py b/tests/unit/completion/test_models.py
index c4fc82830..0c6dc5c33 100644
--- a/tests/unit/completion/test_models.py
+++ b/tests/unit/completion/test_models.py
@@ -189,7 +189,7 @@ def test_command_completion(qtmodeltester, monkeypatch, stubs, config_stub,
     ]
 
 
-def test_help_completion(qtmodeltester, monkeypatch, stubs):
+def test_help_completion(qtmodeltester, monkeypatch, stubs, key_config_stub):
     """Test the results of command completion.
 
     Validates that:
@@ -201,6 +201,7 @@ def test_help_completion(qtmodeltester, monkeypatch, stubs):
         - only the first line of a multiline description is shown
     """
     module = 'qutebrowser.completion.models.miscmodels'
+    key_config_stub.set_bindings_for('normal', {'s': 'stop', 'rr': 'roll'})
     _patch_cmdutils(monkeypatch, stubs, module + '.cmdutils')
     _patch_configdata(monkeypatch, stubs, module + '.configdata.DATA')
     model = miscmodels.HelpCompletionModel()
@@ -212,8 +213,8 @@ def test_help_completion(qtmodeltester, monkeypatch, stubs):
         ("Commands", [
             (':drop', 'drop all user data', ''),
             (':hide', '', ''),
-            (':roll', 'never gonna give you up', ''),
-            (':stop', 'stop qutebrowser', ''),
+            (':roll', 'never gonna give you up', 'rr'),
+            (':stop', 'stop qutebrowser', 's'),
         ]),
         ("Settings", [
             ('general->time', 'Is an illusion.', ''),

From 709470fbe678b1dcd3ce065bbd4d8c054c7daf53 Mon Sep 17 00:00:00 2001
From: Ryan Roden-Corrent <ryan@rcorre.net>
Date: Thu, 4 Aug 2016 06:30:24 -0400
Subject: [PATCH 4/7] Clean up miscmodels and test_models.

- Split up a long conditional into multiple variables.
- Fix a small typo in a related test.
- Remove a trailing comma from an argument list.
---
 qutebrowser/completion/models/miscmodels.py | 8 ++++----
 tests/unit/completion/test_models.py        | 2 +-
 2 files changed, 5 insertions(+), 5 deletions(-)

diff --git a/qutebrowser/completion/models/miscmodels.py b/qutebrowser/completion/models/miscmodels.py
index 591d58b3c..5f5a9a438 100644
--- a/qutebrowser/completion/models/miscmodels.py
+++ b/qutebrowser/completion/models/miscmodels.py
@@ -43,7 +43,7 @@ class CommandCompletionModel(base.BaseCompletionModel):
                                        include_hidden=False)
         cat = self.new_category("Commands")
         for (name, desc, misc) in cmdlist:
-            self.new_item(cat, name, desc, misc, )
+            self.new_item(cat, name, desc, misc)
 
 
 class HelpCompletionModel(base.BaseCompletionModel):
@@ -275,9 +275,9 @@ def _get_cmd_completions(include_hidden, include_aliases, prefix=''):
     cmdlist = []
     cmd_to_keys = objreg.get('key-config').get_reverse_bindings_for('normal')
     for obj in set(cmdutils.cmd_dict.values()):
-        if ((not obj.debug or objreg.get('args').debug) and
-             not obj.deprecated and
-             (include_hidden or not obj.hide)):
+        hide_debug = obj.debug and not objreg.get('args').debug
+        hide_hidden = obj.hide and not include_hidden
+        if not (hide_debug or hide_hidden or obj.deprecated):
             bindings = ', '.join(cmd_to_keys[obj.name])
             cmdlist.append((prefix + obj.name, obj.desc, bindings))
 
diff --git a/tests/unit/completion/test_models.py b/tests/unit/completion/test_models.py
index 0c6dc5c33..519bef60a 100644
--- a/tests/unit/completion/test_models.py
+++ b/tests/unit/completion/test_models.py
@@ -453,7 +453,7 @@ def test_setting_value_completion(qtmodeltester, monkeypatch, stubs,
 
 def test_bind_completion(qtmodeltester, monkeypatch, stubs, config_stub,
                          key_config_stub):
-    """Test the results of command completion.
+    """Test the results of keybinding command completion.
 
     Validates that:
         - only non-hidden and non-deprecated commands are included

From 051a00804b3ca10db14e84b305a85e69343cf976 Mon Sep 17 00:00:00 2001
From: Ryan Roden-Corrent <ryan@rcorre.net>
Date: Thu, 4 Aug 2016 09:17:30 -0400
Subject: [PATCH 5/7] Use regular dict in get_reverse_bindings_for.

Returning a defaultdict made the caller's code look confusing, as it
wasn't clear why there wouldn't be a Keyerror in some cases. Instead,
let the caller explicitly use `get`.
---
 qutebrowser/completion/models/miscmodels.py | 2 +-
 qutebrowser/config/parsers/keyconf.py       | 3 ++-
 2 files changed, 3 insertions(+), 2 deletions(-)

diff --git a/qutebrowser/completion/models/miscmodels.py b/qutebrowser/completion/models/miscmodels.py
index 5f5a9a438..cdf201a79 100644
--- a/qutebrowser/completion/models/miscmodels.py
+++ b/qutebrowser/completion/models/miscmodels.py
@@ -278,7 +278,7 @@ def _get_cmd_completions(include_hidden, include_aliases, prefix=''):
         hide_debug = obj.debug and not objreg.get('args').debug
         hide_hidden = obj.hide and not include_hidden
         if not (hide_debug or hide_hidden or obj.deprecated):
-            bindings = ', '.join(cmd_to_keys[obj.name])
+            bindings = ', '.join(cmd_to_keys.get(obj.name, []))
             cmdlist.append((prefix + obj.name, obj.desc, bindings))
 
     if include_aliases:
diff --git a/qutebrowser/config/parsers/keyconf.py b/qutebrowser/config/parsers/keyconf.py
index 0a88a45bb..c994913db 100644
--- a/qutebrowser/config/parsers/keyconf.py
+++ b/qutebrowser/config/parsers/keyconf.py
@@ -424,8 +424,9 @@ class KeyConfigParser(QObject):
 
     def get_reverse_bindings_for(self, section):
         """Get a dict of commands to a list of bindings for the section."""
-        cmd_to_keys = collections.defaultdict(list)
+        cmd_to_keys = {}
         for key, cmd in self.get_bindings_for(section).items():
+            cmd_to_keys.setdefault(cmd, [])
             # put special bindings last
             if utils.is_special_key(key):
                 cmd_to_keys[cmd].append(key)

From 97a971624bc0ac02b792f912779224caa9b03974 Mon Sep 17 00:00:00 2001
From: Ryan Roden-Corrent <ryan@rcorre.net>
Date: Tue, 9 Aug 2016 07:24:17 -0400
Subject: [PATCH 6/7] Show bindings for aliases in completion.

Command completions for `:bind` and `:` will now show bindings for
aliases. The binding is only included if it is bound to that alias, not
if it is bound to the command the alias points to.
---
 qutebrowser/completion/models/miscmodels.py |  3 ++-
 tests/unit/completion/test_models.py        | 12 ++++++++----
 2 files changed, 10 insertions(+), 5 deletions(-)

diff --git a/qutebrowser/completion/models/miscmodels.py b/qutebrowser/completion/models/miscmodels.py
index cdf201a79..2ff98a195 100644
--- a/qutebrowser/completion/models/miscmodels.py
+++ b/qutebrowser/completion/models/miscmodels.py
@@ -283,6 +283,7 @@ def _get_cmd_completions(include_hidden, include_aliases, prefix=''):
 
     if include_aliases:
         for name, cmd in config.section('aliases').items():
-            cmdlist.append((name, "Alias for '{}'".format(cmd), ''))
+            bindings = ', '.join(cmd_to_keys.get(name, []))
+            cmdlist.append((name, "Alias for '{}'".format(cmd), bindings))
 
     return sorted(cmdlist)
diff --git a/tests/unit/completion/test_models.py b/tests/unit/completion/test_models.py
index 519bef60a..deb8c7179 100644
--- a/tests/unit/completion/test_models.py
+++ b/tests/unit/completion/test_models.py
@@ -173,7 +173,9 @@ def test_command_completion(qtmodeltester, monkeypatch, stubs, config_stub,
     _patch_cmdutils(monkeypatch, stubs,
                     'qutebrowser.completion.models.miscmodels.cmdutils')
     config_stub.data['aliases'] = {'rock': 'roll'}
-    key_config_stub.set_bindings_for('normal', {'s': 'stop', 'rr': 'roll'})
+    key_config_stub.set_bindings_for('normal', {'s': 'stop',
+                                                'rr': 'roll',
+                                                'ro': 'rock'})
     model = miscmodels.CommandCompletionModel()
     qtmodeltester.data_display_may_return_none = True
     qtmodeltester.check(model)
@@ -182,7 +184,7 @@ def test_command_completion(qtmodeltester, monkeypatch, stubs, config_stub,
     assert actual == [
         ("Commands", [
             ('drop', 'drop all user data', ''),
-            ('rock', "Alias for 'roll'", ''),
+            ('rock', "Alias for 'roll'", 'ro'),
             ('roll', 'never gonna give you up', 'rr'),
             ('stop', 'stop qutebrowser', 's')
         ])
@@ -465,7 +467,9 @@ def test_bind_completion(qtmodeltester, monkeypatch, stubs, config_stub,
     _patch_cmdutils(monkeypatch, stubs,
                     'qutebrowser.completion.models.miscmodels.cmdutils')
     config_stub.data['aliases'] = {'rock': 'roll'}
-    key_config_stub.set_bindings_for('normal', {'s': 'stop', 'rr': 'roll'})
+    key_config_stub.set_bindings_for('normal', {'s': 'stop',
+                                                'rr': 'roll',
+                                                'ro': 'rock'})
     model = miscmodels.BindCompletionModel()
     qtmodeltester.data_display_may_return_none = True
     qtmodeltester.check(model)
@@ -475,7 +479,7 @@ def test_bind_completion(qtmodeltester, monkeypatch, stubs, config_stub,
         ("Commands", [
             ('drop', 'drop all user data', ''),
             ('hide', '', ''),
-            ('rock', "Alias for 'roll'", ''),
+            ('rock', "Alias for 'roll'", 'ro'),
             ('roll', 'never gonna give you up', 'rr'),
             ('stop', 'stop qutebrowser', 's')
         ])

From 0c43ca9f094a77214c6e7f2effd456315a94e8ca Mon Sep 17 00:00:00 2001
From: Ryan Roden-Corrent <ryan@rcorre.net>
Date: Tue, 9 Aug 2016 22:06:03 -0400
Subject: [PATCH 7/7] Don't sort command completions in models.

The sortfilter already handles sorting, there is no reason to pre-sort
the entries in the model.
---
 qutebrowser/completion/models/miscmodels.py |   2 +-
 tests/unit/completion/test_models.py        | 160 +++++++++-----------
 2 files changed, 76 insertions(+), 86 deletions(-)

diff --git a/qutebrowser/completion/models/miscmodels.py b/qutebrowser/completion/models/miscmodels.py
index 2ff98a195..aeb9478f0 100644
--- a/qutebrowser/completion/models/miscmodels.py
+++ b/qutebrowser/completion/models/miscmodels.py
@@ -286,4 +286,4 @@ def _get_cmd_completions(include_hidden, include_aliases, prefix=''):
             bindings = ', '.join(cmd_to_keys.get(name, []))
             cmdlist.append((name, "Alias for '{}'".format(cmd), bindings))
 
-    return sorted(cmdlist)
+    return cmdlist
diff --git a/tests/unit/completion/test_models.py b/tests/unit/completion/test_models.py
index deb8c7179..1f9e2bc2d 100644
--- a/tests/unit/completion/test_models.py
+++ b/tests/unit/completion/test_models.py
@@ -31,17 +31,18 @@ from qutebrowser.browser.webkit import history
 from qutebrowser.config import sections, value
 
 
-def _get_completions(model):
-    """Collect all the completion entries of a model, organized by category.
+def _check_completions(model, expected):
+    """Check that a model contains the expected items in any order.
 
-    The result is a list of form:
-    [
-        (CategoryName: [(name, desc, misc), ...]),
-        (CategoryName: [(name, desc, misc), ...]),
-        ...
-    ]
+    Args:
+        expected: A dict of form
+            {
+                CategoryName: [(name, desc, misc), ...],
+                CategoryName: [(name, desc, misc), ...],
+                ...
+            }
     """
-    completions = []
+    actual = {}
     for i in range(0, model.rowCount()):
         category = model.item(i)
         entries = []
@@ -50,8 +51,12 @@ def _get_completions(model):
             desc = category.child(j, 1)
             misc = category.child(j, 2)
             entries.append((name.text(), desc.text(), misc.text()))
-        completions.append((category.text(), entries))
-    return completions
+        actual[category.text()] = entries
+    for cat_name, expected_entries in expected.items():
+        assert cat_name in actual
+        actual_items = actual[cat_name]
+        for expected_item in expected_entries:
+            assert expected_item in actual_items
 
 
 def _patch_cmdutils(monkeypatch, stubs, symbol):
@@ -165,7 +170,6 @@ def test_command_completion(qtmodeltester, monkeypatch, stubs, config_stub,
 
     Validates that:
         - only non-hidden and non-deprecated commands are included
-        - commands are sorted by name
         - the command description is shown in the desc column
         - the binding (if any) is shown in the misc column
         - aliases are included
@@ -180,15 +184,14 @@ def test_command_completion(qtmodeltester, monkeypatch, stubs, config_stub,
     qtmodeltester.data_display_may_return_none = True
     qtmodeltester.check(model)
 
-    actual = _get_completions(model)
-    assert actual == [
-        ("Commands", [
+    _check_completions(model, {
+        "Commands": [
+            ('stop', 'stop qutebrowser', 's'),
             ('drop', 'drop all user data', ''),
-            ('rock', "Alias for 'roll'", 'ro'),
             ('roll', 'never gonna give you up', 'rr'),
-            ('stop', 'stop qutebrowser', 's')
-        ])
-    ]
+            ('rock', "Alias for 'roll'", 'ro'),
+        ]
+    })
 
 
 def test_help_completion(qtmodeltester, monkeypatch, stubs, key_config_stub):
@@ -196,7 +199,6 @@ def test_help_completion(qtmodeltester, monkeypatch, stubs, key_config_stub):
 
     Validates that:
         - only non-deprecated commands are included
-        - commands are sorted by name
         - the command description is shown in the desc column
         - the binding (if any) is shown in the misc column
         - aliases are included
@@ -210,22 +212,21 @@ def test_help_completion(qtmodeltester, monkeypatch, stubs, key_config_stub):
     qtmodeltester.data_display_may_return_none = True
     qtmodeltester.check(model)
 
-    actual = _get_completions(model)
-    assert actual == [
-        ("Commands", [
-            (':drop', 'drop all user data', ''),
-            (':hide', '', ''),
-            (':roll', 'never gonna give you up', 'rr'),
+    _check_completions(model, {
+        "Commands": [
             (':stop', 'stop qutebrowser', 's'),
-        ]),
-        ("Settings", [
+            (':drop', 'drop all user data', ''),
+            (':roll', 'never gonna give you up', 'rr'),
+            (':hide', '', ''),
+        ],
+        "Settings": [
             ('general->time', 'Is an illusion.', ''),
             ('general->volume', 'Goes to 11', ''),
             ('ui->gesture', 'Waggle your hands to control qutebrowser', ''),
             ('ui->mind', 'Enable mind-control ui (experimental)', ''),
             ('ui->voice', 'Whether to respond to voice commands', ''),
-        ])
-    ]
+        ]
+    })
 
 
 def test_quickmark_completion(qtmodeltester, quickmarks):
@@ -234,14 +235,13 @@ def test_quickmark_completion(qtmodeltester, quickmarks):
     qtmodeltester.data_display_may_return_none = True
     qtmodeltester.check(model)
 
-    actual = _get_completions(model)
-    assert actual == [
-        ("Quickmarks", [
+    _check_completions(model, {
+        "Quickmarks": [
             ('aw', 'https://wiki.archlinux.org', ''),
             ('ddg', 'https://duckduckgo.com', ''),
             ('wiki', 'https://wikipedia.org', ''),
-        ])
-    ]
+        ]
+    })
 
 
 def test_bookmark_completion(qtmodeltester, bookmarks):
@@ -250,14 +250,13 @@ def test_bookmark_completion(qtmodeltester, bookmarks):
     qtmodeltester.data_display_may_return_none = True
     qtmodeltester.check(model)
 
-    actual = _get_completions(model)
-    assert actual == [
-        ("Bookmarks", [
+    _check_completions(model, {
+        "Bookmarks": [
             ('https://github.com', 'GitHub', ''),
             ('https://python.org', 'Welcome to Python.org', ''),
             ('http://qutebrowser.org', 'qutebrowser | qutebrowser', ''),
-        ])
-    ]
+        ]
+    })
 
 
 def test_url_completion(qtmodeltester, config_stub, web_history, quickmarks,
@@ -275,23 +274,22 @@ def test_url_completion(qtmodeltester, config_stub, web_history, quickmarks,
     qtmodeltester.data_display_may_return_none = True
     qtmodeltester.check(model)
 
-    actual = _get_completions(model)
-    assert actual == [
-        ("Quickmarks", [
+    _check_completions(model, {
+        "Quickmarks": [
             ('https://wiki.archlinux.org', 'aw', ''),
             ('https://duckduckgo.com', 'ddg', ''),
             ('https://wikipedia.org', 'wiki', ''),
-        ]),
-        ("Bookmarks", [
+        ],
+        "Bookmarks": [
             ('https://github.com', 'GitHub', ''),
             ('https://python.org', 'Welcome to Python.org', ''),
             ('http://qutebrowser.org', 'qutebrowser | qutebrowser', ''),
-        ]),
-        ("History", [
+        ],
+        "History": [
             ('https://python.org', 'Welcome to Python.org', '2016-03-08'),
             ('https://github.com', 'GitHub', '2016-05-01'),
-        ]),
-    ]
+        ],
+    })
 
 
 def test_url_completion_delete_bookmark(qtmodeltester, config_stub,
@@ -336,10 +334,9 @@ def test_session_completion(qtmodeltester, session_manager_stub):
     qtmodeltester.data_display_may_return_none = True
     qtmodeltester.check(model)
 
-    actual = _get_completions(model)
-    assert actual == [
-        ("Sessions", [('default', '', ''), ('1', '', ''), ('2', '', '')])
-    ]
+    _check_completions(model, {
+        "Sessions": [('default', '', ''), ('1', '', ''), ('2', '', '')]
+    })
 
 
 def test_tab_completion(qtmodeltester, fake_web_tab, app_stub, win_registry,
@@ -356,17 +353,16 @@ def test_tab_completion(qtmodeltester, fake_web_tab, app_stub, win_registry,
     qtmodeltester.data_display_may_return_none = True
     qtmodeltester.check(model)
 
-    actual = _get_completions(model)
-    assert actual == [
-        ('0', [
+    _check_completions(model, {
+        '0': [
             ('0/1', 'https://github.com', 'GitHub'),
             ('0/2', 'https://wikipedia.org', 'Wikipedia'),
             ('0/3', 'https://duckduckgo.com', 'DuckDuckGo')
-        ]),
-        ('1', [
+        ],
+        '1': [
             ('1/1', 'https://wiki.archlinux.org', 'ArchWiki'),
-        ])
-    ]
+        ]
+    })
 
 
 def test_tab_completion_delete(qtmodeltester, fake_web_tab, qtbot, app_stub,
@@ -401,13 +397,12 @@ def test_setting_section_completion(qtmodeltester, monkeypatch, stubs):
     qtmodeltester.data_display_may_return_none = True
     qtmodeltester.check(model)
 
-    actual = _get_completions(model)
-    assert actual == [
-        ("Sections", [
+    _check_completions(model, {
+        "Sections": [
             ('general', 'General/miscellaneous options.', ''),
             ('ui', 'General options related to the user interface.', ''),
-        ])
-    ]
+        ]
+    })
 
 
 def test_setting_option_completion(qtmodeltester, monkeypatch, stubs,
@@ -421,14 +416,13 @@ def test_setting_option_completion(qtmodeltester, monkeypatch, stubs,
     qtmodeltester.data_display_may_return_none = True
     qtmodeltester.check(model)
 
-    actual = _get_completions(model)
-    assert actual == [
-        ("ui", [
+    _check_completions(model, {
+        "ui": [
             ('gesture', 'Waggle your hands to control qutebrowser', 'off'),
             ('mind', 'Enable mind-control ui (experimental)', 'on'),
             ('voice', 'Whether to respond to voice commands', 'sometimes'),
-        ])
-    ]
+        ]
+    })
 
 
 def test_setting_value_completion(qtmodeltester, monkeypatch, stubs,
@@ -440,17 +434,16 @@ def test_setting_value_completion(qtmodeltester, monkeypatch, stubs,
     qtmodeltester.data_display_may_return_none = True
     qtmodeltester.check(model)
 
-    actual = _get_completions(model)
-    assert actual == [
-        ("Current/Default", [
+    _check_completions(model, {
+        "Current/Default": [
             ('0', 'Current value', ''),
             ('11', 'Default value', ''),
-        ]),
-        ("Completions", [
+        ],
+        "Completions": [
             ('0', '', ''),
             ('11', '', ''),
-        ])
-    ]
+        ]
+    })
 
 
 def test_bind_completion(qtmodeltester, monkeypatch, stubs, config_stub,
@@ -459,7 +452,6 @@ def test_bind_completion(qtmodeltester, monkeypatch, stubs, config_stub,
 
     Validates that:
         - only non-hidden and non-deprecated commands are included
-        - commands are sorted by name
         - the command description is shown in the desc column
         - the binding (if any) is shown in the misc column
         - aliases are included
@@ -474,13 +466,11 @@ def test_bind_completion(qtmodeltester, monkeypatch, stubs, config_stub,
     qtmodeltester.data_display_may_return_none = True
     qtmodeltester.check(model)
 
-    actual = _get_completions(model)
-    assert actual == [
-        ("Commands", [
+    _check_completions(model, {
+        "Commands": [
+            ('stop', 'stop qutebrowser', 's'),
             ('drop', 'drop all user data', ''),
             ('hide', '', ''),
             ('rock', "Alias for 'roll'", 'ro'),
-            ('roll', 'never gonna give you up', 'rr'),
-            ('stop', 'stop qutebrowser', 's')
-        ])
-    ]
+        ]
+    })