From aedf5b930a7c833e374c92e31c944f2dece23448 Mon Sep 17 00:00:00 2001
From: Ryan Roden-Corrent <ryan@rcorre.net>
Date: Mon, 11 Jul 2016 20:54:16 -0400
Subject: [PATCH 01/11] Test Completer.schedule_completion_update.

update_completion is only used internally, so instead test the real
public entry point which is schedule_completion_update.

This required mocking out QTimer to fire immediately so the test didn't
have to do flaky artificial delays.
---
 tests/helpers/stubs.py                  | 22 ++++++++++++++++++++++
 tests/unit/completion/test_completer.py |  6 ++++--
 2 files changed, 26 insertions(+), 2 deletions(-)

diff --git a/tests/helpers/stubs.py b/tests/helpers/stubs.py
index 419d6bd78..7808b5816 100644
--- a/tests/helpers/stubs.py
+++ b/tests/helpers/stubs.py
@@ -381,6 +381,28 @@ class FakeTimer(QObject):
         return self._started
 
 
+class InstaTimer(QObject):
+
+    """Stub for a QTimer that fires instantly on start().
+
+    Useful to test a time-based event without inserting an artificial delay.
+    """
+
+    timeout = pyqtSignal()
+
+    def __init__(self, parent=None):
+        super().__init__(parent)
+
+    def start(self):
+        self.timeout.emit()
+
+    def setSingleShot(self, yes):
+        pass
+
+    def setInterval(self, interval):
+        pass
+
+
 class FakeConfigType:
 
     """A stub to provide valid_values for typ attribute of a SettingValue."""
diff --git a/tests/unit/completion/test_completer.py b/tests/unit/completion/test_completer.py
index bfb1f0718..34537330c 100644
--- a/tests/unit/completion/test_completer.py
+++ b/tests/unit/completion/test_completer.py
@@ -48,8 +48,10 @@ def cmd(stubs, qtbot):
 
 
 @pytest.fixture
-def completer_obj(qtbot, cmd, config_stub):
+def completer_obj(qtbot, cmd, config_stub, monkeypatch, stubs):
     """Create the completer used for testing."""
+    monkeypatch.setattr('qutebrowser.completion.completer.QTimer',
+        stubs.InstaTimer)
     config_stub.data = {'completion': {'auto-open': False}}
     return completer.Completer(cmd, 0)
 
@@ -162,7 +164,7 @@ def test_update_completion(txt, expected, cmd, completer_obj,
     """Test setting the completion widget's model based on command text."""
     # this test uses | as a placeholder for the current cursor position
     _set_cmd_prompt(cmd, txt)
-    completer_obj.update_completion()
+    completer_obj.schedule_completion_update()
     if expected is None:
         assert not completion_widget_stub.set_model.called
     else:

From e5da179ebf6f77165db61b97a8758babfdaee785 Mon Sep 17 00:00:00 2001
From: Ryan Roden-Corrent <ryan@rcorre.net>
Date: Mon, 11 Jul 2016 21:01:23 -0400
Subject: [PATCH 02/11] Limit the public interface of Completer.

Only schedule_completion_update and selection_changed are used
externally, so mark the rest of the API as private.
---
 qutebrowser/completion/completer.py     | 46 ++++++++++++-------------
 tests/unit/completion/test_completer.py |  3 +-
 2 files changed, 25 insertions(+), 24 deletions(-)

diff --git a/qutebrowser/completion/completer.py b/qutebrowser/completion/completer.py
index ebdb1d10b..418ee05c9 100644
--- a/qutebrowser/completion/completer.py
+++ b/qutebrowser/completion/completer.py
@@ -61,25 +61,25 @@ class Completer(QObject):
         self._timer = QTimer()
         self._timer.setSingleShot(True)
         self._timer.setInterval(0)
-        self._timer.timeout.connect(self.update_completion)
+        self._timer.timeout.connect(self._update_completion)
         self._cursor_part = None
         self._last_cursor_pos = None
         self._last_text = None
 
-        objreg.get('config').changed.connect(self.on_auto_open_changed)
-        self.handle_signal_connections()
+        objreg.get('config').changed.connect(self._on_auto_open_changed)
+        self._handle_signal_connections()
         self._cmd.clear_completion_selection.connect(
-            self.handle_signal_connections)
+            self._handle_signal_connections)
 
     def __repr__(self):
         return utils.get_repr(self)
 
     @config.change_filter('completion', 'auto-open')
-    def on_auto_open_changed(self):
-        self.handle_signal_connections()
+    def _on_auto_open_changed(self):
+        self._handle_signal_connections()
 
     @pyqtSlot()
-    def handle_signal_connections(self):
+    def _handle_signal_connections(self):
         self._connect_signals(config.get('completion', 'auto-open'))
 
     def _connect_signals(self, connect=True):
@@ -95,7 +95,7 @@ class Completer(QObject):
         """
         connections = [
             (self._cmd.update_completion, self.schedule_completion_update),
-            (self._cmd.textChanged, self.on_text_edited),
+            (self._cmd.textChanged, self._on_text_edited),
         ]
 
         if connect and not self._signals_connected:
@@ -121,7 +121,7 @@ class Completer(QObject):
         if not config.get('completion', 'auto-open'):
             connected = self._connect_signals(True)
             if connected:
-                self.update_completion()
+                self._update_completion()
 
     def _model(self):
         """Convenience method to get the current completion model."""
@@ -265,7 +265,7 @@ class Completer(QObject):
         data = model.data(indexes[0])
         if data is None:
             return
-        parts = self.split()
+        parts = self._split()
         try:
             needs_quoting = cmdutils.cmd_dict[parts[0]].maxsplit is None
         except KeyError:
@@ -275,11 +275,11 @@ class Completer(QObject):
         if model.count() == 1 and config.get('completion', 'quick-complete'):
             # If we only have one item, we want to apply it immediately
             # and go on to the next part.
-            self.change_completed_part(data, immediate=True)
+            self._change_completed_part(data, immediate=True)
         else:
             log.completion.debug("Will ignore next completion update.")
             self._ignore_change = True
-            self.change_completed_part(data)
+            self._change_completed_part(data)
 
     @pyqtSlot()
     def schedule_completion_update(self):
@@ -299,10 +299,10 @@ class Completer(QObject):
         self._last_text = self._cmd.text()
 
     @pyqtSlot()
-    def update_completion(self):
+    def _update_completion(self):
         """Check if completions are available and activate them."""
-        self.update_cursor_part()
-        parts = self.split()
+        self._update_cursor_part()
+        parts = self._split()
 
         log.completion.debug(
             "Updating completion - prefix {}, parts {}, cursor_part {}".format(
@@ -354,8 +354,8 @@ class Completer(QObject):
         if completion.enabled:
             completion.show()
 
-    def split(self, keep=False):
-        """Get the text split up in parts.
+    def _split(self, keep=False):
+        """Get the text _split up in parts.
 
         Args:
             keep: Whether to keep special chars and whitespace.
@@ -381,13 +381,13 @@ class Completer(QObject):
         return parts
 
     @pyqtSlot()
-    def update_cursor_part(self):
+    def _update_cursor_part(self):
         """Get the part index of the commandline where the cursor is over."""
         cursor_pos = self._cmd.cursorPosition()
         snippet = slice(cursor_pos - 1, cursor_pos + 1)
         spaces = self._cmd.text()[snippet] == '  '
         cursor_pos -= len(self._cmd.prefix())
-        parts = self.split(keep=True)
+        parts = self._split(keep=True)
         log.completion.vdebug(
             "text: {}, parts: {}, cursor_pos after removing prefix '{}': "
             "{}".format(self._cmd.text(), parts, self._cmd.prefix(),
@@ -429,7 +429,7 @@ class Completer(QObject):
             self._cursor_part, spaces))
         return
 
-    def change_completed_part(self, newtext, immediate=False):
+    def _change_completed_part(self, newtext, immediate=False):
         """Change the part we're currently completing in the commandline.
 
         Args:
