diff --git a/qutebrowser/completion/models/completionmodel.py b/qutebrowser/completion/models/completionmodel.py index 3e48076e5..398673200 100644 --- a/qutebrowser/completion/models/completionmodel.py +++ b/qutebrowser/completion/models/completionmodel.py @@ -22,6 +22,7 @@ from PyQt5.QtCore import Qt, QModelIndex, QAbstractItemModel from qutebrowser.utils import log, qtutils +from qutebrowser.commands import cmdexc class CompletionModel(QAbstractItemModel): @@ -219,6 +220,13 @@ class CompletionModel(QAbstractItemModel): parent = index.parent() cat = self._cat_from_idx(parent) assert cat, "CompletionView sent invalid index for deletion" + if not cat.delete_func: + raise cmdexc.CommandError("Cannot delete this item.") + + data = [cat.data(cat.index(index.row(), i)) + for i in range(cat.columnCount())] + cat.delete_func(data) + self.beginRemoveRows(parent, index.row(), index.row()) - cat.delete_cur_item(cat.index(index.row(), 0)) + cat.removeRow(index.row(), QModelIndex()) self.endRemoveRows() diff --git a/qutebrowser/completion/models/histcategory.py b/qutebrowser/completion/models/histcategory.py index 612eb0bf4..56b7c3819 100644 --- a/qutebrowser/completion/models/histcategory.py +++ b/qutebrowser/completion/models/histcategory.py @@ -90,13 +90,8 @@ class HistoryCategory(QSqlQueryModel): self._query.run(pat=pattern) self.setQuery(self._query) - def delete_cur_item(self, index): - """Delete the row at the given index.""" - if not self.delete_func: - raise cmdexc.CommandError("Cannot delete this item.") - data = [self.data(index.sibling(index.row(), i)) - for i in range(self.columnCount())] - self.delete_func(data) + def removeRow(self, _row, _col): + """Re-run sql query to respond to a row removal.""" # re-run query to reload updated table with debug.log_time('sql', 'Re-running completion query post-delete'): self._query.run() diff --git a/qutebrowser/completion/models/listcategory.py b/qutebrowser/completion/models/listcategory.py index 187ebcad6..5f537d4d7 100644 --- a/qutebrowser/completion/models/listcategory.py +++ b/qutebrowser/completion/models/listcategory.py @@ -91,12 +91,3 @@ class ListCategory(QSortFilterProxyModel): return False else: return left < right - - def delete_cur_item(self, index): - """Delete the row at the given index.""" - if not self.delete_func: - raise cmdexc.CommandError("Cannot delete this item.") - data = [self.data(index.sibling(index.row(), i)) - for i in range(self.columnCount())] - self.delete_func(data) - self.removeRow(index.row(), QModelIndex()) diff --git a/tests/unit/completion/test_completionmodel.py b/tests/unit/completion/test_completionmodel.py index 4d1d3f123..9e73e533a 100644 --- a/tests/unit/completion/test_completionmodel.py +++ b/tests/unit/completion/test_completionmodel.py @@ -28,6 +28,7 @@ from PyQt5.QtCore import QModelIndex from qutebrowser.completion.models import completionmodel, listcategory from qutebrowser.utils import qtutils +from qutebrowser.commands import cmdexc @hypothesis.given(strategies.lists(min_size=0, max_size=3, @@ -92,8 +93,25 @@ def test_delete_cur_item(): func.assert_called_once_with(['foo', 'bar']) +def test_delete_cur_item_no_func(): + callback = mock.Mock(spec=[]) + model = completionmodel.CompletionModel() + cat = listcategory.ListCategory('', [('foo', 'bar')], delete_func=None) + model.rowsAboutToBeRemoved.connect(callback) + model.rowsRemoved.connect(callback) + model.add_category(cat) + parent = model.index(0, 0) + with pytest.raises(cmdexc.CommandError): + model.delete_cur_item(model.index(0, 0, parent)) + assert not callback.called + + def test_delete_cur_item_no_cat(): """Test completion_item_del with no selected category.""" + callback = mock.Mock(spec=[]) model = completionmodel.CompletionModel() + model.rowsAboutToBeRemoved.connect(callback) + model.rowsRemoved.connect(callback) with pytest.raises(qtutils.QtValueError): model.delete_cur_item(QModelIndex()) + assert not callback.called diff --git a/tests/unit/completion/test_histcategory.py b/tests/unit/completion/test_histcategory.py index c53c86ee1..7044f1281 100644 --- a/tests/unit/completion/test_histcategory.py +++ b/tests/unit/completion/test_histcategory.py @@ -23,6 +23,7 @@ import unittest.mock import datetime import pytest +from PyQt5.QtCore import QModelIndex from qutebrowser.misc import sql from qutebrowser.completion.models import histcategory @@ -140,20 +141,13 @@ def test_sorting(max_items, before, after, model_validator, hist, config_stub): model_validator.validate(after) -def test_delete_cur_item(hist): +def test_remove_row(hist, model_validator): hist.insert({'url': 'foo', 'title': 'Foo'}) hist.insert({'url': 'bar', 'title': 'Bar'}) - func = unittest.mock.Mock(spec=[]) - cat = histcategory.HistoryCategory(delete_func=func) - cat.set_pattern('') - cat.delete_cur_item(cat.index(0, 0)) - func.assert_called_with(['foo', 'Foo', '']) - - -def test_delete_cur_item_no_func(hist): - hist.insert({'url': 'foo', 'title': 1}) - hist.insert({'url': 'bar', 'title': 2}) cat = histcategory.HistoryCategory() + model_validator.set_model(cat) cat.set_pattern('') - with pytest.raises(cmdexc.CommandError, match='Cannot delete this item'): - cat.delete_cur_item(cat.index(0, 0)) + hist.delete('url', 'foo') + # histcategory does not care which index was removed, it just regenerates + cat.removeRow(QModelIndex(), QModelIndex()) + model_validator.validate([('bar', 'Bar', '')]) diff --git a/tests/unit/completion/test_listcategory.py b/tests/unit/completion/test_listcategory.py index 3b1c1478a..df1d1de5f 100644 --- a/tests/unit/completion/test_listcategory.py +++ b/tests/unit/completion/test_listcategory.py @@ -51,23 +51,3 @@ def test_set_pattern(pattern, before, after, model_validator): model_validator.set_model(cat) cat.set_pattern(pattern) model_validator.validate(after) - - -def test_delete_cur_item(model_validator): - func = mock.Mock(spec=[]) - cat = listcategory.ListCategory('Foo', [('a', 'b'), ('c', 'd')], - delete_func=func) - model_validator.set_model(cat) - idx = cat.index(0, 0) - cat.delete_cur_item(idx) - func.assert_called_once_with(['a', 'b']) - model_validator.validate([('c', 'd')]) - - -def test_delete_cur_item_no_func(model_validator): - cat = listcategory.ListCategory('Foo', [('a', 'b'), ('c', 'd')]) - model_validator.set_model(cat) - idx = cat.index(0, 0) - with pytest.raises(cmdexc.CommandError, match="Cannot delete this item."): - cat.delete_cur_item(idx) - model_validator.validate([('a', 'b'), ('c', 'd')])