From 1aed2470e5912264c8c90be58b97a729fd0e8bdf Mon Sep 17 00:00:00 2001
From: Ryan Roden-Corrent <ryan@rcorre.net>
Date: Wed, 12 Jul 2017 22:14:27 -0400
Subject: [PATCH] SQL code review.

- Fix flake8
- history.clear should also clear completion table
- call _resize_columns in set_model, not set_pattern
- add more unit-testing for the history completion table
---
 .flake8                                    |  4 ++--
 qutebrowser/browser/history.py             |  1 +
 qutebrowser/completion/completionwidget.py |  3 ++-
 tests/unit/browser/test_history.py         | 16 ++++++++++++++--
 4 files changed, 19 insertions(+), 5 deletions(-)

diff --git a/.flake8 b/.flake8
index b87fef8b2..eada2c86d 100644
--- a/.flake8
+++ b/.flake8
@@ -28,7 +28,7 @@ exclude = .*,__pycache__,resources.py
 ignore =
     E128,E226,E265,E501,E402,E266,E722,E731,
     F401,
-    N802,N806
+    N802,
     P101,P102,P103,
     D102,D103,D104,D105,D209,D211,D402,D403
 min-version = 3.4.0
@@ -39,7 +39,7 @@ putty-ignore =
     /# pragma: no mccabe/ : +C901
     tests/*/test_*.py : +D100,D101,D401
     tests/conftest.py : +F403
-    tests/unit/browser/webkit/test_history.py : +N806
+    tests/unit/browser/test_history.py : +N806
     tests/helpers/fixtures.py : +N806
     tests/unit/browser/webkit/http/test_content_disposition.py : +D400
     scripts/dev/ci/appveyor_install.py : +FI53
diff --git a/qutebrowser/browser/history.py b/qutebrowser/browser/history.py
index b5b8af149..b9e791207 100644
--- a/qutebrowser/browser/history.py
+++ b/qutebrowser/browser/history.py
@@ -115,6 +115,7 @@ class WebHistory(sql.SqlTable):
 
     def _do_clear(self):
         self.delete_all()
+        self.completion.delete_all()
 
     def delete_url(self, url):
         """Remove all history entries with the given url.
diff --git a/qutebrowser/completion/completionwidget.py b/qutebrowser/completion/completionwidget.py
index 68466630a..6e1e51680 100644
--- a/qutebrowser/completion/completionwidget.py
+++ b/qutebrowser/completion/completionwidget.py
@@ -286,16 +286,17 @@ class CompletionView(QTreeView):
         self._active = True
         self._maybe_show()
 
+        self._resize_columns()
         for i in range(model.rowCount()):
             self.expand(model.index(i, 0))
 
     def set_pattern(self, pattern):
+        """Set the pattern on the underlying model."""
         if not self.model():
             return
         self.pattern = pattern
         with debug.log_time(log.completion, 'Set pattern {}'.format(pattern)):
             self.model().set_pattern(pattern)
-            self._resize_columns()
             self._maybe_update_geometry()
             self._maybe_show()
 
diff --git a/tests/unit/browser/test_history.py b/tests/unit/browser/test_history.py
index 5a8425646..81637f3d4 100644
--- a/tests/unit/browser/test_history.py
+++ b/tests/unit/browser/test_history.py
@@ -123,6 +123,7 @@ def test_clear_force(qtbot, tmpdir, hist):
     hist.add_url(QUrl('http://www.qutebrowser.org/'))
     hist.clear(force=True)
     assert not len(hist)
+    assert not len(hist.completion)
 
 
 def test_delete_url(hist):
@@ -131,10 +132,16 @@ def test_delete_url(hist):
     hist.add_url(QUrl('http://example.com/2'), atime=0)
 
     before = set(hist)
+    completion_before = set(hist.completion)
+
     hist.delete_url(QUrl('http://example.com/1'))
+
     diff = before.difference(set(hist))
     assert diff == {('http://example.com/1', '', 0, False)}
 
+    completion_diff = completion_before.difference(set(hist.completion))
+    assert completion_diff == {('http://example.com/1', '', 0)}
+
 
 @pytest.mark.parametrize('url, atime, title, redirect, expected_url', [
     ('http://www.example.com', 12346, 'the title', False,
@@ -146,15 +153,20 @@ def test_delete_url(hist):
     ('https://user:pass@example.com', 12346, 'the title', False,
         'https://user@example.com'),
 ])
-def test_add_item(qtbot, hist, url, atime, title, redirect, expected_url):
+def test_add_url(qtbot, hist, url, atime, title, redirect, expected_url):
     hist.add_url(QUrl(url), atime=atime, title=title, redirect=redirect)
     assert list(hist) == [(expected_url, title, atime, redirect)]
+    if redirect:
+        assert not len(hist.completion)
+    else:
+        assert list(hist.completion) == [(expected_url, title, atime)]
 
 
-def test_add_item_invalid(qtbot, hist, caplog):
+def test_add_url_invalid(qtbot, hist, caplog):
     with caplog.at_level(logging.WARNING):
         hist.add_url(QUrl())
     assert not list(hist)
+    assert not list(hist.completion)
 
 
 @pytest.mark.parametrize('level, url, req_url, expected', [