@@ -438,7 +438,7 @@ class Completer(QObject):
                        including a trailing space and we shouldn't continue
                        completing the current item.
         """
-        parts = self.split()
+        parts = self._split()
         log.completion.debug("changing part {} to '{}'".format(
             self._cursor_part, newtext))
         try:
@@ -465,10 +465,10 @@ class Completer(QObject):
         self._cmd.show_cmd.emit()
 
     @pyqtSlot()
-    def on_text_edited(self):
+    def _on_text_edited(self):
         """Reset _empty_item_idx if text was edited."""
         self._empty_item_idx = None
-        # We also want to update the cursor part and emit update_completion
+        # We also want to update the cursor part and emit _update_completion
         # here, but that's already done for us by cursorPositionChanged
         # anyways, so we don't need to do it twice.
 
diff --git a/tests/unit/completion/test_completer.py b/tests/unit/completion/test_completer.py
index 34537330c..d87d241f3 100644
--- a/tests/unit/completion/test_completer.py
+++ b/tests/unit/completion/test_completer.py
@@ -219,7 +219,8 @@ def test_selection_changed(before, newtxt, count, quick_complete, after,
     selection.indexes = unittest.mock.Mock(return_value=indexes)
     completion_widget_stub.model = unittest.mock.Mock(return_value=model)
     _set_cmd_prompt(cmd, before)
-    completer_obj.update_cursor_part()
+    # schedule_completion_update is needed to pick up the cursor position
+    completer_obj.schedule_completion_update()
     completer_obj.selection_changed(selection, None)
     model.data.assert_called_with(indexes[0])
     _validate_cmd_prompt(cmd, after)

From a740f996077f17734a93ca8c2bac2d15aee74f3c Mon Sep 17 00:00:00 2001
From: Ryan Roden-Corrent <ryan@rcorre.net>
Date: Tue, 12 Jul 2016 12:47:28 -0400
Subject: [PATCH 03/11] Register the fake statusbar command in objreg.

The CompletionView looks in objreg for 'status-cmd', so move it from a
private fixture in test_completer to a public fixture that handles
objreg registration/deletion.
---
 tests/helpers/fixtures.py               | 10 +++++++
 tests/helpers/stubs.py                  |  2 +-
 tests/unit/completion/test_completer.py | 36 ++++++++++---------------
 3 files changed, 25 insertions(+), 23 deletions(-)

diff --git a/tests/helpers/fixtures.py b/tests/helpers/fixtures.py
index 0fd890fa0..7454b46e4 100644
--- a/tests/helpers/fixtures.py
+++ b/tests/helpers/fixtures.py
@@ -275,6 +275,16 @@ def completion_widget_stub(win_registry):
     objreg.delete('completion', scope='window', window=0)
 
 
+@pytest.yield_fixture
+def status_command_stub(stubs, qtbot, win_registry):
+    """Fixture which provides a fake status-command object."""
+    cmd = stubs.StatusBarCommandStub()
+    objreg.register('status-command', cmd, scope='window', window=0)
+    qtbot.addWidget(cmd)
+    yield cmd
+    objreg.delete('status-command', scope='window', window=0)
+
+
 @pytest.fixture(scope='session')
 def stubs():
     """Provide access to stub objects useful for testing."""
diff --git a/tests/helpers/stubs.py b/tests/helpers/stubs.py
index 7808b5816..5027f301b 100644
--- a/tests/helpers/stubs.py
+++ b/tests/helpers/stubs.py
@@ -413,7 +413,7 @@ class FakeConfigType:
         self.complete = lambda: [(val, '') for val in valid_values]
 
 
-class FakeStatusbarCommand(QLineEdit):
+class StatusBarCommandStub(QLineEdit):
 
     """Stub for the statusbar command prompt."""
 
diff --git a/tests/unit/completion/test_completer.py b/tests/unit/completion/test_completer.py
index d87d241f3..9335bbf70 100644
--- a/tests/unit/completion/test_completer.py
+++ b/tests/unit/completion/test_completer.py
@@ -40,20 +40,12 @@ class FakeCompletionModel(QStandardItemModel):
 
 
 @pytest.fixture
-def cmd(stubs, qtbot):
-    """Create the statusbar command prompt the completer uses."""
-    cmd = stubs.FakeStatusbarCommand()
-    qtbot.addWidget(cmd)
-    return cmd
-
-
-@pytest.fixture
-def completer_obj(qtbot, cmd, config_stub, monkeypatch, stubs):
+def completer_obj(qtbot, status_command_stub, config_stub, monkeypatch, stubs):
     """Create the completer used for testing."""
     monkeypatch.setattr('qutebrowser.completion.completer.QTimer',
         stubs.InstaTimer)
     config_stub.data = {'completion': {'auto-open': False}}
-    return completer.Completer(cmd, 0)
+    return completer.Completer(status_command_stub, 0)
 
 
 @pytest.fixture(autouse=True)
@@ -159,11 +151,11 @@ def _validate_cmd_prompt(cmd, txt):
     (':set -t -p |', usertypes.Completion.section),
     (':open -- |', None),
 ])
-def test_update_completion(txt, expected, cmd, completer_obj,
+def test_update_completion(txt, expected, status_command_stub, completer_obj,
                            completion_widget_stub):
     """Test setting the completion widget's model based on command text."""
     # this test uses | as a placeholder for the current cursor position
-    _set_cmd_prompt(cmd, txt)
+    _set_cmd_prompt(status_command_stub, txt)
     completer_obj.schedule_completion_update()
     if expected is None:
         assert not completion_widget_stub.set_model.called
