Peter Maydell <[email protected]> writes: > Make the handling of indentation in doc comments more sophisticated, > so that when we see a section like: > > Notes: some text > some more text > indented line 3 > > we save it for the doc-comment processing code as: > > some text > some more text > indented line 3 > > and when we see a section with the heading on its own line: > > Notes: > > some text > some more text > indented text > > we also accept that and save it in the same form. > > The exception is that we always retain indentation as-is for Examples > sections, because these are literal text.
Does docs/devel/qapi-code-gen.txt need an update? Hmm, looks like you leave it to [PATCH 15] docs/devel/qapi-code-gen.txt: Update to new rST backend conventions. Acceptable. Mentioning it in the commit message now may make sense. > If we detect that the comment document text is not indented as much > as we expect it to be, we throw a parse error. (We don't complain > about over-indented sections, because for rST this can be legitimate > markup.) > > The golden reference for the doc comment text is updated to remove > the two 'wrong' indents; these now form a test case that we correctly > stripped leading whitespace from an indented multi-line argument > definition. > > Reviewed-by: Richard Henderson <[email protected]> > Signed-off-by: Peter Maydell <[email protected]> > --- > v1->v2: Update doc-good.out as per final para. > --- > scripts/qapi/parser.py | 81 +++++++++++++++++++++++++++------- > tests/qapi-schema/doc-good.out | 4 +- > 2 files changed, 67 insertions(+), 18 deletions(-) > > diff --git a/scripts/qapi/parser.py b/scripts/qapi/parser.py > index 7fae4478d34..d9f11eadd96 100644 > --- a/scripts/qapi/parser.py > +++ b/scripts/qapi/parser.py > @@ -308,18 +308,32 @@ class QAPIDoc: > """ > > class Section: > - def __init__(self, name=None): > + def __init__(self, parser, name=None, indent=0): > + # parser, for error messages about indentation > + self._parser = parser > # optional section name (argument/member or section name) > self.name = name > # the list of lines for this section > self.text = '' > + # the expected indent level of the text of this section > + self._indent = indent > > def append(self, line): > + # Strip leading spaces corresponding to the expected indent level > + # Blank lines are always OK. > + if line: > + spacecount = len(line) - len(line.lstrip(" ")) Works, but I'd prefer indent = re.match(r'\s*', line).end() > + if spacecount > self._indent: > + spacecount = self._indent > + if spacecount < self._indent: > + raise QAPIParseError(self._parser, "unexpected > de-indent") New error needs test coverage. I append a possible test. Reporting the expected indentation might be helpful. > + line = line[spacecount:] If you use self._indent instead of spacecount here (which I find clearer), you don't need to cap spacecount at self._indent above. > + > self.text += line.rstrip() + '\n' > > class ArgSection(Section): > - def __init__(self, name): > - super().__init__(name) > + def __init__(self, parser, name, indent=0): > + super().__init__(parser, name, indent) > self.member = None > > def connect(self, member): > @@ -333,7 +347,7 @@ class QAPIDoc: > self._parser = parser > self.info = info > self.symbol = None > - self.body = QAPIDoc.Section() > + self.body = QAPIDoc.Section(parser) > # dict mapping parameter name to ArgSection > self.args = OrderedDict() > self.features = OrderedDict() > @@ -438,7 +452,18 @@ class QAPIDoc: > > if name.startswith('@') and name.endswith(':'): > line = line[len(name)+1:] > - self._start_args_section(name[1:-1]) > + if not line or line.isspace(): > + # Line was just the "@arg:" header; following lines > + # are not indented > + indent = 0 > + line = '' > + else: > + # Line is "@arg: first line of description"; following > + # lines should be indented by len(name) + 1, and we > + # pad out this first line so it is handled the same way > + indent = len(name) + 1 > + line = ' ' * indent + line > + self._start_args_section(name[1:-1], indent) > elif self._is_section_tag(name): > self._append_line = self._append_various_line > self._append_various_line(line) > @@ -460,7 +485,17 @@ class QAPIDoc: > > if name.startswith('@') and name.endswith(':'): > line = line[len(name)+1:] > - self._start_features_section(name[1:-1]) > + if not line or line.isspace(): > + # Line is just the "@name:" header, no ident for following > lines pycodestyle complains: scripts/qapi/parser.py:489:80: E501 line too long (80 > 79 characters) > + indent = 0 > + line = '' > + else: > + # Line is "@arg: first line of description"; following > + # lines should be indented by len(name) + 3, and we > + # pad out this first line so it is handled the same way > + indent = len(name) + 1 Comment claims + 3, code uses + 1. Does this do the right thing when @arg: is followed by multiple whitespace characters? > + line = ' ' * indent + line > + self._start_features_section(name[1:-1], indent) > elif self._is_section_tag(name): > self._append_line = self._append_various_line > self._append_various_line(line) > @@ -493,11 +528,23 @@ class QAPIDoc: > % (name, self.sections[0].name)) > if self._is_section_tag(name): > line = line[len(name)+1:] > - self._start_section(name[:-1]) > + if not line or line.isspace(): > + # Line is just "SectionName:", no indent for following lines > + indent = 0 > + line = '' > + elif name.startswith("Example"): > + # The "Examples" section is literal-text, so preserve > + # all the indentation as-is > + indent = 0 Section "Example" is an exception. Needs to be documented. Do we really need the exception? As far as I can see, it's only ever used in documentation of block-latency-histogram-set. > + else: > + # Line is "SectionName: some text", indent required Same situation as above, much terser comment. > + indent = len(name) + 1 > + line = ' ' * indent + line > + self._start_section(name[:-1], indent) > > self._append_freeform(line) > > - def _start_symbol_section(self, symbols_dict, name): > + def _start_symbol_section(self, symbols_dict, name, indent): > # FIXME invalid names other than the empty string aren't flagged > if not name: > raise QAPIParseError(self._parser, "invalid parameter name") > @@ -506,21 +553,21 @@ class QAPIDoc: > "'%s' parameter name duplicated" % name) > assert not self.sections > self._end_section() > - self._section = QAPIDoc.ArgSection(name) > + self._section = QAPIDoc.ArgSection(self._parser, name, indent) > symbols_dict[name] = self._section > > - def _start_args_section(self, name): > - self._start_symbol_section(self.args, name) > + def _start_args_section(self, name, indent): > + self._start_symbol_section(self.args, name, indent) > > - def _start_features_section(self, name): > - self._start_symbol_section(self.features, name) > + def _start_features_section(self, name, indent): > + self._start_symbol_section(self.features, name, indent) > > - def _start_section(self, name=None): > + def _start_section(self, name=None, indent=0): > if name in ('Returns', 'Since') and self.has_section(name): > raise QAPIParseError(self._parser, > "duplicated '%s' section" % name) > self._end_section() > - self._section = QAPIDoc.Section(name) > + self._section = QAPIDoc.Section(self._parser, name, indent) > self.sections.append(self._section) > > def _end_section(self): > @@ -543,7 +590,7 @@ class QAPIDoc: > def connect_member(self, member): > if member.name not in self.args: > # Undocumented TODO outlaw > - self.args[member.name] = QAPIDoc.ArgSection(member.name) > + self.args[member.name] = QAPIDoc.ArgSection(self._parser, > member.name) pycodestyle complains: scripts/qapi/parser.py:593:80: E501 line too long (82 > 79 characters) > self.args[member.name].connect(member) > > def connect_feature(self, feature): > @@ -551,6 +598,8 @@ class QAPIDoc: > raise QAPISemError(feature.info, > "feature '%s' lacks documentation" > % feature.name) > + self.features[feature.name] = QAPIDoc.ArgSection(self._parser, > + feature.name) pylint points out: scripts/qapi/parser.py:601:12: W0101: Unreachable code (unreachable) > self.features[feature.name].connect(feature) > > def check_expr(self, expr): > diff --git a/tests/qapi-schema/doc-good.out b/tests/qapi-schema/doc-good.out > index 0ef85d959ac..bbf77b08dc3 100644 > --- a/tests/qapi-schema/doc-good.out > +++ b/tests/qapi-schema/doc-good.out > @@ -158,7 +158,7 @@ doc symbol=Alternate > > arg=i > an integer > - @b is undocumented > +@b is undocumented > arg=b > > feature=alt-feat > @@ -173,7 +173,7 @@ doc symbol=cmd > the first argument > arg=arg2 > the second > - argument > +argument > arg=arg3 > > feature=cmd-feat1 Suggested new test doc-bad-deintent.json, cribbed from your PATCH 06 of doc-good.json: ## # @Alternate: # @i: an integer # @b is undocumented ## { 'alternate': 'Alternate', 'data': { 'i': 'int', 'b': 'bool' } }
