Simplify sorting logic in sortfilter.
For URL completion, time-based sorting is handled by the SQL model. All the other models use simple alphabetical sorting. This allowed cleaning up some logic in the sortfilter, removing DUMB_SORT, and removing the completion.Role.sort. This also removes the userdata completion field as it was only used in url completion and is no longer necessary with the SQL model.
This commit is contained in:
parent
acea0d3c67
commit
3005374ada
@ -29,10 +29,6 @@ from PyQt5.QtGui import QStandardItemModel, QStandardItem
|
||||
from qutebrowser.utils import usertypes
|
||||
|
||||
|
||||
Role = usertypes.enum('Role', ['sort', 'userdata'], start=Qt.UserRole,
|
||||
is_int=True)
|
||||
|
||||
|
||||
class CompletionModel(QStandardItemModel):
|
||||
|
||||
"""A simple QStandardItemModel adopted for completions.
|
||||
@ -42,37 +38,30 @@ class CompletionModel(QStandardItemModel):
|
||||
|
||||
Attributes:
|
||||
column_widths: The width percentages of the columns used in the
|
||||
completion view.
|
||||
dumb_sort: the dumb sorting used by the model
|
||||
"""
|
||||
|
||||
def __init__(self, dumb_sort=None, column_widths=(30, 70, 0),
|
||||
columns_to_filter=None, delete_cur_item=None, parent=None):
|
||||
def __init__(self, column_widths=(30, 70, 0), columns_to_filter=None,
|
||||
delete_cur_item=None, parent=None):
|
||||
super().__init__(parent)
|
||||
self.setColumnCount(3)
|
||||
self.columns_to_filter = columns_to_filter or [0]
|
||||
self.dumb_sort = dumb_sort
|
||||
self.column_widths = column_widths
|
||||
self.delete_cur_item = delete_cur_item
|
||||
|
||||
def new_category(self, name, sort=None):
|
||||
def new_category(self, name):
|
||||
"""Add a new category to the model.
|
||||
|
||||
Args:
|
||||
name: The name of the category to add.
|
||||
sort: The value to use for the sort role.
|
||||
|
||||
Return:
|
||||
The created QStandardItem.
|
||||
"""
|
||||
cat = QStandardItem(name)
|
||||
if sort is not None:
|
||||
cat.setData(sort, Role.sort)
|
||||
self.appendRow(cat)
|
||||
return cat
|
||||
|
||||
def new_item(self, cat, name, desc='', misc=None, sort=None,
|
||||
userdata=None):
|
||||
def new_item(self, cat, name, desc='', misc=None):
|
||||
"""Add a new item to a category.
|
||||
|
||||
Args:
|
||||
@ -80,8 +69,6 @@ class CompletionModel(QStandardItemModel):
|
||||
name: The name of the item.
|
||||
desc: The description of the item.
|
||||
misc: Misc text to display.
|
||||
sort: Data for the sort role (int).
|
||||
userdata: User data to be added for the first column.
|
||||
|
||||
Return:
|
||||
A (nameitem, descitem, miscitem) tuple.
|
||||
@ -98,11 +85,6 @@ class CompletionModel(QStandardItemModel):
|
||||
miscitem = QStandardItem(misc)
|
||||
|
||||
cat.appendRow([nameitem, descitem, miscitem])
|
||||
if sort is not None:
|
||||
nameitem.setData(sort, Role.sort)
|
||||
if userdata is not None:
|
||||
nameitem.setData(userdata, Role.userdata)
|
||||
return nameitem, descitem, miscitem
|
||||
|
||||
def flags(self, index):
|
||||
"""Return the item flags for index.
|
||||
|
@ -67,7 +67,7 @@ def value(sectname, optname):
|
||||
optname: The name of the config option this model shows.
|
||||
"""
|
||||
model = base.CompletionModel(column_widths=(20, 70, 10))
|
||||
cur_cat = model.new_category("Current/Default", sort=0)
|
||||
cur_cat = model.new_category("Current/Default")
|
||||
try:
|
||||
val = config.get(sectname, optname, raw=True) or '""'
|
||||
except (configexc.NoSectionError, configexc.NoOptionError):
|
||||
@ -85,7 +85,7 @@ def value(sectname, optname):
|
||||
# Different type for each value (KeyValue)
|
||||
vals = configdata.DATA[sectname][optname].typ.complete()
|
||||
if vals is not None:
|
||||
cat = model.new_category("Completions", sort=1)
|
||||
cat = model.new_category("Completions")
|
||||
for (val, desc) in vals:
|
||||
model.new_item(cat, val, desc)
|
||||
return model
|
||||
|
@ -119,7 +119,6 @@ def buffer():
|
||||
|
||||
model = base.CompletionModel(
|
||||
column_widths=(6, 40, 54),
|
||||
dumb_sort=Qt.DescendingOrder,
|
||||
delete_cur_item=delete_buffer,
|
||||
columns_to_filter=[idx_column, url_column, text_column])
|
||||
|
||||
|
@ -33,14 +33,13 @@ from qutebrowser.completion.models import base as completion
|
||||
|
||||
class CompletionFilterModel(QSortFilterProxyModel):
|
||||
|
||||
"""Subclass of QSortFilterProxyModel with custom sorting/filtering.
|
||||
"""Subclass of QSortFilterProxyModel with custom filtering.
|
||||
|
||||
Attributes:
|
||||
pattern: The pattern to filter with.
|
||||
srcmodel: The current source model.
|
||||
Kept as attribute because calling `sourceModel` takes quite
|
||||
a long time for some reason.
|
||||
_sort_order: The order to use for sorting if using dumb_sort.
|
||||
"""
|
||||
|
||||
def __init__(self, source, parent=None):
|
||||
@ -49,21 +48,12 @@ class CompletionFilterModel(QSortFilterProxyModel):
|
||||
self.srcmodel = source
|
||||
self.pattern = ''
|
||||
self.pattern_re = None
|
||||
|
||||
dumb_sort = self.srcmodel.dumb_sort
|
||||
if dumb_sort is None:
|
||||
# pylint: disable=invalid-name
|
||||
self.lessThan = self.intelligentLessThan
|
||||
self._sort_order = Qt.AscendingOrder
|
||||
else:
|
||||
self.setSortRole(completion.Role.sort)
|
||||
self._sort_order = dumb_sort
|
||||
self.lessThan = self.intelligentLessThan
|
||||
#self._sort_order = self.srcmodel.sort_order or Qt.AscendingOrder
|
||||
|
||||
def set_pattern(self, val):
|
||||
"""Setter for pattern.
|
||||
|
||||
Invalidates the filter and re-sorts the model.
|
||||
|
||||
Args:
|
||||
val: The value to set.
|
||||
"""
|
||||
@ -163,12 +153,6 @@ class CompletionFilterModel(QSortFilterProxyModel):
|
||||
qtutils.ensure_valid(lindex)
|
||||
qtutils.ensure_valid(rindex)
|
||||
|
||||
left_sort = self.srcmodel.data(lindex, role=completion.Role.sort)
|
||||
right_sort = self.srcmodel.data(rindex, role=completion.Role.sort)
|
||||
|
||||
if left_sort is not None and right_sort is not None:
|
||||
return left_sort < right_sort
|
||||
|
||||
left = self.srcmodel.data(lindex)
|
||||
right = self.srcmodel.data(rindex)
|
||||
|
||||
@ -183,9 +167,3 @@ class CompletionFilterModel(QSortFilterProxyModel):
|
||||
return False
|
||||
else:
|
||||
return left < right
|
||||
|
||||
def sort(self, column, order=None):
|
||||
"""Extend sort to respect self._sort_order if no order was given."""
|
||||
if order is None:
|
||||
order = self._sort_order
|
||||
super().sort(column, order)
|
||||
|
@ -37,7 +37,6 @@ class FakeCompletionModel(QStandardItemModel):
|
||||
super().__init__(parent)
|
||||
self.kind = kind
|
||||
self.pos_args = [*pos_args]
|
||||
self.dumb_sort = None
|
||||
|
||||
|
||||
class CompletionWidgetStub(QObject):
|
||||
|
@ -151,80 +151,46 @@ def test_count(tree, expected):
|
||||
assert filter_model.count() == expected
|
||||
|
||||
|
||||
@pytest.mark.parametrize('pattern, dumb_sort, filter_cols, before, after', [
|
||||
('foo', None, [0],
|
||||
@pytest.mark.parametrize('pattern, filter_cols, before, after', [
|
||||
('foo', [0],
|
||||
[[('foo', '', ''), ('bar', '', '')]],
|
||||
[[('foo', '', '')]]),
|
||||
|
||||
('foo', None, [0],
|
||||
('foo', [0],
|
||||
[[('foob', '', ''), ('fooc', '', ''), ('fooa', '', '')]],
|
||||
[[('fooa', '', ''), ('foob', '', ''), ('fooc', '', '')]]),
|
||||
|
||||
('foo', None, [0],
|
||||
('foo', [0],
|
||||
[[('foo', '', '')], [('bar', '', '')]],
|
||||
[[('foo', '', '')], []]),
|
||||
|
||||
# prefer foobar as it starts with the pattern
|
||||
('foo', None, [0],
|
||||
('foo', [0],
|
||||
[[('barfoo', '', ''), ('foobar', '', '')]],
|
||||
[[('foobar', '', ''), ('barfoo', '', '')]]),
|
||||
|
||||
# however, don't rearrange categories
|
||||
('foo', None, [0],
|
||||
('foo', [0],
|
||||
[[('barfoo', '', '')], [('foobar', '', '')]],
|
||||
[[('barfoo', '', '')], [('foobar', '', '')]]),
|
||||
|
||||
('foo', None, [1],
|
||||
('foo', [1],
|
||||
[[('foo', 'bar', ''), ('bar', 'foo', '')]],
|
||||
[[('bar', 'foo', '')]]),
|
||||
|
||||
('foo', None, [0, 1],
|
||||
('foo', [0, 1],
|
||||
[[('foo', 'bar', ''), ('bar', 'foo', ''), ('bar', 'bar', '')]],
|
||||
[[('foo', 'bar', ''), ('bar', 'foo', '')]]),
|
||||
|
||||
('foo', None, [0, 1, 2],
|
||||
('foo', [0, 1, 2],
|
||||
[[('foo', '', ''), ('bar', '')]],
|
||||
[[('foo', '', '')]]),
|
||||
|
||||
# the fourth column is the sort role, which overrides data-based sorting
|
||||
('', None, [0],
|
||||
[[('two', '', '', 2), ('one', '', '', 1), ('three', '', '', 3)]],
|
||||
[[('one', '', ''), ('two', '', ''), ('three', '', '')]]),
|
||||
|
||||
('', Qt.AscendingOrder, [0],
|
||||
[[('two', '', '', 2), ('one', '', '', 1), ('three', '', '', 3)]],
|
||||
[[('one', '', ''), ('two', '', ''), ('three', '', '')]]),
|
||||
|
||||
('', Qt.DescendingOrder, [0],
|
||||
[[('two', '', '', 2), ('one', '', '', 1), ('three', '', '', 3)]],
|
||||
[[('three', '', ''), ('two', '', ''), ('one', '', '')]]),
|
||||
])
|
||||
def test_set_pattern(pattern, dumb_sort, filter_cols, before, after):
|
||||
def test_set_pattern(pattern, filter_cols, before, after):
|
||||
"""Validate the filtering and sorting results of set_pattern."""
|
||||
model = _create_model(before)
|
||||
model.dumb_sort = dumb_sort
|
||||
model.columns_to_filter = filter_cols
|
||||
filter_model = sortfilter.CompletionFilterModel(model)
|
||||
filter_model.set_pattern(pattern)
|
||||
actual = _extract_model_data(filter_model)
|
||||
assert actual == after
|
||||
|
||||
|
||||
def test_sort():
|
||||
"""Ensure that a sort argument passed to sort overrides DUMB_SORT.
|
||||
|
||||
While test_set_pattern above covers most of the sorting logic, this
|
||||
particular case is easier to test separately.
|
||||
"""
|
||||
model = _create_model([[('B', '', '', 1),
|
||||
('C', '', '', 2),
|
||||
('A', '', '', 0)]])
|
||||
filter_model = sortfilter.CompletionFilterModel(model)
|
||||
|
||||
filter_model.sort(0, Qt.AscendingOrder)
|
||||
actual = _extract_model_data(filter_model)
|
||||
assert actual == [[('A', '', ''), ('B', '', ''), ('C', '', '')]]
|
||||
|
||||
filter_model.sort(0, Qt.DescendingOrder)
|
||||
actual = _extract_model_data(filter_model)
|
||||
assert actual == [[('C', '', ''), ('B', '', ''), ('A', '', '')]]
|
||||
|
Loading…
Reference in New Issue
Block a user