@@ -174,19 +166,19 @@ def test_update_completion(txt, expected, cmd, completer_obj,
         assert arg.srcmodel.kind == expected
 
 
-def test_completion_item_prev(completer_obj, cmd, completion_widget_stub,
-                              config_stub, qtbot):
+def test_completion_item_prev(completer_obj, status_command_stub,
+                              completion_widget_stub, config_stub, qtbot):
     """Test that completion_item_prev emits next_prev_item."""
-    cmd.setText(':')
+    status_command_stub.setText(':')
     with qtbot.waitSignal(completer_obj.next_prev_item) as blocker:
         completer_obj.completion_item_prev()
     assert blocker.args == [True]
 
 
-def test_completion_item_next(completer_obj, cmd, completion_widget_stub,
-                              config_stub, qtbot):
+def test_completion_item_next(completer_obj, status_command_stub,
+                              completion_widget_stub, config_stub, qtbot):
     """Test that completion_item_next emits next_prev_item."""
-    cmd.setText(':')
+    status_command_stub.setText(':')
     with qtbot.waitSignal(completer_obj.next_prev_item) as blocker:
         completer_obj.completion_item_next()
     assert blocker.args == [False]
@@ -202,8 +194,8 @@ def test_completion_item_next(completer_obj, cmd, completion_widget_stub,
     (':foo |', None, True, 1, ":foo |"),
 ])
 def test_selection_changed(before, newtxt, count, quick_complete, after,
-                           completer_obj, cmd, completion_widget_stub,
-                           config_stub):
+                           completer_obj, status_command_stub,
+                           completion_widget_stub, config_stub):
     """Test that change_completed_part modifies the cmd text properly.
 
     The | represents the current cursor position in the cmd prompt.
@@ -218,9 +210,9 @@ def test_selection_changed(before, newtxt, count, quick_complete, after,
     selection = unittest.mock.Mock()
     selection.indexes = unittest.mock.Mock(return_value=indexes)
     completion_widget_stub.model = unittest.mock.Mock(return_value=model)
-    _set_cmd_prompt(cmd, before)
+    _set_cmd_prompt(status_command_stub, before)
     # schedule_completion_update is needed to pick up the cursor position
     completer_obj.schedule_completion_update()
     completer_obj.selection_changed(selection, None)
     model.data.assert_called_with(indexes[0])
-    _validate_cmd_prompt(cmd, after)
+    _validate_cmd_prompt(status_command_stub, after)

From 415ad7a638c09c5f7d07af392cf025796d3e7e84 Mon Sep 17 00:00:00 2001
From: Ryan Roden-Corrent <ryan@rcorre.net>
Date: Tue, 12 Jul 2016 20:04:56 -0400
Subject: [PATCH 04/11] Unit test completion view.

---
 .../unit/completion/test_completionwidget.py  | 152 ++++++++++++++++++
 1 file changed, 152 insertions(+)
 create mode 100644 tests/unit/completion/test_completionwidget.py

diff --git a/tests/unit/completion/test_completionwidget.py b/tests/unit/completion/test_completionwidget.py
new file mode 100644
index 000000000..2148a59f5
--- /dev/null
+++ b/tests/unit/completion/test_completionwidget.py
@@ -0,0 +1,152 @@
+# vim: ft=python fileencoding=utf-8 sts=4 sw=4 et:
+
+# Copyright 2016 Ryan Roden-Corrent (rcorre) <ryan@rcorre.net>
+#
+# This file is part of qutebrowser.
+#
+# qutebrowser is free software: you can redistribute it and/or modify
+# it under the terms of the GNU General Public License as published by
+# the Free Software Foundation, either version 3 of the License, or
+# (at your option) any later version.
+#
+# qutebrowser is distributed in the hope that it will be useful,
+# but WITHOUT ANY WARRANTY; without even the implied warranty of
+# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+# GNU General Public License for more details.
+#
+# You should have received a copy of the GNU General Public License
+# along with qutebrowser.  If not, see <http://www.gnu.org/licenses/>.
+
+"""Tests for the CompletionView Object."""
+
+import unittest.mock
+
+import pytest
+from PyQt5.QtGui import QStandardItem, QColor
+
+from qutebrowser.completion import completionwidget
+from qutebrowser.completion.models import base, sortfilter
+from qutebrowser.utils import objreg
+
+
+@pytest.yield_fixture
+def completionview(qtbot, status_command_stub, config_stub, win_registry,
+                   monkeypatch):
+    """Create the CompletionView used for testing."""
+    config_stub.data = {
+        'completion': {
+            'show': True,
+            'auto-open': True,
+            'scrollbar-width': 12,
+            'scrollbar-padding': 2,
+            'shrink': False,
+        },
+        'colors': {
+            'completion.fg': QColor(),
+            'completion.bg': QColor(),
+            'completion.alternate-bg': QColor(),
+            'completion.category.fg': QColor(),
+            'completion.category.bg': QColor(),
+            'completion.category.border.top': QColor(),
+            'completion.category.border.bottom': QColor(),
+            'completion.item.selected.fg': QColor(),
+            'completion.item.selected.bg': QColor(),
+            'completion.item.selected.border.top': QColor(),
+            'completion.item.selected.border.bottom': QColor(),
+            'completion.match.fg': QColor(),
+            'completion.scrollbar.fg': QColor(),
+            'completion.scrollbar.bg': QColor(),
+        },
+        'fonts': {
+            'completion': 'Comic Sans Monospace'
+        }
+    }
+    # mock the Completer that the widget creates in its constructor
+    monkeypatch.setattr('qutebrowser.completion.completer.Completer',
+        lambda *args: unittest.mock.Mock())
+    view = completionwidget.CompletionView(win_id=0)
+    qtbot.addWidget(view)
+    yield view
+    # the constructor registers both 'completion' (itself) and 'completer'
+    objreg.delete('completion', scope='window', window=0)
+    objreg.delete('completer', scope='window', window=0)
+
+
+def test_set_model(completionview):
+    """Ensure set_model actually sets the model and expands all categories."""
+    model = base.BaseCompletionModel()
+    filtermodel = sortfilter.CompletionFilterModel(model)
+    for i in range(3):
+        model.appendRow(QStandardItem(str(i)))
+    completionview.set_model(filtermodel)
+    assert completionview.model() == filtermodel
+    for i in range(model.rowCount()):
+        assert completionview.isExpanded(filtermodel.index(i, 0))
+
+
+def test_set_pattern(completionview):
+    model = sortfilter.CompletionFilterModel(base.BaseCompletionModel())
+    model.set_pattern = unittest.mock.Mock()
+    completionview.set_model(model)
+    completionview.set_pattern('foo')
+    model.set_pattern.assert_called_with('foo')
+
+
+def test_maybe_resize_completion(completionview, config_stub, qtbot):
+    """Ensure completion is resized only if shrink is True."""
+    with qtbot.assertNotEmitted(completionview.resize_completion):
+        completionview.maybe_resize_completion()
+    config_stub.data = {'completion': {'shrink': True}}
+    with qtbot.waitSignal(completionview.resize_completion):
+        completionview.maybe_resize_completion()
+
+
+@pytest.mark.parametrize('tree, count, expected', [
+    ([['Aa']], 1, 'Aa'),
+    ([['Aa']], -1, 'Aa'),
+    ([['Aa'], ['Ba']], 1, 'Aa'),
+    ([['Aa'], ['Ba']], -1, 'Ba'),
+    ([['Aa'], ['Ba']], 2, 'Ba'),
+    ([['Aa'], ['Ba']], -2, 'Aa'),
+    ([['Aa', 'Ab', 'Ac'], ['Ba', 'Bb'], ['Ca']], 3, 'Ac'),
+    ([['Aa', 'Ab', 'Ac'], ['Ba', 'Bb'], ['Ca']], 4, 'Ba'),
+    ([['Aa', 'Ab', 'Ac'], ['Ba', 'Bb'], ['Ca']], 6, 'Ca'),
+    ([['Aa', 'Ab', 'Ac'], ['Ba', 'Bb'], ['Ca']], 7, 'Aa'),
+    ([['Aa', 'Ab', 'Ac'], ['Ba', 'Bb'], ['Ca']], -1, 'Ca'),
+    ([['Aa', 'Ab', 'Ac'], ['Ba', 'Bb'], ['Ca']], -2, 'Bb'),
+    ([['Aa', 'Ab', 'Ac'], ['Ba', 'Bb'], ['Ca']], -4, 'Ac'),
+    ([[], ['Ba', 'Bb']], 1, 'Ba'),
+    ([[], ['Ba', 'Bb']], -1, 'Bb'),
+    ([[], [], ['Ca', 'Cb']], 1, 'Ca'),
+    ([[], [], ['Ca', 'Cb']], -1, 'Cb'),
+    ([['Aa'], []], 1, 'Aa'),
+    ([['Aa'], []], -1, 'Aa'),
+    ([['Aa'], [], []], 1, 'Aa'),
+    ([['Aa'], [], []], -1, 'Aa'),
+])
+def test_on_next_prev_item(tree, count, expected, completionview,
+                           config_stub, qtbot, monkeypatch,
+                           status_command_stub):
+    """Test that on_next_prev_item moves the selection properly.
+
+    Args:
+        tree: Each entry array represents a completion category, with each
+              string being an item under that category.
+        count: Number of times to go forward (or back if negative).
+        expected: item data that should be selected after going back/forward.
+    """
+    model = base.BaseCompletionModel()
+    for catdata in tree:
+        cat = QStandardItem()
+        model.appendRow(cat)
+        for name in catdata:
+            cat.appendRow(QStandardItem(name))
+    filtermodel = sortfilter.CompletionFilterModel(model)
+    completionview.set_model(filtermodel)
+    # actually calling show() will pop a window during the test, so just fool
+    # the completionview into thinking it is visible instead
+    monkeypatch.setattr(completionview, 'isVisible', lambda: True)
+    for i in range(abs(count)):
+        completionview.on_next_prev_item(count < 0)
+    idx = completionview.selectionModel().currentIndex()
+    assert filtermodel.data(idx) == expected

From d836fcb118989d8178705ef6f0d1086373faa466 Mon Sep 17 00:00:00 2001
From: Ryan Roden-Corrent <ryan@rcorre.net>
Date: Mon, 25 Jul 2016 07:07:47 -0400
Subject: [PATCH 05/11] Code review for completion tests.

- clean up docstring typos
- use _ to name an unused loop variable
- parent the filter model to avoid an issue with disposal
- use mocker.patch instead of monkeypatch to mock Completer creation
- use is instead of == to compare by identity
---
 qutebrowser/completion/completer.py            |  2 +-
 tests/unit/completion/test_completionwidget.py | 16 ++++++++--------
 2 files changed, 9 insertions(+), 9 deletions(-)

diff --git a/qutebrowser/completion/completer.py b/qutebrowser/completion/completer.py
index 418ee05c9..24c669180 100644
--- a/qutebrowser/completion/completer.py
+++ b/qutebrowser/completion/completer.py
@@ -355,7 +355,7 @@ class Completer(QObject):
             completion.show()
 
     def _split(self, keep=False):
-        """Get the text _split up in parts.
+        """Get the text split up in parts.
 
         Args:
             keep: Whether to keep special chars and whitespace.
diff --git a/tests/unit/completion/test_completionwidget.py b/tests/unit/completion/test_completionwidget.py
index 2148a59f5..02d3977eb 100644
--- a/tests/unit/completion/test_completionwidget.py
+++ b/tests/unit/completion/test_completionwidget.py
@@ -31,7 +31,7 @@ from qutebrowser.utils import objreg
 
 @pytest.yield_fixture
 def completionview(qtbot, status_command_stub, config_stub, win_registry,
-                   monkeypatch):
+                   mocker):
     """Create the CompletionView used for testing."""
     config_stub.data = {
         'completion': {
@@ -62,8 +62,7 @@ def completionview(qtbot, status_command_stub, config_stub, win_registry,
         }
     }
     # mock the Completer that the widget creates in its constructor
-    monkeypatch.setattr('qutebrowser.completion.completer.Completer',
-        lambda *args: unittest.mock.Mock())
+    mocker.patch('qutebrowser.completion.completer.Completer', autospec=True)
     view = completionwidget.CompletionView(win_id=0)
     qtbot.addWidget(view)
     yield view
@@ -79,7 +78,7 @@ def test_set_model(completionview):
     for i in range(3):
         model.appendRow(QStandardItem(str(i)))
     completionview.set_model(filtermodel)
-    assert completionview.model() == filtermodel
+    assert completionview.model() is filtermodel
     for i in range(model.rowCount()):
         assert completionview.isExpanded(filtermodel.index(i, 0))
 
@@ -130,8 +129,8 @@ def test_on_next_prev_item(tree, count, expected, completionview,
     """Test that on_next_prev_item moves the selection properly.
 
     Args:
