mypy: Attempt to extract base class from completion categories
The methods in `completionmodel.py` dealing with completion categories
were annotated with `QAbstractItemModel`. In mypy's latest 3.11 update
it correctly pointed out that there is code relying on some attributes,
like `name`, being on the categories but `QAbstractItemModel` didn't
implement those attributes.
This commit adds a new base class for completion categories which
defines the extra attributes we expect. It also changes the type hints
to ensure all list categories inherit from it.
There is a couple of downsides to the current implementation:
* It's using multiple inheritance
* the completionmodel code currently expects categories to have all
the methods of `QAbstractItemModel` plus a few other attributes.
Each of the categories inherit from a different Qt model, so we
can't just remove the Qt model from their class definition.
* trying to extract the Qt models to a `widget` class is way too much
work to fit in a dependency update, and I'm not sure it'll be the
right thing to do because the categories are primarily Qt models, so
we would have have to proxy most methods. Perhaps if they added
their extra metadata to a central registry or something
* I tried using a typing.Protocol for BaseCategory but when trying to
make it also inherit from QAbstractItemModel it got grumpy at me
* It doesn't enforce that the attributes are actually set
* it makes mypy happy that they are there, but there is nothing
warning child classes they have forgotten to set them. Mypy does at
least warn about categories that don't inherit from `BaseCategory`
so implementors will hopefully go there an look at it.
* Apparently you can do some stuff with abstract properties, that
might even have type hinting support. But that's a bit much for me
to want to pile in there tonight
At lest the type hints in `completionmodel.py` are more correct now!
This commit is contained in:
parent
98f85cbfcc
commit
aceef82b24
|
|
@ -3,3 +3,21 @@
|
|||
# SPDX-License-Identifier: GPL-3.0-or-later
|
||||
|
||||
"""Models for the command completion."""
|
||||
|
||||
from typing import Sequence, Optional
|
||||
from qutebrowser.completion.models.util import DeleteFuncType
|
||||
from qutebrowser.qt.core import QAbstractItemModel
|
||||
|
||||
|
||||
class BaseCategory(QAbstractItemModel):
|
||||
"""Abstract base class for categories of CompletionModels.
|
||||
|
||||
Extends QAbstractItemModel with a few attributes we expect to be present.
|
||||
|
||||
TODO: actually enforce that child classes set these variables, either via
|
||||
mypy (how) or turning these variables into abstract properties, eg https://stackoverflow.com/a/50381071
|
||||
"""
|
||||
|
||||
name: str
|
||||
columns_to_filter: Sequence[int]
|
||||
delete_func: Optional[DeleteFuncType] = None
|
||||
|
|
|
|||
|
|
@ -10,6 +10,7 @@ from qutebrowser.qt.core import Qt, QModelIndex, QAbstractItemModel, QObject
|
|||
|
||||
from qutebrowser.utils import log, qtutils, utils
|
||||
from qutebrowser.api import cmdutils
|
||||
from qutebrowser.completion.models import BaseCategory
|
||||
|
||||
|
||||
class CompletionModel(QAbstractItemModel):
|
||||
|
|
@ -28,9 +29,9 @@ class CompletionModel(QAbstractItemModel):
|
|||
def __init__(self, *, column_widths=(30, 70, 0), parent=None):
|
||||
super().__init__(parent)
|
||||
self.column_widths = column_widths
|
||||
self._categories: MutableSequence[QAbstractItemModel] = []
|
||||
self._categories: MutableSequence[BaseCategory] = []
|
||||
|
||||
def _cat_from_idx(self, index: QModelIndex):
|
||||
def _cat_from_idx(self, index: QModelIndex) -> Optional[BaseCategory]:
|
||||
"""Return the category pointed to by the given index.
|
||||
|
||||
Args:
|
||||
|
|
@ -44,7 +45,7 @@ class CompletionModel(QAbstractItemModel):
|
|||
return self._categories[index.row()]
|
||||
return None
|
||||
|
||||
def add_category(self, cat):
|
||||
def add_category(self, cat: BaseCategory) -> None:
|
||||
"""Add a completion category to the model."""
|
||||
self._categories.append(cat)
|
||||
|
||||
|
|
|
|||
|
|
@ -18,11 +18,12 @@ from typing import List, Optional, Iterable
|
|||
|
||||
from qutebrowser.qt.core import QAbstractListModel, QModelIndex, QObject, Qt, QUrl
|
||||
|
||||
from qutebrowser.completion.models import BaseCategory
|
||||
from qutebrowser.config import config
|
||||
from qutebrowser.utils import log
|
||||
|
||||
|
||||
class FilePathCategory(QAbstractListModel):
|
||||
class FilePathCategory(QAbstractListModel, BaseCategory):
|
||||
"""Represent filesystem paths matching a pattern."""
|
||||
|
||||
def __init__(self, name: str, parent: QObject = None) -> None:
|
||||
|
|
|
|||
|
|
@ -12,10 +12,10 @@ from qutebrowser.qt.widgets import QWidget
|
|||
from qutebrowser.misc import sql
|
||||
from qutebrowser.utils import debug, message, log
|
||||
from qutebrowser.config import config
|
||||
from qutebrowser.completion.models import util
|
||||
from qutebrowser.completion.models import util, BaseCategory
|
||||
|
||||
|
||||
class HistoryCategory(QSqlQueryModel):
|
||||
class HistoryCategory(QSqlQueryModel, BaseCategory):
|
||||
|
||||
"""A completion category that queries the SQL history store."""
|
||||
|
||||
|
|
|
|||
|
|
@ -11,11 +11,11 @@ from qutebrowser.qt.core import QSortFilterProxyModel, QRegularExpression
|
|||
from qutebrowser.qt.gui import QStandardItem, QStandardItemModel
|
||||
from qutebrowser.qt.widgets import QWidget
|
||||
|
||||
from qutebrowser.completion.models import util
|
||||
from qutebrowser.completion.models import util, BaseCategory
|
||||
from qutebrowser.utils import qtutils, log
|
||||
|
||||
|
||||
class ListCategory(QSortFilterProxyModel):
|
||||
class ListCategory(QSortFilterProxyModel, BaseCategory):
|
||||
|
||||
"""Expose a list of items as a category for the CompletionModel."""
|
||||
|
||||
|
|
|
|||
|
|
@ -6,10 +6,9 @@
|
|||
|
||||
from typing import Dict, Sequence
|
||||
|
||||
from qutebrowser.qt.core import QAbstractItemModel
|
||||
|
||||
from qutebrowser.completion.models import (completionmodel, filepathcategory,
|
||||
listcategory, histcategory)
|
||||
listcategory, histcategory,
|
||||
BaseCategory)
|
||||
from qutebrowser.browser import history
|
||||
from qutebrowser.utils import log, objreg
|
||||
from qutebrowser.config import config
|
||||
|
|
@ -59,7 +58,7 @@ def url(*, info):
|
|||
in sorted(config.val.url.searchengines.items())
|
||||
if k != 'DEFAULT']
|
||||
categories = config.val.completion.open_categories
|
||||
models: Dict[str, QAbstractItemModel] = {}
|
||||
models: Dict[str, BaseCategory] = {}
|
||||
|
||||
if searchengines and 'searchengines' in categories:
|
||||
models['searchengines'] = listcategory.ListCategory(
|
||||
|
|
|
|||
Loading…
Reference in New Issue