John Snow <js...@redhat.com> writes: > This is a minor adjustment that allows the 'required' and 'optional' > keys fields to take a default value of an empty, immutable sequence (the > empty tuple).
Quite marginal, but simple enough for me not to say no to it. > This reveals a quirk of this function, which is that "a + b" is > list-specific behavior. We can accept a wider variety of types if we > avoid that behavior. Using Collection allows us to accept things like > lists, tuples, sets, and so on. > > (Iterable would also have worked, but Iterable also includes things like > generator expressions which are consumed upon iteration, which would > require a rewrite to make sure that each input was only traversed once.) Totally not worth it now :) > Signed-off-by: John Snow <js...@redhat.com> > --- > scripts/qapi/expr.py | 7 ++++--- > 1 file changed, 4 insertions(+), 3 deletions(-) > > diff --git a/scripts/qapi/expr.py b/scripts/qapi/expr.py > index 2b96bec722f..0b841f292d7 100644 > --- a/scripts/qapi/expr.py > +++ b/scripts/qapi/expr.py > @@ -33,6 +33,7 @@ > > import re > from typing import ( > + Collection, > Iterable, > List, > MutableMapping, > @@ -133,8 +134,8 @@ def check_defn_name_str(name: str, info: QAPISourceInfo, > meta: str) -> None: > def check_keys(value: _JSObject, > info: QAPISourceInfo, > source: str, > - required: List[str], > - optional: List[str]) -> None: > + required: Collection[str] = (), > + optional: Collection[str] = ()) -> None: > """ > Ensures an object has a specific set of keys. [Const] > Any particular reason not to squash this part into PATCH 08? Oh, I figure it's because it requires the next hunk, and squashing that one would kill PATCH 08's "This commit *only* adds annotations." What about putting the next hunk before 08, and squash the remainder? > @@ -155,7 +156,7 @@ def pprint(elems: Iterable[str]) -> str: > "%s misses key%s %s" > % (source, 's' if len(missing) > 1 else '', > pprint(missing))) > - allowed = set(required + optional) > + allowed = set(required) | set(optional) > unknown = set(value) - allowed > if unknown: > raise QAPISemError(