-        tree: Each entry array represents a completion category, with each
-              string being an item under that category.
+        tree: Each list represents a completion category, with each string
+              being an item under that category.
         count: Number of times to go forward (or back if negative).
         expected: item data that should be selected after going back/forward.
     """
@@ -141,12 +140,13 @@ def test_on_next_prev_item(tree, count, expected, completionview,
         model.appendRow(cat)
         for name in catdata:
             cat.appendRow(QStandardItem(name))
-    filtermodel = sortfilter.CompletionFilterModel(model)
+    filtermodel = sortfilter.CompletionFilterModel(model,
+                                                   parent=completionview)
     completionview.set_model(filtermodel)
     # actually calling show() will pop a window during the test, so just fool
     # the completionview into thinking it is visible instead
     monkeypatch.setattr(completionview, 'isVisible', lambda: True)
-    for i in range(abs(count)):
+    for _ in range(abs(count)):
         completionview.on_next_prev_item(count < 0)
     idx = completionview.selectionModel().currentIndex()
     assert filtermodel.data(idx) == expected

From b9cf9d180baef76130b47307ac91c1a25fdfe40c Mon Sep 17 00:00:00 2001
From: Ryan Roden-Corrent <ryan@rcorre.net>
Date: Wed, 27 Jul 2016 07:39:25 -0400
Subject: [PATCH 06/11] Decouple Completer and CompletionView.

Rather than having a CompletionView instantiate and register a
Completer, instantiate both in MainWindow. The CompletionView is the
parent of the Completer, and communicates by emitting
selection_changed, meaning it no longer needs to contain a reference to
the Completer.
---
 qutebrowser/completion/completer.py            |  5 +++--
 qutebrowser/completion/completionwidget.py     | 15 +++++----------
 qutebrowser/mainwindow/mainwindow.py           | 11 ++++++++++-
 tests/unit/completion/test_completer.py        |  6 +++---
 tests/unit/completion/test_completionwidget.py |  7 ++-----
 5 files changed, 23 insertions(+), 21 deletions(-)

diff --git a/qutebrowser/completion/completer.py b/qutebrowser/completion/completer.py
index 24c669180..5a919bca5 100644
--- a/qutebrowser/completion/completer.py
+++ b/qutebrowser/completion/completer.py
@@ -19,7 +19,7 @@
 
 """Completer attached to a CompletionView."""
 
-from PyQt5.QtCore import pyqtSignal, pyqtSlot, QObject, QTimer
+from PyQt5.QtCore import pyqtSignal, pyqtSlot, QObject, QTimer, QItemSelection
 
 from qutebrowser.config import config
 from qutebrowser.commands import cmdutils, runners
@@ -249,7 +249,8 @@ class Completer(QObject):
         else:
             return s
 
-    def selection_changed(self, selected, _deselected):
+    @pyqtSlot(QItemSelection, QItemSelection)
+    def on_selection_changed(self, selected, _deselected):
         """Change the completed part if a new item was selected.
 
         Called from the views selectionChanged method.
