Eric Blake <ebl...@redhat.com> writes: > Silence pep8, and make pylint a bit happier. Just style cleanups; > no semantic changes.
I had planned to clean it up after resolving the TODO fold into QAPISchema, but I'm fine with doing it right away. I think we'll want to do a bit more for pylint, but limiting ourselves to really obvious changes now makes sense. > Signed-off-by: Eric Blake <ebl...@redhat.com> > --- > scripts/qapi.py | 165 > ++++++++++++++++++++++++++++++++++++-------------------- > 1 file changed, 107 insertions(+), 58 deletions(-) > > diff --git a/scripts/qapi.py b/scripts/qapi.py > index 06478bb..31c4bcc 100644 > --- a/scripts/qapi.py > +++ b/scripts/qapi.py > @@ -71,6 +71,7 @@ all_names = {} > # Parsing the schema into expressions > # > > + > def error_path(parent): > res = "" > while parent: > @@ -79,8 +80,10 @@ def error_path(parent): > parent = parent['parent'] > return res > > + > class QAPISchemaError(Exception): > def __init__(self, schema, msg): > + Exception.__init__(self) Not a style change. One more below. Separate patch? > self.fname = schema.fname > self.msg = msg > self.col = 1 > @@ -96,8 +99,10 @@ class QAPISchemaError(Exception): > return error_path(self.info) + \ > "%s:%d:%d: %s" % (self.fname, self.line, self.col, self.msg) > > + > class QAPIExprError(Exception): > def __init__(self, expr_info, msg): > + Exception.__init__(self) > self.info = expr_info > self.msg = msg > > @@ -105,9 +110,10 @@ class QAPIExprError(Exception): > return error_path(self.info['parent']) + \ > "%s:%d: %s" % (self.info['file'], self.info['line'], self.msg) > > + > class QAPISchemaParser(object): > > - def __init__(self, fp, previously_included = [], incl_info = None): > + def __init__(self, fp, previously_included=[], incl_info=None): > abs_fname = os.path.abspath(fp.name) > fname = fp.name > self.fname = fname > @@ -122,18 +128,18 @@ class QAPISchemaParser(object): > self.exprs = [] > self.accept() > > - while self.tok != None: > + while self.tok is not None: > expr_info = {'file': fname, 'line': self.line, > 'parent': self.incl_info} > expr = self.get_expr(False) > if isinstance(expr, dict) and "include" in expr: > if len(expr) != 1: > - raise QAPIExprError(expr_info, "Invalid 'include' > directive") > + raise QAPIExprError(expr_info, > + "Invalid 'include' directive") > include = expr["include"] > if not isinstance(include, str): > - raise QAPIExprError(expr_info, > - 'Expected a file name (string), got: > %s' > - % include) > + raise QAPIExprError(expr_info, 'Expected a file name ' > + '(string), got: %s' % include) I don't like breaking lines in the middle of an argument when another argument shares a line with a part. Perhaps: raise QAPIExprError(expr_info, 'Expected a file name (string), got: %s' % include) > incl_abs_fname = os.path.join(os.path.dirname(abs_fname), > include) > # catch inclusion cycle [...] > @@ -1292,6 +1323,7 @@ def camel_case(name): > new_name += ch.lower() > return new_name > > + > # ENUMName -> ENUM_NAME, EnumName1 -> ENUM_NAME1 > # ENUM_NAME -> ENUM_NAME, ENUM_NAME1 -> ENUM_NAME1, ENUM_Name2 -> ENUM_NAME2 > # ENUM24_Name -> ENUM24_NAME > @@ -1308,12 +1340,13 @@ def camel_to_upper(value): > if c.isupper() and (i > 0) and c_fun_str[i - 1] != "_": > # Case 1: next string is lower > # Case 2: previous string is digit > - if (i < (l - 1) and c_fun_str[i + 1].islower()) or \ > - c_fun_str[i - 1].isdigit(): > + next_lower = i < (l - 1) and c_fun_str[i + 1].islower() > + if next_lower or c_fun_str[i - 1].isdigit(): > new_name += '_' > new_name += c Dunno. Perhaps: if i < (l - 1) and c_fun_str[i + 1].islower(): new_name += '_' elif c_fun_str[i - 1].isdigit(): new_name += '_' Differently ugly, I guess. The two comment lines are pretty worthless. > return new_name.lstrip('_').upper() > > + > def c_enum_const(type_name, const_name, prefix=None): > if prefix is not None: > type_name = prefix [...]