| Message ID | 20210511220601.2110055-5-jsnow@redhat.com |
|---|---|
| State | New |
| Headers | show |
| Series | qapi: static typing conversion, pt5a | expand |
John Snow <jsnow@redhat.com> writes: > For the sake of keeping __init__ smaller (and treating it more like a > gallery of what state variables we can expect to see), put the actual > parsing action into a parse method. It remains invoked from the init > method to reduce churn. I'm kind of *shrug* about this. Well short of "no". > > To accomplish this, 'previously_included' becomes the private data > member '_included', and the filename is stashed as _fname. Nitpick: you enclose most identifiers in quotes, but not all. Comments and commit messages often profit from "marking up" identifiers. Especially when the identifiers are also common words. I like to use function(), @variable, .attribute, and .method(). > > Add any missing declarations to the init method, and group them by > function so they can be understood quickly at a glance. > > Signed-off-by: John Snow <jsnow@redhat.com>
On 5/18/21 5:57 AM, Markus Armbruster wrote: > John Snow <jsnow@redhat.com> writes: > >> For the sake of keeping __init__ smaller (and treating it more like a >> gallery of what state variables we can expect to see), put the actual >> parsing action into a parse method. It remains invoked from the init >> method to reduce churn. > > I'm kind of *shrug* about this. Well short of "no". > Understood. Hopefully it's lateral movement at best to you; I don't want to make it distinctly worse in your eyes. >> >> To accomplish this, 'previously_included' becomes the private data >> member '_included', and the filename is stashed as _fname. > > Nitpick: you enclose most identifiers in quotes, but not all. > > Comments and commit messages often profit from "marking up" identifiers. > Especially when the identifiers are also common words. I like to use > function(), @variable, .attribute, and .method(). > Consistency in this regard is ... OK, not my strong suite. It can be hard to remember when to use which systems, and I'm caught between md, rst, kerneldoc, qapidoc, etc. More or less, I am always happy if you just edit little things like this on pull as you see fit, I won't be mad. I'll edit this in my local branch for now. --js >> >> Add any missing declarations to the init method, and group them by >> function so they can be understood quickly at a glance. >> >> Signed-off-by: John Snow <jsnow@redhat.com>
diff --git a/scripts/qapi/parser.py b/scripts/qapi/parser.py index 39dbcc4eacc..d620706fffb 100644 --- a/scripts/qapi/parser.py +++ b/scripts/qapi/parser.py @@ -37,23 +37,39 @@ def __init__(self, parser, msg): class QAPISchemaParser: def __init__(self, fname, previously_included=None, incl_info=None): - previously_included = previously_included or set() - previously_included.add(os.path.abspath(fname)) + self._fname = fname + self._included = previously_included or set() + self._included.add(os.path.abspath(self._fname)) + self.src = '' - # May raise OSError; allow the caller to handle it. - with open(fname, 'r', encoding='utf-8') as fp: - self.src = fp.read() - - if self.src == '' or self.src[-1] != '\n': - self.src += '\n' + # Lexer state (see `accept` for details): + self.info = QAPISourceInfo(self._fname, incl_info) + self.tok = None + self.pos = 0 self.cursor = 0 - self.info = QAPISourceInfo(fname, incl_info) + self.val = None self.line_pos = 0 + + # Parser output: self.exprs = [] self.docs = [] - self.accept() + + # Showtime! + self._parse() + + def _parse(self): cur_doc = None + # May raise OSError; allow the caller to handle it. + with open(self._fname, 'r', encoding='utf-8') as fp: + self.src = fp.read() + if self.src == '' or self.src[-1] != '\n': + self.src += '\n' + + # Prime the lexer: + self.accept() + + # Parse until done: while self.tok is not None: info = self.info if self.tok == '#': @@ -71,12 +87,12 @@ def __init__(self, fname, previously_included=None, incl_info=None): if not isinstance(include, str): raise QAPISemError(info, "value of 'include' must be a string") - incl_fname = os.path.join(os.path.dirname(fname), + incl_fname = os.path.join(os.path.dirname(self._fname), include) self.exprs.append({'expr': {'include': incl_fname}, 'info': info}) exprs_include = self._include(include, info, incl_fname, - previously_included) + self._included) if exprs_include: self.exprs.extend(exprs_include.exprs) self.docs.extend(exprs_include.docs)
For the sake of keeping __init__ smaller (and treating it more like a gallery of what state variables we can expect to see), put the actual parsing action into a parse method. It remains invoked from the init method to reduce churn. To accomplish this, 'previously_included' becomes the private data member '_included', and the filename is stashed as _fname. Add any missing declarations to the init method, and group them by function so they can be understood quickly at a glance. Signed-off-by: John Snow <jsnow@redhat.com> --- scripts/qapi/parser.py | 40 ++++++++++++++++++++++++++++------------ 1 file changed, 28 insertions(+), 12 deletions(-)