diff --git a/qutebrowser/completion/completionwidget.py b/qutebrowser/completion/completionwidget.py
index 80e87dbe4..3e33f6614 100644
--- a/qutebrowser/completion/completionwidget.py
+++ b/qutebrowser/completion/completionwidget.py
@@ -24,7 +24,8 @@ subclasses to provide completions.
 """
 
 from PyQt5.QtWidgets import QStyle, QTreeView, QSizePolicy
-from PyQt5.QtCore import pyqtSlot, pyqtSignal, Qt, QItemSelectionModel
+from PyQt5.QtCore import (pyqtSlot, pyqtSignal, Qt, QItemSelectionModel,
+                          QItemSelection)
 
 from qutebrowser.config import config, style
 from qutebrowser.completion import completiondelegate, completer
@@ -50,6 +51,7 @@ class CompletionView(QTreeView):
 
     Signals:
         resize_completion: Emitted when the completion should be resized.
+        selection_changed: Emitted when the completion item selection changes.
     """
 
     # Drawing the item foreground will be done by CompletionItemDelegate, so we
@@ -102,16 +104,11 @@ class CompletionView(QTreeView):
     """
 
     resize_completion = pyqtSignal()
+    selection_changed = pyqtSignal(QItemSelection, QItemSelection)
 
     def __init__(self, win_id, parent=None):
         super().__init__(parent)
         self._win_id = win_id
-        objreg.register('completion', self, scope='window', window=win_id)
-        cmd = objreg.get('status-command', scope='window', window=win_id)
-        completer_obj = completer.Completer(cmd, win_id, self)
-        completer_obj.next_prev_item.connect(self.on_next_prev_item)
-        objreg.register('completer', completer_obj, scope='window',
-                        window=win_id)
         self.enabled = config.get('completion', 'show')
         objreg.get('config').changed.connect(self.set_enabled)
         # FIXME handle new aliases.
@@ -262,9 +259,7 @@ class CompletionView(QTreeView):
     def selectionChanged(self, selected, deselected):
         """Extend selectionChanged to call completers selection_changed."""
         super().selectionChanged(selected, deselected)
-        completer_obj = objreg.get('completer', scope='window',
-                                   window=self._win_id)
-        completer_obj.selection_changed(selected, deselected)
+        self.selection_changed.emit(selected, deselected)
 
     def resizeEvent(self, e):
         """Extend resizeEvent to adjust column size."""
diff --git a/qutebrowser/mainwindow/mainwindow.py b/qutebrowser/mainwindow/mainwindow.py
index 79a5546d3..1f549f847 100644
--- a/qutebrowser/mainwindow/mainwindow.py
+++ b/qutebrowser/mainwindow/mainwindow.py
@@ -32,7 +32,7 @@ from qutebrowser.config import config
 from qutebrowser.utils import message, log, usertypes, qtutils, objreg, utils
 from qutebrowser.mainwindow import tabbedbrowser
 from qutebrowser.mainwindow.statusbar import bar
-from qutebrowser.completion import completionwidget
+from qutebrowser.completion import completionwidget, completer
 from qutebrowser.keyinput import modeman
 from qutebrowser.browser import commands, downloadview, hints
 from qutebrowser.browser.webkit import downloads
@@ -158,6 +158,15 @@ class MainWindow(QWidget):
         self._downloadview.show()
 
         self._completion = completionwidget.CompletionView(self.win_id, self)
+        cmd = objreg.get('status-command', scope='window', window=self.win_id)
+        completer_obj = completer.Completer(cmd, self.win_id, self._completion)
+        completer_obj.next_prev_item.connect(self._completion.on_next_prev_item)
+        self._completion.selection_changed.connect(
+            completer_obj.on_selection_changed)
+        objreg.register('completer', completer_obj, scope='window',
+                        window=self.win_id)
+        objreg.register('completion', self._completion, scope='window',
+                        window=self.win_id)
 
         self._commandrunner = runners.CommandRunner(self.win_id,
                                                     partial_match=True)
diff --git a/tests/unit/completion/test_completer.py b/tests/unit/completion/test_completer.py
index 9335bbf70..381871657 100644
--- a/tests/unit/completion/test_completer.py
+++ b/tests/unit/completion/test_completer.py
@@ -193,10 +193,10 @@ def test_completion_item_next(completer_obj, status_command_stub,
     (':foo |', '', True, 1, ":foo '' |"),
     (':foo |', None, True, 1, ":foo |"),
 ])
-def test_selection_changed(before, newtxt, count, quick_complete, after,
+def test_on_selection_changed(before, newtxt, count, quick_complete, after,
                            completer_obj, status_command_stub,
                            completion_widget_stub, config_stub):
-    """Test that change_completed_part modifies the cmd text properly.
+    """Test that on_selection_changed modifies the cmd text properly.
 
     The | represents the current cursor position in the cmd prompt.
     If quick-complete is True and there is only 1 completion (count == 1),
@@ -213,6 +213,6 @@ def test_selection_changed(before, newtxt, count, quick_complete, after,
     _set_cmd_prompt(status_command_stub, before)
     # schedule_completion_update is needed to pick up the cursor position
     completer_obj.schedule_completion_update()
-    completer_obj.selection_changed(selection, None)
+    completer_obj.on_selection_changed(selection, None)
     model.data.assert_called_with(indexes[0])
     _validate_cmd_prompt(status_command_stub, after)
diff --git a/tests/unit/completion/test_completionwidget.py b/tests/unit/completion/test_completionwidget.py
index 02d3977eb..e52b85fa0 100644
--- a/tests/unit/completion/test_completionwidget.py
+++ b/tests/unit/completion/test_completionwidget.py
@@ -29,7 +29,7 @@ from qutebrowser.completion.models import base, sortfilter
 from qutebrowser.utils import objreg
 
 
-@pytest.yield_fixture
+@pytest.fixture
 def completionview(qtbot, status_command_stub, config_stub, win_registry,
                    mocker):
     """Create the CompletionView used for testing."""
@@ -65,10 +65,7 @@ def completionview(qtbot, status_command_stub, config_stub, win_registry,
     mocker.patch('qutebrowser.completion.completer.Completer', autospec=True)
     view = completionwidget.CompletionView(win_id=0)
     qtbot.addWidget(view)
-    yield view
-    # the constructor registers both 'completion' (itself) and 'completer'
-    objreg.delete('completion', scope='window', window=0)
-    objreg.delete('completer', scope='window', window=0)
+    return view
 
 
 def test_set_model(completionview):

From ffc5a42d0475f07583cefc2f6ee7f6bea0809a52 Mon Sep 17 00:00:00 2001
From: Ryan Roden-Corrent <ryan@rcorre.net>
Date: Wed, 27 Jul 2016 12:14:42 -0400
Subject: [PATCH 07/11] Don't use objreg to get CompletionView.

The CompletionView is the parent of the Completer, so there's no need
to use objreg to get it.
See #640.
---
 qutebrowser/completion/completer.py     |  6 ++---
 tests/helpers/fixtures.py               |  8 ------
 tests/unit/completion/test_completer.py | 35 ++++++++++++++++++++-----
 3 files changed, 30 insertions(+), 19 deletions(-)

diff --git a/qutebrowser/completion/completer.py b/qutebrowser/completion/completer.py
index 5a919bca5..8ad072713 100644
--- a/qutebrowser/completion/completer.py
+++ b/qutebrowser/completion/completer.py
@@ -125,8 +125,7 @@ class Completer(QObject):
 
     def _model(self):
         """Convenience method to get the current completion model."""
-        completion = objreg.get('completion', scope='window',
-                                window=self._win_id)
+        completion = self.parent()
         return completion.model()
 
     def _get_completion_model(self, completion, parts, cursor_part):
@@ -315,8 +314,7 @@ class Completer(QObject):
             self._ignore_change = False
             return
 
-        completion = objreg.get('completion', scope='window',
-                                window=self._win_id)
+        completion = self.parent()
 
         if self._cmd.prefix() != ':':
             # This is a search or gibberish, so we don't need to complete
diff --git a/tests/helpers/fixtures.py b/tests/helpers/fixtures.py
index 7454b46e4..749f0d4f0 100644
--- a/tests/helpers/fixtures.py
+++ b/tests/helpers/fixtures.py
@@ -267,14 +267,6 @@ def app_stub(stubs):
     objreg.delete('app')
 
 
-@pytest.yield_fixture
-def completion_widget_stub(win_registry):
-    stub = unittest.mock.Mock()
-    objreg.register('completion', stub, scope='window', window=0)
-    yield stub
-    objreg.delete('completion', scope='window', window=0)
-
-
 @pytest.yield_fixture
 def status_command_stub(stubs, qtbot, win_registry):
     """Fixture which provides a fake status-command object."""
diff --git a/tests/unit/completion/test_completer.py b/tests/unit/completion/test_completer.py
index 381871657..67232a0a0 100644
--- a/tests/unit/completion/test_completer.py
+++ b/tests/unit/completion/test_completer.py
@@ -22,6 +22,7 @@
 import unittest.mock
 
 import pytest
+from PyQt5.QtCore import QObject
 from PyQt5.QtGui import QStandardItemModel
 
 from qutebrowser.completion import completer
@@ -39,13 +40,33 @@ class FakeCompletionModel(QStandardItemModel):
         self.kind = kind
 
 
+class CompletionWidgetStub(QObject):
+
+    """Stub for the CompletionView."""
+
+    def __init__(self, parent=None):
+        super().__init__(parent)
+        self.hide = unittest.mock.Mock()
+        self.show = unittest.mock.Mock()
+        self.set_pattern = unittest.mock.Mock()
+        self.model = unittest.mock.Mock()
+        self.set_model = unittest.mock.Mock()
+        self.enabled = unittest.mock.Mock()
+
+
 @pytest.fixture
-def completer_obj(qtbot, status_command_stub, config_stub, monkeypatch, stubs):
+def completion_widget_stub():
+    return CompletionWidgetStub()
+
+
+@pytest.fixture
+def completer_obj(qtbot, status_command_stub, config_stub, monkeypatch, stubs,
+                  completion_widget_stub):
     """Create the completer used for testing."""
     monkeypatch.setattr('qutebrowser.completion.completer.QTimer',
         stubs.InstaTimer)
     config_stub.data = {'completion': {'auto-open': False}}
-    return completer.Completer(status_command_stub, 0)
+    return completer.Completer(status_command_stub, 0, completion_widget_stub)
 
 
 @pytest.fixture(autouse=True)
@@ -166,8 +187,8 @@ def test_update_completion(txt, expected, status_command_stub, completer_obj,
         assert arg.srcmodel.kind == expected
 
 
-def test_completion_item_prev(completer_obj, status_command_stub,
-                              completion_widget_stub, config_stub, qtbot):
+def test_completion_item_prev(completer_obj, status_command_stub, config_stub,
+                              qtbot):
     """Test that completion_item_prev emits next_prev_item."""
     status_command_stub.setText(':')
     with qtbot.waitSignal(completer_obj.next_prev_item) as blocker:
@@ -175,8 +196,8 @@ def test_completion_item_prev(completer_obj, status_command_stub,
     assert blocker.args == [True]
 
 
-def test_completion_item_next(completer_obj, status_command_stub,
-                              completion_widget_stub, config_stub, qtbot):
+def test_completion_item_next(completer_obj, status_command_stub, config_stub,
+                              qtbot):
     """Test that completion_item_next emits next_prev_item."""
     status_command_stub.setText(':')
     with qtbot.waitSignal(completer_obj.next_prev_item) as blocker:
@@ -209,7 +230,7 @@ def test_on_selection_changed(before, newtxt, count, quick_complete, after,
     indexes = [unittest.mock.Mock()]
     selection = unittest.mock.Mock()
     selection.indexes = unittest.mock.Mock(return_value=indexes)
-    completion_widget_stub.model = unittest.mock.Mock(return_value=model)
+    completion_widget_stub.model.return_value = model
     _set_cmd_prompt(status_command_stub, before)
     # schedule_completion_update is needed to pick up the cursor position
     completer_obj.schedule_completion_update()

From f31e8908627edce679127712b096c2b04a16e07f Mon Sep 17 00:00:00 2001
From: Ryan Roden-Corrent <ryan@rcorre.net>
Date: Wed, 27 Jul 2016 22:29:38 -0400
Subject: [PATCH 08/11] Move completion_item_{next,prev} to CompletionView.

These commands are more closely tied to the CompletionView than
Completer. This removes the need for an extra signal tying the
CompletionView to the Completer.

The call to _open_completion_if_needed was moved to
on_selection_changed, as this will already be called when a new item is
selected.
---
 qutebrowser/completion/completer.py           | 22 +------------------
 qutebrowser/completion/completionwidget.py    | 18 ++++++++++-----
 qutebrowser/mainwindow/mainwindow.py          |  3 ---
 tests/unit/completion/test_completer.py       | 18 ---------------
 .../unit/completion/test_completionwidget.py  | 15 ++++++-------
 5 files changed, 21 insertions(+), 55 deletions(-)

diff --git a/qutebrowser/completion/completer.py b/qutebrowser/completion/completer.py
index 8ad072713..ca3ffbe95 100644
--- a/qutebrowser/completion/completer.py
+++ b/qutebrowser/completion/completer.py
@@ -42,15 +42,8 @@ class Completer(QObject):
         _last_text: The old command text so we avoid double completion updates.
         _signals_connected: Whether the signals are connected to update the
                             completion when the command widget requests that.
-
-    Signals:
-        next_prev_item: Emitted to select the next/previous item in the
-                        completion.
-                        arg0: True for the previous item, False for the next.
     """
 
