From 8909e03f1cea998b6e997486e2541bce3d94d71b Mon Sep 17 00:00:00 2001 From: Ryan Roden-Corrent Date: Sun, 10 Dec 2017 15:06:45 -0500 Subject: [PATCH] Match url completion terms in any order. Perviously, 'foo bar' would match 'foo/bar' but not 'bar/foo'. Now it will match both, using a query with a WHERE clause like: WHERE ((url || title) like '%foo%' AND (url || title) like '%bar%') This does not seem to change the performance benchmark. However, it does create a new query for every character added rather than re-running the same query with different parameters. We could re-use queries if we maintained a list like self._queries=[1_arg_query, 2_arg_query, ...]. However, it isn't clear that such a complexity would be necessary. Resolves #1651. --- qutebrowser/completion/models/histcategory.py | 42 ++++++++++--------- tests/unit/completion/test_histcategory.py | 2 +- tests/unit/completion/test_models.py | 4 +- 3 files changed, 26 insertions(+), 22 deletions(-) diff --git a/qutebrowser/completion/models/histcategory.py b/qutebrowser/completion/models/histcategory.py index fe89dc79b..03ddb5ff8 100644 --- a/qutebrowser/completion/models/histcategory.py +++ b/qutebrowser/completion/models/histcategory.py @@ -37,21 +37,6 @@ class HistoryCategory(QSqlQueryModel): super().__init__(parent=parent) self.name = "History" - # replace ' in timestamp-format to avoid breaking the query - timestamp_format = config.val.completion.timestamp_format - timefmt = ("strftime('{}', last_atime, 'unixepoch', 'localtime')" - .format(timestamp_format.replace("'", "`"))) - - self._query = sql.Query(' '.join([ - "SELECT url, title, {}".format(timefmt), - "FROM CompletionHistory", - # the incoming pattern will have literal % and _ escaped with '\' - # we need to tell sql to treat '\' as an escape character - "WHERE ((url || title) LIKE :pat escape '\\')", - self._atime_expr(), - "ORDER BY last_atime DESC", - ]), forward_only=False) - # advertise that this model filters by URL and title self.columns_to_filter = [0, 1] self.delete_func = delete_func @@ -86,11 +71,30 @@ class HistoryCategory(QSqlQueryModel): # escape to treat a user input % or _ as a literal, not a wildcard pattern = pattern.replace('%', '\\%') pattern = pattern.replace('_', '\\_') - # treat spaces as wildcards to match any of the typed words - pattern = re.sub(r' +', '%', pattern) - pattern = '%{}%'.format(pattern) + words = ['%{}%'.format(w) for w in pattern.split(' ')] + + wheres = ' AND '.join([ + "(url || title) LIKE :pat{} escape '\\'".format(i) + for i in range(len(words))]) + + # replace ' in timestamp-format to avoid breaking the query + timestamp_format = config.val.completion.timestamp_format + timefmt = ("strftime('{}', last_atime, 'unixepoch', 'localtime')" + .format(timestamp_format.replace("'", "`"))) + + self._query = sql.Query(' '.join([ + "SELECT url, title, {}".format(timefmt), + "FROM CompletionHistory", + # the incoming pattern will have literal % and _ escaped with '\' + # we need to tell sql to treat '\' as an escape character + 'WHERE ({})'.format(wheres), + self._atime_expr(), + "ORDER BY last_atime DESC", + ]), forward_only=False) + with debug.log_time('sql', 'Running completion query'): - self._query.run(pat=pattern) + self._query.run(**{ + 'pat{}'.format(i): w for i, w in enumerate(words)}) self.setQuery(self._query) def removeRows(self, row, _count, _parent=None): diff --git a/tests/unit/completion/test_histcategory.py b/tests/unit/completion/test_histcategory.py index b87eb6ac2..8458c3311 100644 --- a/tests/unit/completion/test_histcategory.py +++ b/tests/unit/completion/test_histcategory.py @@ -61,7 +61,7 @@ def hist(init_sql, config_stub): ('foo bar', [('foo', ''), ('bar foo', ''), ('xfooyybarz', '')], - [('xfooyybarz', '')]), + [('bar foo', ''), ('xfooyybarz', '')]), ('foo%bar', [('foo%bar', ''), ('foo bar', ''), ('foobar', '')], diff --git a/tests/unit/completion/test_models.py b/tests/unit/completion/test_models.py index 8879f3201..c4d224dcc 100644 --- a/tests/unit/completion/test_models.py +++ b/tests/unit/completion/test_models.py @@ -414,12 +414,12 @@ def test_url_completion_no_bookmarks(qtmodeltester, web_history_populated, ('example.com', 'Site Title', 'am', 1), ('example.com', 'Site Title', 'com', 1), ('example.com', 'Site Title', 'ex com', 1), - ('example.com', 'Site Title', 'com ex', 0), + ('example.com', 'Site Title', 'com ex', 1), ('example.com', 'Site Title', 'ex foo', 0), ('example.com', 'Site Title', 'foo com', 0), ('example.com', 'Site Title', 'exm', 0), ('example.com', 'Site Title', 'Si Ti', 1), - ('example.com', 'Site Title', 'Ti Si', 0), + ('example.com', 'Site Title', 'Ti Si', 1), ('example.com', '', 'foo', 0), ('foo_bar', '', '_', 1), ('foobar', '', '_', 0),