Signed-off-by: John Snow <jsnow@redhat.com>
---
scripts/qapi/.flake8 | 1 +
scripts/qapi/common.py | 8 +++++---
scripts/qapi/events.py | 9 +++++----
scripts/qapi/gen.py | 8 ++++----
scripts/qapi/introspect.py | 8 +++++---
scripts/qapi/main.py | 4 ++--
scripts/qapi/parser.py | 15 ++++++++-------
scripts/qapi/schema.py | 23 +++++++++++++----------
scripts/qapi/types.py | 7 ++++---
9 files changed, 47 insertions(+), 36 deletions(-)
diff --git a/scripts/qapi/.flake8 b/scripts/qapi/.flake8
index 6b158c68b8..4f00455290 100644
--- a/scripts/qapi/.flake8
+++ b/scripts/qapi/.flake8
@@ -1,2 +1,3 @@
[flake8]
extend-ignore = E722 # Prefer pylint's bare-except checks to flake8's
+max-doc-length = 72
\ No newline at end of file
Since we intend to make use of PEP 8's license to go over the line
length limit, having the build gripe about it is not useful. Drop.
diff --git a/scripts/qapi/common.py b/scripts/qapi/common.py
index cbd3fd81d3..6e3d9b8ecd 100644
--- a/scripts/qapi/common.py
+++ b/scripts/qapi/common.py
@@ -41,7 +41,8 @@ def camel_to_upper(value: str) -> str:
length = len(c_fun_str)
for i in range(length):
char = c_fun_str[i]
- # When char is upper case and no '_' appears before, do more checks
+ # When char is upper case and no '_' appears before,
+ # do more checks
if char.isupper() and (i > 0) and c_fun_str[i - 1] != '_':
if i < length - 1 and c_fun_str[i + 1].islower():
new_name += '_'
The comment paraphrases the if condition. Feels useless. Let's drop
it.
@@ -78,8 +79,9 @@ def c_name(name: str, protect: bool = True) -> str:
protect=True: 'int' -> 'q_int'; protect=False: 'int' -> 'int'
:param name: The name to map.
- :param protect: If true, avoid returning certain ticklish identifiers
- (like C keywords) by prepending ``q_``.
+ :param protect: If true, avoid returning certain ticklish
+ identifiers (like C keywords) by prepending
+ ``q_``.
Better:
:param protect: If true, avoid returning certain ticklish
identifiers (like C keywords) by prepending ``q_``.
For what it's worth, this indentation style is also used in the
Sphinx-RTD-Tutorial[*]. I like it much better than aligning the text
like you did, because that wastes screen real estate when the parameter
names are long, and tempts people to aligning all the parameters, like
:param name: The name to map.
:param protect: If true, avoid returning certain ticklish identifiers
(like C keywords) by prepending ``q_``.
which leads to either churn or inconsistency when parameters with longer
names get added.
"""
# ANSI X3J11/88-090, 3.1.1
c89_words = set(['auto', 'break', 'case', 'char', 'const', 'continue',
diff --git a/scripts/qapi/events.py b/scripts/qapi/events.py
index fee8c671e7..210b56974f 100644
--- a/scripts/qapi/events.py
+++ b/scripts/qapi/events.py
@@ -48,7 +48,8 @@ def gen_param_var(typ: QAPISchemaObjectType) -> str:
"""
Generate a struct variable holding the event parameters.
- Initialize it with the function arguments defined in `gen_event_send`.
+ Initialize it with the function arguments defined in
+ `gen_event_send`.
"""
assert not typ.variants
ret = mcgen('''
Looks like a wash. I figure the doc string will be rewritten to Sphinx
format (or whatever other format we adopt for our Python code) anyway,
so let's not mess with it now.
@@ -86,9 +87,9 @@ def gen_event_send(name: str,
# FIXME: Our declaration of local variables (and of 'errp' in the
# parameter list) can collide with exploded members of the event's
# data type passed in as parameters. If this collision ever hits in
- # practice, we can rename our local variables with a leading _ prefix,
- # or split the code into a wrapper function that creates a boxed
- # 'param' object then calls another to do the real work.
+ # practice, we can rename our local variables with a leading _
+ # prefix, or split the code into a wrapper function that creates a
+ # boxed 'param' object then calls another to do the real work.
have_args = boxed or (arg_type and not arg_type.is_empty())
ret = mcgen('''
Improvement.
diff --git a/scripts/qapi/gen.py b/scripts/qapi/gen.py
index 1fa503bdbd..c54980074e 100644
--- a/scripts/qapi/gen.py
+++ b/scripts/qapi/gen.py
@@ -63,9 +63,9 @@ def _bottom(self) -> str:
return ''
def write(self, output_dir: str) -> None:
- # Include paths starting with ../ are used to reuse modules of the main
- # schema in specialised schemas. Don't overwrite the files that are
- # already generated for the main schema.
+ # Include paths starting with ../ are used to reuse modules
+ # of the main schema in specialised schemas. Don't overwrite
+ # the files that are already generated for the main schema.
if self.fname.startswith('../'):
return
pathname = os.path.join(output_dir, self.fname)
Improvement, but mind PEP 8's "You should use two spaces after a
sentence-ending period in multi-sentence comments".
@@ -189,7 +189,7 @@ def _bottom(self) -> str:
@contextmanager
def ifcontext(ifcond: Sequence[str], *args: QAPIGenCCode) -> Iterator[None]:
"""
- A with-statement context manager that wraps with `start_if()` / `end_if()`.
+ A context manager that wraps output with `start_if()` / `end_if()`.
:param ifcond: A sequence of conditionals, passed to `start_if()`.
:param args: any number of `QAPIGenCCode`.
Improvement.
diff --git a/scripts/qapi/introspect.py b/scripts/qapi/introspect.py
index 9a348ca2e5..faf00013ad 100644
--- a/scripts/qapi/introspect.py
+++ b/scripts/qapi/introspect.py
@@ -61,8 +61,9 @@
# With optional annotations, the type of all values is:
# JSONValue = Union[_Value, Annotated[_Value]]
#
-# Sadly, mypy does not support recursive types; so the _Stub alias is used to
-# mark the imprecision in the type model where we'd otherwise use JSONValue.
+# Sadly, mypy does not support recursive types; so the _Stub alias is
+# used to mark the imprecision in the type model where we'd otherwise
+# use JSONValue.
_Stub = Any
_Scalar = Union[str, bool, None]
_NonScalar = Union[Dict[str, _Stub], List[_Stub]]
Improvement.
@@ -217,7 +218,8 @@ def visit_end(self) -> None:
self._name_map = {}
def visit_needed(self, entity: QAPISchemaEntity) -> bool:
- # Ignore types on first pass; visit_end() will pick up used types
+ # Ignore types on first pass;
+ # visit_end() will pick up used types
Looks a bit odd. Since the original is only slightly over the limit, we
can keep it. Alternatively.
# Ignore types on first pass; visit_end() will pick up the
# types that are actually used
return not isinstance(entity, QAPISchemaType)
def _name(self, name: str) -> str:
diff --git a/scripts/qapi/main.py b/scripts/qapi/main.py
index 703e7ed1ed..5bcac83985 100644
--- a/scripts/qapi/main.py
+++ b/scripts/qapi/main.py
@@ -1,5 +1,5 @@
-# This work is licensed under the terms of the GNU GPL, version 2 or later.
-# See the COPYING file in the top-level directory.
+# This work is licensed under the terms of the GNU GPL, version 2 or
+# later. See the COPYING file in the top-level directory.
Let's drop this one. The line is only slightly too long, and
consistency with the copright notices elsewhere is more important.
"""
QAPI Generator
diff --git a/scripts/qapi/parser.py b/scripts/qapi/parser.py
index 58267c3db9..d5bf91f2b0 100644
--- a/scripts/qapi/parser.py
+++ b/scripts/qapi/parser.py
@@ -331,8 +331,8 @@ def __init__(self, parser, name=None, indent=0):
self._indent = indent
def append(self, line):
- # Strip leading spaces corresponding to the expected indent level
- # Blank lines are always OK.
+ # Strip leading spaces corresponding to the expected indent
+ # level. Blank lines are always OK.
if line:
indent = re.match(r'\s*', line).end()
if indent < self._indent:
Improvement, but mind PEP 8's "You should use two spaces after a
sentence-ending period".
@@ -353,10 +353,10 @@ def connect(self, member):
self.member = member
def __init__(self, parser, info):
- # self._parser is used to report errors with QAPIParseError. The
- # resulting error position depends on the state of the parser.
- # It happens to be the beginning of the comment. More or less
- # servicable, but action at a distance.
+ # self._parser is used to report errors with QAPIParseError.
+ # The resulting error position depends on the state of the
+ # parser. It happens to be the beginning of the comment. More
+ # or less servicable, but action at a distance.
self._parser = parser
self.info = info
self.symbol = None
Why not. Two spaces again.
@@ -430,7 +430,8 @@ def _append_body_line(self, line):
if not line.endswith(':'):
raise QAPIParseError(self._parser, "line should end with ':'")
self.symbol = line[1:-1]
- # FIXME invalid names other than the empty string aren't flagged
+ # FIXME invalid names other than the empty string aren't
+ # flagged
if not self.symbol:
raise QAPIParseError(self._parser, "invalid name")
elif self.symbol:
Not an improvement, drop the hunk.
diff --git a/scripts/qapi/schema.py b/scripts/qapi/schema.py
index 703b446fd2..01cdd753cd 100644
--- a/scripts/qapi/schema.py
+++ b/scripts/qapi/schema.py
@@ -166,9 +166,10 @@ def is_user_module(cls, name: str) -> bool:
@classmethod
def is_builtin_module(cls, name: str) -> bool:
"""
- The built-in module is a single System module for the built-in types.
+ Return true when given the built-in module name.
- It is always "./builtin".
+ The built-in module is a specific System module for the built-in
+ types. It is always "./builtin".
"""
return name == cls.BUILTIN_MODULE_NAME
I figure the doc string will be rewritten to Sphinx format anyway, so
let's not mess with it now.
@@ -294,7 +295,8 @@ def connect_doc(self, doc=None):
m.connect_doc(doc)
def is_implicit(self):
- # See QAPISchema._make_implicit_enum_type() and ._def_predefineds()
+ # See QAPISchema._make_implicit_enum_type() and
+ # ._def_predefineds()
return self.name.endswith('Kind') or self.name == 'QType'
def c_type(self):
Not an improvement, drop the hunk.
@@ -421,9 +423,9 @@ def check(self, schema):
self.members = members # mark completed
- # Check that the members of this type do not cause duplicate JSON members,
- # and update seen to track the members seen so far. Report any errors
- # on behalf of info, which is not necessarily self.info
+ # Check that the members of this type do not cause duplicate JSON
+ # members, and update seen to track the members seen so far. Report
+ # any errors on behalf of info, which is not necessarily self.info
def check_clash(self, info, seen):
assert self._checked
assert not self.variants # not implemented
Improvement. Two spaces again.
@@ -494,11 +496,12 @@ def __init__(self, name, info, doc, ifcond, features,
variants):
def check(self, schema):
super().check(schema)
self.variants.tag_member.check(schema)
- # Not calling self.variants.check_clash(), because there's nothing
- # to clash with
+ # Not calling self.variants.check_clash(), because there's
+ # nothing to clash with
self.variants.check(schema, {})
- # Alternate branch names have no relation to the tag enum values;
- # so we have to check for potential name collisions ourselves.
+ # Alternate branch names have no relation to the tag enum
+ # values; so we have to check for potential name collisions
+ # ourselves.
seen = {}
types_seen = {}
for v in self.variants.variants:
Why not.
diff --git a/scripts/qapi/types.py b/scripts/qapi/types.py
index 20d572a23a..2e67ab1752 100644
--- a/scripts/qapi/types.py
+++ b/scripts/qapi/types.py
@@ -35,8 +35,8 @@
from .source import QAPISourceInfo
-# variants must be emitted before their container; track what has already
-# been output
+# variants must be emitted before their container; track what has
+# already been output
objects_seen = set()
Why not.
@@ -297,7 +297,8 @@ def _begin_user_module(self, name: str) -> None:
'''))
def visit_begin(self, schema: QAPISchema) -> None:
- # gen_object() is recursive, ensure it doesn't visit the empty type
+ # gen_object() is recursive, ensure
+ # it doesn't visit the empty type
Looks a bit odd. Since the original is only slightly over the limit, we
can keep it.
Pattern: turning single line comments into multi-line comments to avoid
small length overruns is usually not an improvement.