-    next_prev_item = pyqtSignal(bool)
-
     def __init__(self, cmd, win_id, parent=None):
         super().__init__(parent)
         self._win_id = win_id
@@ -258,6 +251,7 @@ class Completer(QObject):
             selected: New selection.
             _deselected: Previous selection.
         """
+        self._open_completion_if_needed()
         indexes = selected.indexes()
         if not indexes:
             return
@@ -470,17 +464,3 @@ class Completer(QObject):
         # We also want to update the cursor part and emit _update_completion
         # here, but that's already done for us by cursorPositionChanged
         # anyways, so we don't need to do it twice.
-
-    @cmdutils.register(instance='completer', hide=True,
-                       modes=[usertypes.KeyMode.command], scope='window')
-    def completion_item_prev(self):
-        """Select the previous completion item."""
-        self._open_completion_if_needed()
-        self.next_prev_item.emit(True)
-
-    @cmdutils.register(instance='completer', hide=True,
-                       modes=[usertypes.KeyMode.command], scope='window')
-    def completion_item_next(self):
-        """Select the next completion item."""
-        self._open_completion_if_needed()
-        self.next_prev_item.emit(False)
diff --git a/qutebrowser/completion/completionwidget.py b/qutebrowser/completion/completionwidget.py
index 3e33f6614..aa749da89 100644
--- a/qutebrowser/completion/completionwidget.py
+++ b/qutebrowser/completion/completionwidget.py
@@ -181,8 +181,7 @@ class CompletionView(QTreeView):
                 # Item is a real item, not a category header -> success
                 return idx
 
-    @pyqtSlot(bool)
-    def on_next_prev_item(self, prev):
+    def _next_prev_item(self, prev):
         """Handle a tab press for the CompletionView.
 
         Select the previous/next item and write the new text to the
