From d122ab7ce13f537ac52c9ede2ef5ab8332eb809e Mon Sep 17 00:00:00 2001 From: Florian Bruhin Date: Tue, 19 Jan 2021 14:24:53 +0100 Subject: [PATCH] Refactor configtype __init__ methods - Move the "completions" argument (to override completions) to BaseType so any class can use it. - Don't use **kwargs anywhere for better type checking. - Make arguments kw-only where useful. --- qutebrowser/config/configtypes.py | 278 +++++++++++++++++--------- tests/unit/config/test_configtypes.py | 15 +- 2 files changed, 200 insertions(+), 93 deletions(-) diff --git a/qutebrowser/config/configtypes.py b/qutebrowser/config/configtypes.py index 30f02811f..0ca1408de 100644 --- a/qutebrowser/config/configtypes.py +++ b/qutebrowser/config/configtypes.py @@ -148,13 +148,19 @@ class BaseType: Attributes: none_ok: Whether to allow None (or an empty string for :set) as value. + _completions: Override for completions for the given setting. Class attributes: valid_values: Possible values if they can be expressed as a fixed string. ValidValues instance. """ - def __init__(self, none_ok: bool = False) -> None: + def __init__( + self, *, + none_ok: bool = False, + completions: _Completions = None, + ) -> None: + self._completions = completions self.none_ok = none_ok self.valid_values: Optional[ValidValues] = None @@ -313,7 +319,9 @@ class BaseType: Return: A list of (value, description) tuples or None. """ - if self.valid_values is None: + if self._completions is not None: + return self._completions + elif self.valid_values is None: return None return [ (val, self.valid_values.descriptions.get(val, "")) @@ -321,7 +329,7 @@ class BaseType: ] def __repr__(self) -> str: - return utils.get_repr(self, none_ok=self.none_ok) + return utils.get_repr(self, none_ok=self.none_ok, completions=self._completions) class MappingType(BaseType): @@ -334,8 +342,12 @@ class MappingType(BaseType): MAPPING: DictType[str, Tuple[Any, Optional[str]]] = {} - def __init__(self, none_ok: bool = False) -> None: - super().__init__(none_ok) + def __init__( + self, *, + none_ok: bool = False, + completions: _Completions = None, + ) -> None: + super().__init__(none_ok=none_ok, completions=completions) self.valid_values = ValidValues( *[(key, doc) for (key, (_val, doc)) in self.MAPPING.items()]) @@ -368,12 +380,18 @@ class String(BaseType): completions: completions to be used, or None """ - def __init__(self, *, minlen: int = None, maxlen: int = None, - forbidden: str = None, regex: str = None, - encoding: str = None, none_ok: bool = False, - completions: _Completions = None, - valid_values: ValidValues = None) -> None: - super().__init__(none_ok) + def __init__( + self, *, + minlen: int = None, + maxlen: int = None, + forbidden: str = None, + regex: str = None, + encoding: str = None, + none_ok: bool = False, + completions: _Completions = None, + valid_values: ValidValues = None, + ) -> None: + super().__init__(none_ok=none_ok, completions=completions) self.valid_values = valid_values if minlen is not None and minlen < 1: @@ -386,7 +404,6 @@ class String(BaseType): self.minlen = minlen self.maxlen = maxlen self.forbidden = forbidden - self._completions = completions self.encoding = encoding self.regex = regex @@ -434,12 +451,6 @@ class String(BaseType): return value - def complete(self) -> _Completions: - if self._completions is not None: - return self._completions - else: - return super().complete() - def __repr__(self) -> str: return utils.get_repr(self, none_ok=self.none_ok, valid_values=self.valid_values, @@ -477,10 +488,15 @@ class List(BaseType): _show_valtype = True - def __init__(self, valtype: BaseType, - none_ok: bool = False, - length: int = None) -> None: - super().__init__(none_ok) + def __init__( + self, + valtype: BaseType, + *, + length: int = None, + none_ok: bool = False, + completions: _Completions = None, + ) -> None: + super().__init__(none_ok=none_ok, completions=completions) self.valtype = valtype self.length = length @@ -569,12 +585,17 @@ class ListOrValue(BaseType): _show_valtype = True - def __init__(self, valtype: BaseType, *, - none_ok: bool = False, - **kwargs: Any) -> None: - super().__init__(none_ok) + def __init__( + self, + valtype: BaseType, + *, + none_ok: bool = False, + completions: _Completions = None, + **kwargs: Any, + ) -> None: + super().__init__(none_ok=none_ok, completions=completions) assert not isinstance(valtype, (List, ListOrValue)), valtype - self.listtype = List(valtype, none_ok=none_ok, **kwargs) + self.listtype = List(valtype=valtype, none_ok=none_ok, **kwargs) self.valtype = valtype def _val_and_type(self, value: Any) -> Tuple[Any, BaseType]: @@ -643,10 +664,19 @@ class FlagList(List): _show_valtype = False - def __init__(self, none_ok: bool = False, - valid_values: ValidValues = None, - length: int = None) -> None: - super().__init__(valtype=String(), none_ok=none_ok, length=length) + def __init__( + self, *, + none_ok: bool = False, + completions: _Completions = None, + valid_values: ValidValues = None, + length: int = None, + ) -> None: + super().__init__( + valtype=String(), + none_ok=none_ok, + length=length, + completions=completions, + ) self.valtype.valid_values = valid_values def _check_duplicates(self, values: ListType) -> None: @@ -664,6 +694,9 @@ class FlagList(List): return vals def complete(self) -> _Completions: + if self._completions is not None: + return self._completions + valid_values = self.valtype.valid_values if valid_values is None: return None @@ -697,8 +730,12 @@ class Bool(BaseType): while `0`, `no`, `off` and `false` count as false (case-insensitive). """ - def __init__(self, none_ok: bool = False) -> None: - super().__init__(none_ok) + def __init__( + self, *, + none_ok: bool = False, + completions: _Completions = None + ) -> None: + super().__init__(none_ok=none_ok, completions=completions) self.valid_values = ValidValues('true', 'false', generate_docs=False) def to_py(self, value: Union[bool, str, None]) -> Optional[bool]: @@ -729,8 +766,12 @@ class BoolAsk(Bool): """Like `Bool`, but `ask` is allowed as additional value.""" - def __init__(self, none_ok: bool = False) -> None: - super().__init__(none_ok) + def __init__( + self, *, + none_ok: bool = False, + completions: _Completions = None, + ) -> None: + super().__init__(none_ok=none_ok, completions=completions) self.valid_values = ValidValues('true', 'false', 'ask') def to_py(self, # type: ignore[override] @@ -768,11 +809,15 @@ class _Numeric(BaseType): # pylint: disable=abstract-method maxval: Maximum value (inclusive). """ - def __init__(self, minval: int = None, - maxval: int = None, - zero_ok: bool = True, - none_ok: bool = False) -> None: - super().__init__(none_ok) + def __init__( + self, *, + minval: int = None, + maxval: int = None, + zero_ok: bool = True, + none_ok: bool = False, + completions: _Completions = None, + ) -> None: + super().__init__(none_ok=none_ok, completions=completions) self.minval = self._parse_bound(minval) self.maxval = self._parse_bound(maxval) self.zero_ok = zero_ok @@ -912,10 +957,21 @@ class PercOrInt(_Numeric): maxint: Maximum value for integer (inclusive). """ - def __init__(self, minperc: int = None, maxperc: int = None, - minint: int = None, maxint: int = None, - none_ok: bool = False) -> None: - super().__init__(minval=minint, maxval=maxint, none_ok=none_ok) + def __init__( + self, *, + minperc: int = None, + maxperc: int = None, + minint: int = None, + maxint: int = None, + none_ok: bool = False, + completions: _Completions = None, + ) -> None: + super().__init__( + minval=minint, + maxval=maxint, + none_ok=none_ok, + completions=completions, + ) self.minperc = self._parse_bound(minperc) self.maxperc = self._parse_bound(maxperc) if (self.maxperc is not None and self.minperc is not None and @@ -986,6 +1042,9 @@ class Command(BaseType): """ def complete(self) -> _Completions: + if self._completions is not None: + return self._completions + out = [] for cmdname, obj in objects.commands.items(): out.append((cmdname, obj.desc)) @@ -1239,9 +1298,13 @@ class Regex(BaseType): _regex_type: The Python type of a regex object. """ - def __init__(self, flags: str = None, - none_ok: bool = False) -> None: - super().__init__(none_ok) + def __init__( + self, *, + flags: str = None, + none_ok: bool = False, + completions: _Completions = None, + ) -> None: + super().__init__(none_ok=none_ok, completions=completions) self._regex_type = type(re.compile('')) # Parse flags from configdata.yml if flags is None: @@ -1305,12 +1368,16 @@ class Dict(BaseType): When setting from a string, pass a json-like dict, e.g. `{"key", "value"}`. """ - def __init__(self, keytype: Union[String, 'Key'], - valtype: BaseType, *, - fixed_keys: Iterable = None, - required_keys: Iterable = None, - none_ok: bool = False) -> None: - super().__init__(none_ok) + def __init__( + self, *, + keytype: Union[String, 'Key'], + valtype: BaseType, + fixed_keys: Iterable = None, + required_keys: Iterable = None, + none_ok: bool = False, + completions: _Completions = None, + ) -> None: + super().__init__(none_ok=none_ok, completions=completions) # If the keytype is not a string, we'll get problems with showing it as # json in to_str() as json converts keys to strings. assert isinstance(keytype, (String, Key)), keytype @@ -1409,8 +1476,13 @@ class File(BaseType): """A file on the local filesystem.""" - def __init__(self, required: bool = True, **kwargs: Any) -> None: - super().__init__(**kwargs) + def __init__( + self, *, + required: bool = True, + none_ok: bool = False, + completions: _Completions = None, + ) -> None: + super().__init__(none_ok=none_ok, completions=completions) self.required = required def to_py(self, value: _StrUnset) -> _StrUnsetNone: @@ -1475,10 +1547,14 @@ class FormatString(BaseType): completions: completions to be used, or None """ - def __init__(self, fields: Iterable[str], - none_ok: bool = False, - completions: _Completions = None) -> None: - super().__init__(none_ok) + def __init__( + self, *, + fields: Iterable[str], + none_ok: bool = False, + completions: _Completions = None, + ) -> None: + super().__init__( + none_ok=none_ok, completions=completions) self.fields = fields self._completions = completions @@ -1499,12 +1575,6 @@ class FormatString(BaseType): return value - def complete(self) -> _Completions: - if self._completions is not None: - return self._completions - else: - return super().complete() - def __repr__(self) -> str: return utils.get_repr(self, none_ok=self.none_ok, fields=self.fields) @@ -1521,18 +1591,14 @@ class ShellCommand(List): _show_valtype = False - def __init__(self, placeholder: bool = False, - completions: _Completions = None, - none_ok: bool = False) -> None: - super().__init__(valtype=String(), none_ok=none_ok) + def __init__( + self, *, + placeholder: bool = False, + none_ok: bool = False, + completions: _Completions = None, + ) -> None: + super().__init__(valtype=String(), none_ok=none_ok, completions=completions) self.placeholder = placeholder - self._completions = completions - - def complete(self) -> _Completions: - if self._completions is not None: - return self._completions # pragma: no cover - else: - return super().complete() def to_py( self, @@ -1561,8 +1627,12 @@ class Proxy(BaseType): """A proxy URL, or `system`/`none`.""" - def __init__(self, none_ok: bool = False) -> None: - super().__init__(none_ok) + def __init__( + self, *, + none_ok: bool = False, + completions: _Completions = None + ) -> None: + super().__init__(none_ok=none_ok, completions=completions) self.valid_values = ValidValues( ('system', "Use the system wide proxy."), ('none', "Don't use any proxy")) @@ -1594,6 +1664,9 @@ class Proxy(BaseType): raise configexc.ValidationError(value, e) def complete(self) -> _Completions: + if self._completions is not None: + return self._completions + assert self.valid_values is not None out = [] for val in self.valid_values: @@ -1670,11 +1743,18 @@ class Padding(Dict): _show_valtype = False - def __init__(self, none_ok: bool = False) -> None: - super().__init__(keytype=String(), - valtype=Int(minval=0, none_ok=none_ok), - fixed_keys=['top', 'bottom', 'left', 'right'], - none_ok=none_ok) + def __init__( + self, *, + none_ok: bool = False, + completions: _Completions = None, + ) -> None: + super().__init__( + keytype=String(), + valtype=Int(minval=0, none_ok=none_ok), + fixed_keys=['top', 'bottom', 'left', 'right'], + none_ok=none_ok, + completions=completions + ) def to_py( # type: ignore[override] self, @@ -1731,8 +1811,12 @@ class VerticalPosition(String): """The position of the download bar.""" - def __init__(self, none_ok: bool = False) -> None: - super().__init__(none_ok=none_ok) + def __init__( + self, *, + none_ok: bool = False, + completions: _Completions = None, + ) -> None: + super().__init__(none_ok=none_ok, completions=completions) self.valid_values = ValidValues('top', 'bottom') @@ -1798,8 +1882,12 @@ class ConfirmQuit(FlagList): # Values that can be combined with commas combinable_values = ('multiple-tabs', 'downloads') - def __init__(self, none_ok: bool = False) -> None: - super().__init__(none_ok) + def __init__( + self, *, + none_ok: bool = False, + completions: _Completions = None, + ) -> None: + super().__init__(none_ok=none_ok, completions=completions) self.valtype.none_ok = none_ok self.valtype.valid_values = ValidValues( ('always', "Always show a confirmation."), @@ -1835,8 +1923,12 @@ class NewTabPosition(String): """How new tabs are positioned.""" - def __init__(self, none_ok: bool = False) -> None: - super().__init__(none_ok=none_ok) + def __init__( + self, *, + none_ok: bool = False, + completions: _Completions = None, + ) -> None: + super().__init__(none_ok=none_ok, completions=completions) self.valid_values = ValidValues( ('prev', "Before the current tab."), ('next', "After the current tab."), @@ -1848,8 +1940,12 @@ class LogLevel(String): """A logging level.""" - def __init__(self, none_ok: bool = False) -> None: - super().__init__(none_ok=none_ok) + def __init__( + self, *, + none_ok: bool = False, + completions: _Completions = None, + ) -> None: + super().__init__(none_ok=none_ok, completions=completions) self.valid_values = ValidValues(*[level.lower() for level in log.LOG_LEVELS]) diff --git a/tests/unit/config/test_configtypes.py b/tests/unit/config/test_configtypes.py index b1418c73a..729876103 100644 --- a/tests/unit/config/test_configtypes.py +++ b/tests/unit/config/test_configtypes.py @@ -311,6 +311,18 @@ class TestAll: for value, _desc in completions: typ.from_str(value) + def test_custom_completions(self, klass): + """Make sure we can pass custom completions.""" + completions = [('1', 'one'), ('2', 'two')] + typ = klass(completions=completions) + assert typ.complete() == completions + + def test_signature(self, klass): + """Make sure flag arguments are kw-only.""" + sig = inspect.signature(klass) + for name in ['none_ok', 'completions']: + assert sig.parameters[name].kind == inspect.Parameter.KEYWORD_ONLY + class TestBaseType: @@ -534,8 +546,7 @@ class ListSubclass(configtypes.List): elemtype = configtypes.String(none_ok=none_ok_inner) super().__init__(elemtype, none_ok=none_ok_outer, length=length) if set_valid_values: - self.valtype.valid_values = configtypes.ValidValues( - 'foo', 'bar', 'baz') + self.valtype.valid_values = configtypes.ValidValues('foo', 'bar', 'baz') class FlagListSubclass(configtypes.FlagList):