On 2/17/21 4:39 AM, Markus Armbruster wrote:
John Snow <js...@redhat.com> writes:Signed-off-by: John Snow <js...@redhat.com> --- scripts/qapi/introspect.py | 18 ++++++++++++++++++ 1 file changed, 18 insertions(+) diff --git a/scripts/qapi/introspect.py b/scripts/qapi/introspect.py index 45284af1330..5d4f5e23f7e 100644 --- a/scripts/qapi/introspect.py +++ b/scripts/qapi/introspect.py @@ -99,6 +99,15 @@ def __init__(self, value: _ValueT, ifcond: Iterable[str], def _tree_to_qlit(obj: JSONValue, level: int = 0, dict_value: bool = False) -> str: + """ + Convert the type tree into a QLIT C string, recursively. + + :param obj: The value to convert. + This value may not be Annotated when dict_value is True. + :param level: The indentation level for this particular value. + :param dict_value: True when the value being processed belongs to a + dict key; which suppresses the output indent. + """def indent(level: int) -> str:return level * 4 * ' ' @@ -244,6 +253,15 @@ def _gen_features(features: List[QAPISchemaFeature] def _gen_tree(self, name: str, mtype: str, obj: Dict[str, object], ifcond: Sequence[str], features: Optional[List[QAPISchemaFeature]]) -> None: + """ + Build and append a SchemaInfo object to self._trees. + + :param name: The entity's name. + :param mtype: The entity's meta-type. + :param obj: Additional entity fields, as appropriate for the meta-type."Additional members", since we're talking about a JSON object.
I thought "field" was also appropriate for JSON, but I suppose the spec doesn't use that word. Over time, "field", "member" and "property" have become just meaningless word-slurry to me.
OK. "Additional members as appropriate for the meta-type."
+ :param ifcond: Sequence of conditionals that apply to this entity. + :param features: Optional features field for SchemaInfo.Likewise.
"Optional features member for SchemaInfo" ? Sure.
Sure we want to restate parts of the type ("Sequence of", "Optional") in
the doc string?
I usually avoid it, but sometimes for non-scalars I found that it read better to give a nod to the plural, as in:
[ifcond is a] sequence of conditionals ...but, yes, I haven't been consistent about it. right below for @obj I omit the type of the container.
"Conditionals that apply to this entity" feels almost too terse in isolation.
I don't feel like it's a requisite to state the type, but in some cases I unconsciously chose to mention the structure.
With regards to "Optional", I use this word specifically to indicate parameters that have default values -- distinct from a type that's Optional[], which is really actually like Nullable[T] ... If it makes you feel better, Guido says he regrets that naming decision. Oops!
I'm probably not consistent about when I decided to write it, though.Ehm. If it's not harmful to leave it as-is, I think it'd be okay to do so. If you prefer a consistency all one way or all the other, I'd have to run the vacuum back over the series to check for it.
+ """ comment: Optional[str] = None if mtype not in ('command', 'event', 'builtin', 'array'): if not self._unmask:Also: more line-wrapping for PEP 8.
I thought the 72 column limit was for things like comments and docstrings.