@@ -193,9 +192,6 @@ class CompletionView(QTreeView):
         Args:
             prev: True for prev item, False for next one.
         """
-        if not self.isVisible():
-            # No completion running at the moment, ignore keypress
-            return
         idx = self._next_idx(prev)
         qtutils.ensure_valid(idx)
         self.selectionModel().setCurrentIndex(
@@ -274,6 +270,18 @@ class CompletionView(QTreeView):
             scrollbar.setValue(scrollbar.minimum())
         super().showEvent(e)
 
+    @cmdutils.register(instance='completion', hide=True,
+                       modes=[usertypes.KeyMode.command], scope='window')
+    def completion_item_prev(self):
+        """Select the previous completion item."""
+        self._next_prev_item(True)
+
+    @cmdutils.register(instance='completion', hide=True,
+                       modes=[usertypes.KeyMode.command], scope='window')
+    def completion_item_next(self):
+        """Select the next completion item."""
+        self._next_prev_item(False)
+
     @cmdutils.register(instance='completion', hide=True,
                        modes=[usertypes.KeyMode.command], scope='window')
     def completion_item_del(self):
diff --git a/qutebrowser/mainwindow/mainwindow.py b/qutebrowser/mainwindow/mainwindow.py
index 1f549f847..dc2d55ccd 100644
--- a/qutebrowser/mainwindow/mainwindow.py
+++ b/qutebrowser/mainwindow/mainwindow.py
@@ -160,11 +160,8 @@ class MainWindow(QWidget):
         self._completion = completionwidget.CompletionView(self.win_id, self)
         cmd = objreg.get('status-command', scope='window', window=self.win_id)
         completer_obj = completer.Completer(cmd, self.win_id, self._completion)
-        completer_obj.next_prev_item.connect(self._completion.on_next_prev_item)
         self._completion.selection_changed.connect(
             completer_obj.on_selection_changed)
-        objreg.register('completer', completer_obj, scope='window',
-                        window=self.win_id)
         objreg.register('completion', self._completion, scope='window',
                         window=self.win_id)
 
diff --git a/tests/unit/completion/test_completer.py b/tests/unit/completion/test_completer.py
index 67232a0a0..c5b4ccdf8 100644
--- a/tests/unit/completion/test_completer.py
+++ b/tests/unit/completion/test_completer.py
@@ -187,24 +187,6 @@ def test_update_completion(txt, expected, status_command_stub, completer_obj,
         assert arg.srcmodel.kind == expected
 
 
-def test_completion_item_prev(completer_obj, status_command_stub, config_stub,
-                              qtbot):
-    """Test that completion_item_prev emits next_prev_item."""
-    status_command_stub.setText(':')
-    with qtbot.waitSignal(completer_obj.next_prev_item) as blocker:
-        completer_obj.completion_item_prev()
-    assert blocker.args == [True]
-
-
-def test_completion_item_next(completer_obj, status_command_stub, config_stub,
-                              qtbot):
-    """Test that completion_item_next emits next_prev_item."""
-    status_command_stub.setText(':')
-    with qtbot.waitSignal(completer_obj.next_prev_item) as blocker:
-        completer_obj.completion_item_next()
-    assert blocker.args == [False]
-
-
 @pytest.mark.parametrize('before, newtxt, quick_complete, count, after', [
     (':foo |', 'bar', False, 1, ':foo bar|'),
     (':foo |', 'bar', True, 2, ':foo bar|'),
diff --git a/tests/unit/completion/test_completionwidget.py b/tests/unit/completion/test_completionwidget.py
index e52b85fa0..ac9f3bc0d 100644
--- a/tests/unit/completion/test_completionwidget.py
+++ b/tests/unit/completion/test_completionwidget.py
@@ -120,9 +120,7 @@ def test_maybe_resize_completion(completionview, config_stub, qtbot):
     ([['Aa'], [], []], 1, 'Aa'),
     ([['Aa'], [], []], -1, 'Aa'),
 ])
-def test_on_next_prev_item(tree, count, expected, completionview,
-                           config_stub, qtbot, monkeypatch,
-                           status_command_stub):
+def test_completion_item_next_prev(tree, count, expected, completionview):
     """Test that on_next_prev_item moves the selection properly.
 
     Args:
@@ -140,10 +138,11 @@ def test_on_next_prev_item(tree, count, expected, completionview,
     filtermodel = sortfilter.CompletionFilterModel(model,
                                                    parent=completionview)
     completionview.set_model(filtermodel)
-    # actually calling show() will pop a window during the test, so just fool
-    # the completionview into thinking it is visible instead
-    monkeypatch.setattr(completionview, 'isVisible', lambda: True)
-    for _ in range(abs(count)):
-        completionview.on_next_prev_item(count < 0)
+    if count < 0:
+        for _ in range(-count):
+            completionview.completion_item_prev()
+    else:
+        for _ in range(count):
+            completionview.completion_item_next()
     idx = completionview.selectionModel().currentIndex()
     assert filtermodel.data(idx) == expected

From dd827332c02ed0c8528e41a96408ae30ec16974a Mon Sep 17 00:00:00 2001
From: Ryan Roden-Corrent <ryan@rcorre.net>
Date: Thu, 28 Jul 2016 07:26:20 -0400
Subject: [PATCH 09/11] Clean up unused imports from completion refactor.

---
 qutebrowser/completion/completer.py            | 2 +-
 qutebrowser/completion/completionwidget.py     | 4 ++--
 tests/unit/completion/test_completionwidget.py | 1 -
 3 files changed, 3 insertions(+), 4 deletions(-)

diff --git a/qutebrowser/completion/completer.py b/qutebrowser/completion/completer.py
index ca3ffbe95..92e774ed2 100644
--- a/qutebrowser/completion/completer.py
+++ b/qutebrowser/completion/completer.py
@@ -19,7 +19,7 @@
 
 """Completer attached to a CompletionView."""
 
-from PyQt5.QtCore import pyqtSignal, pyqtSlot, QObject, QTimer, QItemSelection
+from PyQt5.QtCore import pyqtSlot, QObject, QTimer, QItemSelection
 
 from qutebrowser.config import config
 from qutebrowser.commands import cmdutils, runners
diff --git a/qutebrowser/completion/completionwidget.py b/qutebrowser/completion/completionwidget.py
index aa749da89..8feefffd9 100644
--- a/qutebrowser/completion/completionwidget.py
+++ b/qutebrowser/completion/completionwidget.py
@@ -28,7 +28,7 @@ from PyQt5.QtCore import (pyqtSlot, pyqtSignal, Qt, QItemSelectionModel,
                           QItemSelection)
 
 from qutebrowser.config import config, style
-from qutebrowser.completion import completiondelegate, completer
+from qutebrowser.completion import completiondelegate
 from qutebrowser.completion.models import base
 from qutebrowser.utils import qtutils, objreg, utils, usertypes
 from qutebrowser.commands import cmdexc, cmdutils
@@ -187,7 +187,7 @@ class CompletionView(QTreeView):
         Select the previous/next item and write the new text to the
         statusbar.
 
-        Called from the Completer's next_prev_item signal.
+        Helper for completion_item_next and completion_item_prev.
 
         Args:
             prev: True for prev item, False for next one.
diff --git a/tests/unit/completion/test_completionwidget.py b/tests/unit/completion/test_completionwidget.py
index ac9f3bc0d..9bc2a2e55 100644
--- a/tests/unit/completion/test_completionwidget.py
+++ b/tests/unit/completion/test_completionwidget.py
@@ -26,7 +26,6 @@ from PyQt5.QtGui import QStandardItem, QColor
 
 from qutebrowser.completion import completionwidget
 from qutebrowser.completion.models import base, sortfilter
-from qutebrowser.utils import objreg
 
 
 @pytest.fixture

From e8f73b0fe6a475eee9b6de053c0afbdd2ad1183d Mon Sep 17 00:00:00 2001
From: Ryan Roden-Corrent <ryan@rcorre.net>
Date: Thu, 28 Jul 2016 08:43:01 -0400
Subject: [PATCH 10/11] Break up MainWindow.__init__.

Split out initialization of a few areas into private functions so
pylint won't complain about a long method.
---
 qutebrowser/mainwindow/mainwindow.py | 46 ++++++++++++++++------------
 1 file changed, 27 insertions(+), 19 deletions(-)

diff --git a/qutebrowser/mainwindow/mainwindow.py b/qutebrowser/mainwindow/mainwindow.py
index dc2d55ccd..826472518 100644
--- a/qutebrowser/mainwindow/mainwindow.py
+++ b/qutebrowser/mainwindow/mainwindow.py
@@ -131,23 +131,13 @@ class MainWindow(QWidget):
         self._vbox.setContentsMargins(0, 0, 0, 0)
         self._vbox.setSpacing(0)
 
-        log.init.debug("Initializing downloads...")
-        download_manager = downloads.DownloadManager(self.win_id, self)
-        objreg.register('download-manager', download_manager, scope='window',
-                        window=self.win_id)
-
+        self._init_downloadmanager()
         self._downloadview = downloadview.DownloadView(self.win_id)
 
         self.tabbed_browser = tabbedbrowser.TabbedBrowser(self.win_id)
         objreg.register('tabbed-browser', self.tabbed_browser, scope='window',
                         window=self.win_id)
-        dispatcher = commands.CommandDispatcher(self.win_id,
-                                                self.tabbed_browser)
-        objreg.register('command-dispatcher', dispatcher, scope='window',
-                        window=self.win_id)
-        self.tabbed_browser.destroyed.connect(
-            functools.partial(objreg.delete, 'command-dispatcher',
-                              scope='window', window=self.win_id))
+        self._init_command_dispatcher()
 
         # We need to set an explicit parent for StatusBar because it does some
         # show/hide magic immediately which would mean it'd show up as a
@@ -157,13 +147,7 @@ class MainWindow(QWidget):
         self._add_widgets()
         self._downloadview.show()
 
-        self._completion = completionwidget.CompletionView(self.win_id, self)
-        cmd = objreg.get('status-command', scope='window', window=self.win_id)
-        completer_obj = completer.Completer(cmd, self.win_id, self._completion)
-        self._completion.selection_changed.connect(
-            completer_obj.on_selection_changed)
-        objreg.register('completion', self._completion, scope='window',
-                        window=self.win_id)
+        self._init_completion()
 
         self._commandrunner = runners.CommandRunner(self.win_id,
                                                     partial_match=True)
@@ -196,6 +180,30 @@ class MainWindow(QWidget):
 
         objreg.get("app").new_window.emit(self)
 
+    def _init_downloadmanager(self):
+        log.init.debug("Initializing downloads...")
+        download_manager = downloads.DownloadManager(self.win_id, self)
+        objreg.register('download-manager', download_manager, scope='window',
+                        window=self.win_id)
+
+    def _init_completion(self):
+        self._completion = completionwidget.CompletionView(self.win_id, self)
+        cmd = objreg.get('status-command', scope='window', window=self.win_id)
+        completer_obj = completer.Completer(cmd, self.win_id, self._completion)
+        self._completion.selection_changed.connect(
+            completer_obj.on_selection_changed)
+        objreg.register('completion', self._completion, scope='window',
+                        window=self.win_id)
+
+    def _init_command_dispatcher(self):
+        dispatcher = commands.CommandDispatcher(self.win_id,
+                                                self.tabbed_browser)
+        objreg.register('command-dispatcher', dispatcher, scope='window',
+                        window=self.win_id)
+        self.tabbed_browser.destroyed.connect(
+            functools.partial(objreg.delete, 'command-dispatcher',
+                              scope='window', window=self.win_id))
+
     def __repr__(self):
         return utils.get_repr(self)
 

From fbadc5e6685e2036aa460881898903658985e082 Mon Sep 17 00:00:00 2001
From: Ryan Roden-Corrent <ryan@rcorre.net>
Date: Fri, 29 Jul 2016 08:48:24 -0400
Subject: [PATCH 11/11] Remove second arg from on_selection_changed.

The deselected argument was unused, so remove it from the signal and
the slot.
---
 qutebrowser/completion/completer.py        | 4 ++--
 qutebrowser/completion/completionwidget.py | 4 ++--
 tests/unit/completion/test_completer.py    | 2 +-
 3 files changed, 5 insertions(+), 5 deletions(-)

diff --git a/qutebrowser/completion/completer.py b/qutebrowser/completion/completer.py
index 92e774ed2..71202d131 100644
--- a/qutebrowser/completion/completer.py
+++ b/qutebrowser/completion/completer.py
@@ -241,8 +241,8 @@ class Completer(QObject):
         else:
             return s
 
-    @pyqtSlot(QItemSelection, QItemSelection)
-    def on_selection_changed(self, selected, _deselected):
+    @pyqtSlot(QItemSelection)
+    def on_selection_changed(self, selected):
         """Change the completed part if a new item was selected.
 
         Called from the views selectionChanged method.
diff --git a/qutebrowser/completion/completionwidget.py b/qutebrowser/completion/completionwidget.py
index 8feefffd9..9fc1ef5d7 100644
--- a/qutebrowser/completion/completionwidget.py
+++ b/qutebrowser/completion/completionwidget.py
@@ -104,7 +104,7 @@ class CompletionView(QTreeView):
     """
 
     resize_completion = pyqtSignal()
-    selection_changed = pyqtSignal(QItemSelection, QItemSelection)
+    selection_changed = pyqtSignal(QItemSelection)
 
     def __init__(self, win_id, parent=None):
         super().__init__(parent)
@@ -255,7 +255,7 @@ class CompletionView(QTreeView):
     def selectionChanged(self, selected, deselected):
         """Extend selectionChanged to call completers selection_changed."""
         super().selectionChanged(selected, deselected)
-        self.selection_changed.emit(selected, deselected)
+        self.selection_changed.emit(selected)
 
     def resizeEvent(self, e):
         """Extend resizeEvent to adjust column size."""
diff --git a/tests/unit/completion/test_completer.py b/tests/unit/completion/test_completer.py
index c5b4ccdf8..29196ea3b 100644
--- a/tests/unit/completion/test_completer.py
+++ b/tests/unit/completion/test_completer.py
@@ -216,6 +216,6 @@ def test_on_selection_changed(before, newtxt, count, quick_complete, after,
     _set_cmd_prompt(status_command_stub, before)
     # schedule_completion_update is needed to pick up the cursor position
     completer_obj.schedule_completion_update()
-    completer_obj.on_selection_changed(selection, None)
+    completer_obj.on_selection_changed(selection)
     model.data.assert_called_with(indexes[0])
     _validate_cmd_prompt(status_command_stub, after)