Hiya, this is a series that explores a potential syntax for a designated "Intro" section. Markus knows why I want this, but for everyone else: a designated "Introduction" section is useful for the desired "inliner" feature for the new QAPI doc system. Commits explain a bit more. This is prep work and doesn't really change anything tangibly except source code syntax for the QAPI docs. It is designed so that this conversion can happen incrementally with no actual difference to the rendered manuals, so each QAPI module can be converted one at a time for easier review and merging in an arbitrary order. This series demonstrates conversion of just four modules; if I'm given a thumbs up, I will convert the rest of QAPI, one module (or maintainer stanza) per patch like how I handled adding cross-references. ** V3: changed intro section syntax, made various fixes and changes; I'm going to be honest Markus I did not keep a strict account this time ;_; V2: punt intro sections to the subsequent line and adjust the parser accordingly. Left as an RFC patch that undoes some things this series does instead of properly factoring into the series. John Snow (16): python: temporarily restrict max mypy version tests/qapi: generate output in source order qapi/docs: remove unused QAPIDoc subsection members qapi/docs: add has_features property qapi/docs: make remaining subsection members "private" qapi/docs: fix comment phrasing qapi/docs: add "Intro" section qapi/parser: move _insert_near_kind() method qapi/docs: adjust stub member insertion algorithm qapi/docs: remove implicit Plain section qapi/docs: add rendering for INTRO sections qapi/docs: add "Intro" section parsing qapi: convert intro sections for accelerator.json qapi: convert intro sections for acpi-hest.json qapi: convert intro sections for acpi.json qapi: convert intro sections for audio.json docs/devel/qapi-code-gen.rst | 16 ++-- docs/sphinx/qapidoc.py | 59 +++++++----- qapi/accelerator.json | 18 ++-- qapi/acpi-hest.json | 4 +- qapi/acpi.json | 18 ++-- qapi/audio.json | 88 +++++++----------- python/setup.cfg | 3 +- scripts/qapi/parser.py | 162 +++++++++++++++++++-------------- tests/qapi-schema/doc-good.out | 86 ++++++++--------- tests/qapi-schema/test-qapi.py | 14 +-- 10 files changed, 236 insertions(+), 232 deletions(-) -- 2.54.0 The newest versions of mypy do not support targeting Python 3.9, which we still support. I want to address that soon, but in the meantime it's nice if the tests pass. Signed-off-by: John Snow <jsnow@redhat.com> --- python/setup.cfg | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/python/setup.cfg b/python/setup.cfg index XXXXXXX..XXXXXXX 100644 --- a/python/setup.cfg +++ b/python/setup.cfg @@ -XXX,XX +XXX,XX @@ devel = flake8 >= 5.0.4 fusepy >= 2.0.4 isort >= 5.6.0 - mypy >= 1.4.0 + # Max ver is temporary until we drop 3.9 + mypy >= 1.4.0, <2.0.0 pylint >= 2.17.3 pylint != 3.2.4; python_version<"3.9" pytest >= 6.0.2 -- 2.54.0 Rewrite the test doc generator to produce output in source order instead of arbitrarily by section name. This patch removes our last use of the "body" field, which has an effect on how the sections of each test documention block are printed. We now print the name of the section followed by the section text for all sections except Members and Features, which are printed as "Member=%s" or "Feature=%s" followed by the section text, respectively. This patch is motivated by a desire to move the QAPIDoc API away from named fields for specific sections in a bid to force all users to simply iterate through all_sections in order, instead - and to remove the named subsections. Signed-off-by: John Snow <jsnow@redhat.com> --- tests/qapi-schema/doc-good.out | 82 +++++++++++++++++----------------- tests/qapi-schema/test-qapi.py | 14 +++--- 2 files changed, 48 insertions(+), 48 deletions(-) diff --git a/tests/qapi-schema/doc-good.out b/tests/qapi-schema/doc-good.out index XXXXXXX..XXXXXXX 100644 --- a/tests/qapi-schema/doc-good.out +++ b/tests/qapi-schema/doc-good.out @@ -XXX,XX +XXX,XX @@ event EVT_BOXED Object boxed=True feature feat3 doc freeform - body= + Plain ******* Section ******* doc freeform - body= + Plain Just text, no heading. doc freeform - body= + Plain Subsection ========== @@ -XXX,XX +XXX,XX @@ Examples: - *verbatim* - {braces} doc symbol=Enum - body= + Plain - arg=one + Member=one The _one_ {and only}, description on the same line - arg=two + Member=two - feature=enum-feat + Feature=enum-feat Also _one_ {and only} - feature=enum-member-feat + Feature=enum-member-feat a member feature - section=Plain + Plain @two is undocumented doc symbol=Base - body= + Plain - arg=base1 + Member=base1 description starts on a new line, minimally indented doc symbol=Variant1 - body= + Plain A paragraph Another paragraph @var1 is undocumented - arg=var1 + Member=var1 - feature=variant1-feat + Feature=variant1-feat a feature - feature=member-feat + Feature=member-feat a member feature doc symbol=Variant2 - body= + Plain doc symbol=Object - body= + Plain - feature=union-feat1 + Feature=union-feat1 a feature doc symbol=Alternate - body= + Plain - arg=i + Member=i description starts on the same line remainder indented the same @b is undocumented - arg=b + Member=b - feature=alt-feat + Feature=alt-feat a feature doc freeform - body= + Plain Another subsection ================== doc symbol=cmd - body= + Plain - arg=arg1 + Member=arg1 description starts on a new line, indented - arg=arg2 + Member=arg2 description starts on the same line remainder indented differently - arg=arg3 + Member=arg3 - feature=cmd-feat1 + Feature=cmd-feat1 a feature - feature=cmd-feat2 + Feature=cmd-feat2 another feature - section=Plain + Plain .. note:: @arg3 is undocumented - section=Returns + Returns @Object - section=Errors + Errors some - section=Todo + Todo frobnicate - section=Plain + Plain .. admonition:: Notes - Lorem ipsum dolor sit amet @@ -XXX,XX +XXX,XX @@ Examples:: Note:: Ceci n'est pas une note - section=Since + Since 2.10 doc symbol=cmd-boxed - body= + Plain If you're bored enough to read this, go see a video of boxed cats - feature=cmd-feat1 + Feature=cmd-feat1 a feature - feature=cmd-feat2 + Feature=cmd-feat2 another feature - section=Plain + Plain .. qmp-example:: -> "this example" <- ... has no title ... doc symbol=EVT_BOXED - body= + Plain - feature=feat3 + Feature=feat3 a feature diff --git a/tests/qapi-schema/test-qapi.py b/tests/qapi-schema/test-qapi.py index XXXXXXX..XXXXXXX 100755 --- a/tests/qapi-schema/test-qapi.py +++ b/tests/qapi-schema/test-qapi.py @@ -XXX,XX +XXX,XX @@ from io import StringIO from qapi.error import QAPIError +from qapi.parser import QAPIDoc from qapi.schema import QAPISchema, QAPISchemaVisitor @@ -XXX,XX +XXX,XX @@ def test_frontend(fname): print('doc symbol=%s' % doc.symbol) else: print('doc freeform') - print(' body=\n%s' % doc.body.text) - for arg, section in doc.args.items(): - print(' arg=%s\n%s' % (arg, section.text)) - for feat, section in doc.features.items(): - print(' feature=%s\n%s' % (feat, section.text)) - for section in doc.sections: - print(' section=%s\n%s' % (section.kind, section.text)) + for section in doc.all_sections: + if isinstance(section, QAPIDoc.ArgSection): + print(' %s=%s' % (section.kind, section.name)) + else: + print(' %s' % section.kind) + print(section.text) def open_test_result(dir_name, file_name, update): -- 2.54.0 All users of the QAPIDoc object should be iterating over all_sections and not grabbing arbitrary subsections, if possible. Remove the 'body' and 'sections' members, as they are no longer used. Signed-off-by: John Snow <jsnow@redhat.com> Reviewed-by: Markus Armbruster <armbru@redhat.com> --- scripts/qapi/parser.py | 6 ------ 1 file changed, 6 deletions(-) diff --git a/scripts/qapi/parser.py b/scripts/qapi/parser.py index XXXXXXX..XXXXXXX 100644 --- a/scripts/qapi/parser.py +++ b/scripts/qapi/parser.py @@ -XXX,XX +XXX,XX @@ def __init__(self, info: QAPISourceInfo, symbol: Optional[str] = None): self.all_sections: List[QAPIDoc.Section] = [ QAPIDoc.Section(info, QAPIDoc.Kind.PLAIN) ] - # the body section - self.body: Optional[QAPIDoc.Section] = self.all_sections[0] # dicts mapping parameter/feature names to their description self.args: Dict[str, QAPIDoc.ArgSection] = {} self.features: Dict[str, QAPIDoc.ArgSection] = {} @@ -XXX,XX +XXX,XX @@ def __init__(self, info: QAPISourceInfo, symbol: Optional[str] = None): self.errors: Optional[QAPIDoc.Section] = None # "Since" section self.since: Optional[QAPIDoc.Section] = None - # sections other than .body, .args, .features - self.sections: List[QAPIDoc.Section] = [] def end(self) -> None: for section in self.all_sections: @@ -XXX,XX +XXX,XX @@ def ensure_untagged_section(self, info: QAPISourceInfo) -> None: # start new section section = self.Section(info, kind) - self.sections.append(section) self.all_sections.append(section) def new_tagged_section( @@ -XXX,XX +XXX,XX @@ def new_tagged_section( raise QAPISemError( info, "duplicated '%s' section" % kind) self.since = section - self.sections.append(section) self.all_sections.append(section) def _new_description( -- 2.54.0 Begin preparing to remove public access to the .features member by introducing a semantic "has features" property instead. Signed-off-by: John Snow <jsnow@redhat.com> --- scripts/qapi/parser.py | 8 ++++++-- 1 file changed, 6 insertions(+), 2 deletions(-) diff --git a/scripts/qapi/parser.py b/scripts/qapi/parser.py index XXXXXXX..XXXXXXX 100644 --- a/scripts/qapi/parser.py +++ b/scripts/qapi/parser.py @@ -XXX,XX +XXX,XX @@ def get_doc(self) -> 'QAPIDoc': break # Non-blank line, first of a section if line == 'Features:': - if doc.features: + if doc.has_features: raise QAPIParseError( self, "duplicated 'Features:' line") self.accept(False) @@ -XXX,XX +XXX,XX @@ def get_doc(self) -> 'QAPIDoc': if text: doc.append_line(text) line = self.get_doc_indented(doc) - if not doc.features: + if not doc.has_features: raise QAPIParseError( self, 'feature descriptions expected') no_more_args = True @@ -XXX,XX +XXX,XX @@ def __init__(self, info: QAPISourceInfo, symbol: Optional[str] = None): # "Since" section self.since: Optional[QAPIDoc.Section] = None + @property + def has_features(self) -> bool: + return bool(self.features) + def end(self) -> None: for section in self.all_sections: section.text = section.text.strip('\n') -- 2.54.0 These fields are used to provide error checking and internal logistics and should not be used by a user of the library to directly access documentation sections, so make them private. The "since" field alone is left public, as the qapidoc generator does use this field to pull that section out of the regular flow of the document. Signed-off-by: John Snow <jsnow@redhat.com> --- scripts/qapi/parser.py | 48 +++++++++++++++++++++--------------------- 1 file changed, 24 insertions(+), 24 deletions(-) diff --git a/scripts/qapi/parser.py b/scripts/qapi/parser.py index XXXXXXX..XXXXXXX 100644 --- a/scripts/qapi/parser.py +++ b/scripts/qapi/parser.py @@ -XXX,XX +XXX,XX @@ def __init__(self, info: QAPISourceInfo, symbol: Optional[str] = None): QAPIDoc.Section(info, QAPIDoc.Kind.PLAIN) ] # dicts mapping parameter/feature names to their description - self.args: Dict[str, QAPIDoc.ArgSection] = {} - self.features: Dict[str, QAPIDoc.ArgSection] = {} + self._args: Dict[str, QAPIDoc.ArgSection] = {} + self._features: Dict[str, QAPIDoc.ArgSection] = {} # a command's "Returns" and "Errors" section - self.returns: Optional[QAPIDoc.Section] = None - self.errors: Optional[QAPIDoc.Section] = None + self._returns: Optional[QAPIDoc.Section] = None + self._errors: Optional[QAPIDoc.Section] = None # "Since" section self.since: Optional[QAPIDoc.Section] = None @property def has_features(self) -> bool: - return bool(self.features) + return bool(self._features) def end(self) -> None: for section in self.all_sections: @@ -XXX,XX +XXX,XX @@ def new_tagged_section( ) -> None: section = self.Section(info, kind) if kind == QAPIDoc.Kind.RETURNS: - if self.returns: + if self._returns: raise QAPISemError( info, "duplicated '%s' section" % kind) - self.returns = section + self._returns = section elif kind == QAPIDoc.Kind.ERRORS: - if self.errors: + if self._errors: raise QAPISemError( info, "duplicated '%s' section" % kind) - self.errors = section + self._errors = section elif kind == QAPIDoc.Kind.SINCE: if self.since: raise QAPISemError( @@ -XXX,XX +XXX,XX @@ def _new_description( desc[name] = section def new_argument(self, info: QAPISourceInfo, name: str) -> None: - self._new_description(info, name, QAPIDoc.Kind.MEMBER, self.args) + self._new_description(info, name, QAPIDoc.Kind.MEMBER, self._args) def new_feature(self, info: QAPISourceInfo, name: str) -> None: - self._new_description(info, name, QAPIDoc.Kind.FEATURE, self.features) + self._new_description(info, name, QAPIDoc.Kind.FEATURE, self._features) def append_line(self, line: str) -> None: self.all_sections[-1].append_line(line) def connect_member(self, member: 'QAPISchemaMember') -> None: - if member.name not in self.args: + if member.name not in self._args: assert member.info if self.symbol not in member.info.pragma.documentation_exceptions: raise QAPISemError(member.info, @@ -XXX,XX +XXX,XX @@ def connect_member(self, member: 'QAPISchemaMember') -> None: section = QAPIDoc.ArgSection( self.info, QAPIDoc.Kind.MEMBER, member.name) - self.args[member.name] = section + self._args[member.name] = section # Determine where to insert stub doc - it should go at the # end of the members section(s), if any. Note that index 0 @@ -XXX,XX +XXX,XX @@ def connect_member(self, member: 'QAPISchemaMember') -> None: index += 1 self.all_sections.insert(index, section) - self.args[member.name].connect(member) + self._args[member.name].connect(member) def connect_feature(self, feature: 'QAPISchemaFeature') -> None: - if feature.name not in self.features: + if feature.name not in self._features: raise QAPISemError(feature.info, "feature '%s' lacks documentation" % feature.name) - self.features[feature.name].connect(feature) + self._features[feature.name].connect(feature) def ensure_returns(self, info: QAPISourceInfo) -> None: @@ -XXX,XX +XXX,XX @@ def _insert_near_kind( def check_expr(self, expr: QAPIExpression) -> None: if 'command' in expr: - if self.returns and 'returns' not in expr: + if self._returns and 'returns' not in expr: raise QAPISemError( - self.returns.info, + self._returns.info, "'Returns' section, but command doesn't return anything") else: - if self.returns: + if self._returns: raise QAPISemError( - self.returns.info, + self._returns.info, "'Returns' section is only valid for commands") - if self.errors: + if self._errors: raise QAPISemError( - self.errors.info, + self._errors.info, "'Errors' section is only valid for commands") def check(self) -> None: @@ -XXX,XX +XXX,XX @@ def check_args_section( "do" if len(bogus) > 1 else "does" )) - check_args_section(self.args, 'member') - check_args_section(self.features, 'feature') + check_args_section(self._args, 'member') + check_args_section(self._features, 'feature') -- 2.54.0 Plaintext sections can be one or more paragraphs, the original comment was a mistake. Signed-off-by: John Snow <jsnow@redhat.com> --- scripts/qapi/parser.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/scripts/qapi/parser.py b/scripts/qapi/parser.py index XXXXXXX..XXXXXXX 100644 --- a/scripts/qapi/parser.py +++ b/scripts/qapi/parser.py @@ -XXX,XX +XXX,XX @@ def get_doc(self) -> 'QAPIDoc': line = self.get_doc_indented(doc) no_more_args = True else: - # plain paragraph + # plain paragraph(s) doc.ensure_untagged_section(self.info) doc.append_line(line) line = self.get_doc_paragraph(doc) -- 2.54.0 This patch adds an "Intro" doc section, intended to eventually replace the "Plain" doc section alongside a forthcoming "Details" section. For now, this section is not actually instantiated or used, but subsequent patches will slowly convert the leading introductory sections of QAPIDoc documentation to use this new section. A main motivation of this series of changes is to more explicitly delineate the "Introductory" documentation for each QAPI definition for the sake of the inliner. When inlining members, examples, and details from multiple QAPIDoc sections, we will want to omit the "Introductory" text from inlined definitions while keeping notes, caution boxes, examples, and so on. This new section facilitates that. Signed-off-by: John Snow <jsnow@redhat.com> --- docs/sphinx/qapidoc.py | 2 +- scripts/qapi/parser.py | 3 ++- 2 files changed, 3 insertions(+), 2 deletions(-) diff --git a/docs/sphinx/qapidoc.py b/docs/sphinx/qapidoc.py index XXXXXXX..XXXXXXX 100644 --- a/docs/sphinx/qapidoc.py +++ b/docs/sphinx/qapidoc.py @@ -XXX,XX +XXX,XX @@ def visit_sections(self, ent: QAPISchemaDefinition) -> None: for i, section in enumerate(sections): section.text = self.reformat_arobase(section.text) - if section.kind == QAPIDoc.Kind.PLAIN: + if section.kind.name in ("PLAIN", "INTRO"): self.visit_paragraph(section) elif section.kind == QAPIDoc.Kind.MEMBER: assert isinstance(section, QAPIDoc.ArgSection) diff --git a/scripts/qapi/parser.py b/scripts/qapi/parser.py index XXXXXXX..XXXXXXX 100644 --- a/scripts/qapi/parser.py +++ b/scripts/qapi/parser.py @@ -XXX,XX +XXX,XX @@ class Kind(enum.Enum): ERRORS = 4 SINCE = 5 TODO = 6 + INTRO = 7 @staticmethod def from_string(kind: str) -> 'QAPIDoc.Kind': @@ -XXX,XX +XXX,XX @@ def has_features(self) -> bool: def end(self) -> None: for section in self.all_sections: section.text = section.text.strip('\n') - if section.kind != QAPIDoc.Kind.PLAIN and section.text == '': + if not (section.kind.name in ("INTRO", "PLAIN") or section.text): raise QAPISemError( section.info, "text required after '%s:'" % section.kind) -- 2.54.0 Move this function out from underneath `ensure_returns` and make it available for general purpose use as an object method instead. This is purely a scope-level patch with no functional changes. Signed-off-by: John Snow <jsnow@redhat.com> --- scripts/qapi/parser.py | 33 +++++++++++++++++---------------- 1 file changed, 17 insertions(+), 16 deletions(-) diff --git a/scripts/qapi/parser.py b/scripts/qapi/parser.py index XXXXXXX..XXXXXXX 100644 --- a/scripts/qapi/parser.py +++ b/scripts/qapi/parser.py @@ -XXX,XX +XXX,XX @@ def new_feature(self, info: QAPISourceInfo, name: str) -> None: def append_line(self, line: str) -> None: self.all_sections[-1].append_line(line) + def _insert_near_kind( + self, + kind: 'QAPIDoc.Kind', + new_sect: 'QAPIDoc.Section', + after: bool = False, + ) -> bool: + """Insert or append a new doc section at a specific point.""" + for idx, sect in enumerate(reversed(self.all_sections)): + if sect.kind == kind: + pos = len(self.all_sections) - idx - 1 + if after: + pos += 1 + self.all_sections.insert(pos, new_sect) + return True + return False + def connect_member(self, member: 'QAPISchemaMember') -> None: if member.name not in self._args: assert member.info @@ -XXX,XX +XXX,XX @@ def connect_feature(self, feature: 'QAPISchemaFeature') -> None: self._features[feature.name].connect(feature) def ensure_returns(self, info: QAPISourceInfo) -> None: - - def _insert_near_kind( - kind: QAPIDoc.Kind, - new_sect: QAPIDoc.Section, - after: bool = False, - ) -> bool: - for idx, sect in enumerate(reversed(self.all_sections)): - if sect.kind == kind: - pos = len(self.all_sections) - idx - 1 - if after: - pos += 1 - self.all_sections.insert(pos, new_sect) - return True - return False - if any(s.kind == QAPIDoc.Kind.RETURNS for s in self.all_sections): return # Stub "Returns" section for undocumented returns value stub = QAPIDoc.Section(info, QAPIDoc.Kind.RETURNS) - if any(_insert_near_kind(kind, stub, after) for kind, after in ( + if any(self._insert_near_kind(kind, stub, after) for kind, after in ( # 1. If arguments, right after those. (QAPIDoc.Kind.MEMBER, True), # 2. Elif errors, right *before* those. -- 2.54.0 A forthcoming patch removes the implicit PLAIN section that always starts a QAPIDoc section list. Further future changes begin converting "PLAIN" sections to "INTRO" sections. To accommodate this, the insertion algorithm that places stub and dummy members must be adjusted to cope with not only the finished state, but temporary intermediate states while the series is merged. This algorithm can handle zero-or-more PLAIN *or* INTRO sections at the beginning of a QAPIDoc object, in contrast to the previous algorithm which assumed and relied upon there being always one PLAIN section at the beginning of every QAPIDoc section list. In other words: (PLAIN | INTRO)* <EverythingElse> This does not impact what the parser itself will actually produce. As of this patch, the parser will still always generate QAPIDoc section lists that start with precisely one PLAIN section (whether or not it is empty), followed by the remaining sections. Those remaining sections may or may not include additional PLAIN sections, but never two such sections contiguously as the parser will always treat that layout as one PLAIN section consisting of multiple paragraph(s). In other other words: This insertion algorithm is more lenient than the parser, but this is on purpose for flexibility mid-stream as we convert QAPI to using explicit introductory sections. The allowed order of sections will eventually become strictly enforced in the parser, which will in turn allow dramatic simplifications to the insertion algorithm. This only exists as transitory code until we are able to enforce that order. Fear not: the intermediate rest output before and after this patch are byte identical, so failing all else, we at least know it doesn't make anything worse. Lastly, because we have three places in the code that need to insert stub/dummy sections, we take the opportunity to consolidate this code to handle all three cases with one function. This winds up necessitating the qapidoc.py generator actually modify the section list to insert a "dummy" member that acts as a placeholder for "The members of ..." text. While it looks like a code smell to modify the caller's argument, it is ultimately safe because the QAPI Schema object is re-parsed and re-constructed in memory for each individual process that needs to operate on it. In other words, the Sphinx document generator already does have "its own copy" of the section lists, so it is "safe" to modify here without regards to other consumers of the QAPIDoc objects. It only *looks* like it smells bad. Ultimately, this code will also be removed once the inliner is merged, so it is only a temporary aesthetic issue regardless. That's my story and I'm sticking to it. Signed-off-by: John Snow <jsnow@redhat.com> --- docs/sphinx/qapidoc.py | 42 ++++++++++++++++-------------- scripts/qapi/parser.py | 58 +++++++++++++++++++++++++++++------------- 2 files changed, 64 insertions(+), 36 deletions(-) diff --git a/docs/sphinx/qapidoc.py b/docs/sphinx/qapidoc.py index XXXXXXX..XXXXXXX 100644 --- a/docs/sphinx/qapidoc.py +++ b/docs/sphinx/qapidoc.py @@ -XXX,XX +XXX,XX @@ def _get_target( ) def visit_sections(self, ent: QAPISchemaDefinition) -> None: + # Generate a placeholder right after the member section(s) which + # may be used to generate documentation for "The members of..." + # pointers in the rendered document. + # + # This is a temporary hack until the inliner is merged. Note + # that although we modify the caller's section list, the + # Sphinx document generator has its own copy of the parsed + # schema in memory, so this action does not interfere with + # other users of the QAPISchema or QAPIDoc objects outside of + # the document generator. Fishy, but not harmful. + if ent.doc: + ent.doc.append_member_stub( + QAPIDoc.ArgSection( + ent.doc.info, QAPIDoc.Kind.MEMBER, "q_dummy" + ) + ) + sections = ent.doc.all_sections if ent.doc else [] - # Determine the index location at which we should generate - # documentation for "The members of ..." pointers. This should - # go at the end of the members section(s) if any. Note that - # index 0 is assumed to be a plain intro section, even if it is - # empty; and that a members section if present will always - # immediately follow the opening PLAIN section. - gen_index = 1 - if len(sections) > 1: - while sections[gen_index].kind == QAPIDoc.Kind.MEMBER: - gen_index += 1 - if gen_index >= len(sections): - break - # Add sections in source order: - for i, section in enumerate(sections): + for section in sections: section.text = self.reformat_arobase(section.text) if section.kind.name in ("PLAIN", "INTRO"): self.visit_paragraph(section) elif section.kind == QAPIDoc.Kind.MEMBER: assert isinstance(section, QAPIDoc.ArgSection) - self.visit_member(section) + if section.name == "q_dummy": + # Generate "The members of ..." entries if necessary + self._insert_member_pointer(ent) + else: + self.visit_member(section) elif section.kind == QAPIDoc.Kind.FEATURE: assert isinstance(section, QAPIDoc.ArgSection) self.visit_feature(section) @@ -XXX,XX +XXX,XX @@ def visit_sections(self, ent: QAPISchemaDefinition) -> None: else: assert False - # Generate "The members of ..." entries if necessary: - if i == gen_index - 1: - self._insert_member_pointer(ent) - self.ensure_blank_line() # Transmogrification core methods diff --git a/scripts/qapi/parser.py b/scripts/qapi/parser.py index XXXXXXX..XXXXXXX 100644 --- a/scripts/qapi/parser.py +++ b/scripts/qapi/parser.py @@ -XXX,XX +XXX,XX @@ def _insert_near_kind( return True return False + def _insert_after_intro( + self, + section: 'QAPIDoc.Section', + ) -> None: + """ + Insert a section immediately after the intro section. + + While we convert PLAIN sections to INTRO sections, all + contiguous INTRO/PLAIN sections at the start of a QAPIDoc + section list are treated as "the intro". + + Once INTRO conversion is complete, this helper will no longer be + needed and ``_insert_near_kind(QAPIDoc.Kind.INTRO, ...)`` will + be sufficient. + """ + index = 0 + for index, ref_section in enumerate(self.all_sections): + if ref_section.kind.name in ("PLAIN", "INTRO"): + continue + break + else: + index += 1 + + self.all_sections.insert(index, section) + + def append_member_stub(self, stub: 'QAPIDoc.Section') -> None: + + """ + Append a stub section after any Member sections. + """ + if self._insert_near_kind(QAPIDoc.Kind.MEMBER, stub, True): + return + + # No MEMBER sections present. Insert after INTRO/PLAIN sections. + self._insert_after_intro(stub) + def connect_member(self, member: 'QAPISchemaMember') -> None: if member.name not in self._args: assert member.info @@ -XXX,XX +XXX,XX @@ def connect_member(self, member: 'QAPISchemaMember') -> None: % (member.role, member.name)) # Insert stub documentation section for missing member docs. # TODO: drop when undocumented members are outlawed - - section = QAPIDoc.ArgSection( + stub_section = QAPIDoc.ArgSection( self.info, QAPIDoc.Kind.MEMBER, member.name) - self._args[member.name] = section - - # Determine where to insert stub doc - it should go at the - # end of the members section(s), if any. Note that index 0 - # is assumed to be an untagged intro section, even if it is - # empty. - index = 1 - if len(self.all_sections) > 1: - while self.all_sections[index].kind == QAPIDoc.Kind.MEMBER: - index += 1 - self.all_sections.insert(index, section) + self._args[member.name] = stub_section + self.append_member_stub(stub_section) self._args[member.name].connect(member) @@ -XXX,XX +XXX,XX @@ def ensure_returns(self, info: QAPISourceInfo) -> None: )): return - # Otherwise, it should go right after the intro. The intro - # is always the first section and is always present (even - # when empty), so we can insert directly at index=1 blindly. - self.all_sections.insert(1, stub) + # Otherwise, it should go right after the intro. + self._insert_after_intro(stub) def check_expr(self, expr: QAPIExpression) -> None: if 'command' in expr: -- 2.54.0 Prior to this patch, we always create an empty Plain section. Removing this allows us to gradually phase out the "Plain" section in favor of "Intro" and "Details" sections while keeping "Plain" around for the interim churn during the series - meaning that we don't actually know at __init__ time which type of section we'll have first. Signed-off-by: John Snow <jsnow@redhat.com> --- scripts/qapi/parser.py | 4 +--- tests/qapi-schema/doc-good.out | 14 -------------- 2 files changed, 1 insertion(+), 17 deletions(-) diff --git a/scripts/qapi/parser.py b/scripts/qapi/parser.py index XXXXXXX..XXXXXXX 100644 --- a/scripts/qapi/parser.py +++ b/scripts/qapi/parser.py @@ -XXX,XX +XXX,XX @@ def __init__(self, info: QAPISourceInfo, symbol: Optional[str] = None): # definition doc's symbol, None for free-form doc self.symbol: Optional[str] = symbol # the sections in textual order - self.all_sections: List[QAPIDoc.Section] = [ - QAPIDoc.Section(info, QAPIDoc.Kind.PLAIN) - ] + self.all_sections: List[QAPIDoc.Section] = [] # dicts mapping parameter/feature names to their description self._args: Dict[str, QAPIDoc.ArgSection] = {} self._features: Dict[str, QAPIDoc.ArgSection] = {} diff --git a/tests/qapi-schema/doc-good.out b/tests/qapi-schema/doc-good.out index XXXXXXX..XXXXXXX 100644 --- a/tests/qapi-schema/doc-good.out +++ b/tests/qapi-schema/doc-good.out @@ -XXX,XX +XXX,XX @@ Examples: - *verbatim* - {braces} doc symbol=Enum - Plain - Member=one The _one_ {and only}, description on the same line Member=two @@ -XXX,XX +XXX,XX @@ a member feature Plain @two is undocumented doc symbol=Base - Plain - Member=base1 description starts on a new line, minimally indented @@ -XXX,XX +XXX,XX @@ a feature Feature=member-feat a member feature doc symbol=Variant2 - Plain - doc symbol=Object - Plain - Feature=union-feat1 a feature doc symbol=Alternate - Plain - Member=i description starts on the same line remainder indented the same @@ -XXX,XX +XXX,XX @@ doc freeform Another subsection ================== doc symbol=cmd - Plain - Member=arg1 description starts on a new line, indented @@ -XXX,XX +XXX,XX @@ another feature <- ... has no title ... doc symbol=EVT_BOXED - Plain - Feature=feat3 a feature -- 2.54.0 Amend the qapidoc generator to handle and render INTRO sections. The only real difference here from other sections is that we need to dedent the text so it renders correctly. Members and Features are also indented, but do not require a dedent() because they are always used in tandem with an rST construct that forms the start of a new indented block; there is coincidental harmony. Plaintext sections, however, do not start their own block and thus need to be dedented to prevent accidentally rendering them as a blockquote or a syntax error. This dedent transformation on the text does not reflow the text, so source line information remains accurate, and the "blame" chain of custody for sphinx rST parsing error messages continues to be correct even through this transformation. Signed-off-by: John Snow <jsnow@redhat.com> --- docs/sphinx/qapidoc.py | 15 ++++++++++++--- 1 file changed, 12 insertions(+), 3 deletions(-) diff --git a/docs/sphinx/qapidoc.py b/docs/sphinx/qapidoc.py index XXXXXXX..XXXXXXX 100644 --- a/docs/sphinx/qapidoc.py +++ b/docs/sphinx/qapidoc.py @@ -XXX,XX +XXX,XX @@ from pathlib import Path import re import sys +import textwrap from typing import TYPE_CHECKING from docutils import nodes @@ -XXX,XX +XXX,XX @@ def add_lines( self, content: str, info: QAPISourceInfo, + dedent: bool = False, ) -> None: lines = content.splitlines(True) + + if dedent: + lines = textwrap.dedent(content).splitlines(True) + else: + lines = content.splitlines(True) + for i, line in enumerate(lines): self.add_line_raw(line, info.fname, info.line + i) @@ -XXX,XX +XXX,XX @@ def reformat_arobase(text: str) -> str: # Transmogrification helpers - def visit_paragraph(self, section: QAPIDoc.Section) -> None: + def visit_plaintext(self, section: QAPIDoc.Section) -> None: # Squelch empty paragraphs. if not section.text: return + dedent = bool(section.kind == QAPIDoc.Kind.INTRO) self.ensure_blank_line() - self.add_lines(section.text, section.info) + self.add_lines(section.text, section.info, dedent) self.ensure_blank_line() def visit_member(self, section: QAPIDoc.ArgSection) -> None: @@ -XXX,XX +XXX,XX @@ def visit_sections(self, ent: QAPISchemaDefinition) -> None: section.text = self.reformat_arobase(section.text) if section.kind.name in ("PLAIN", "INTRO"): - self.visit_paragraph(section) + self.visit_plaintext(section) elif section.kind == QAPIDoc.Kind.MEMBER: assert isinstance(section, QAPIDoc.ArgSection) if section.name == "q_dummy": -- 2.54.0 Add parsing for explicit Intro section syntax. A side effect of this patch is that we will (currently) always create an empty Intro section, similar to how we used to have an empty Plain section. The tests are adjusted accordingly, rendered document output does not change at all. Signed-off-by: John Snow <jsnow@redhat.com> --- docs/devel/qapi-code-gen.rst | 16 +++++++--------- scripts/qapi/parser.py | 4 ++-- tests/qapi-schema/doc-good.out | 18 ++++++++++++++++++ 3 files changed, 27 insertions(+), 11 deletions(-) diff --git a/docs/devel/qapi-code-gen.rst b/docs/devel/qapi-code-gen.rst index XXXXXXX..XXXXXXX 100644 --- a/docs/devel/qapi-code-gen.rst +++ b/docs/devel/qapi-code-gen.rst @@ -XXX,XX +XXX,XX @@ definition it documents. When documentation is required (see pragma_ 'doc-required'), every definition must have documentation. -Definition documentation starts with a line naming the definition, -followed by an optional overview, a description of each argument (for -commands and events), member (for structs and unions), branch (for -alternates), or value (for enums), a description of each feature (if -any), and finally optional tagged sections. +Definition documentation starts with a description naming the +definition with an optional indented overview, a description of each +argument (for commands and events), member (for structs and unions), +branch (for alternates), or value (for enums), a description of each +feature (if any), and finally optional tagged sections. Descriptions start with '\@name:'. The description text must be indented like this:: @@ -XXX,XX +XXX,XX @@ Examples of complete definition documentation:: ## # @BlockStats: - # - # Statistics of a virtual block device or a block backing device. + # Statistics of a virtual block device or a block backing device. # # @device: If the stats are for a virtual block device, the name # corresponding to the virtual block device. @@ -XXX,XX +XXX,XX @@ Examples of complete definition documentation:: ## # @query-blockstats: - # - # Query the @BlockStats for all virtual block devices. + # Query the @BlockStats for all virtual block devices. # # @query-nodes: If true, the command will query all the block nodes # ... explain, explain ... diff --git a/scripts/qapi/parser.py b/scripts/qapi/parser.py index XXXXXXX..XXXXXXX 100644 --- a/scripts/qapi/parser.py +++ b/scripts/qapi/parser.py @@ -XXX,XX +XXX,XX @@ def get_doc(self) -> 'QAPIDoc': if not symbol: raise QAPIParseError(self, "name required after '@'") doc = QAPIDoc(info, symbol) - self.accept(False) - line = self.get_doc_line() + doc.all_sections.append(QAPIDoc.Section(info, QAPIDoc.Kind.INTRO)) + line = self.get_doc_indented(doc) no_more_args = False while line is not None: diff --git a/tests/qapi-schema/doc-good.out b/tests/qapi-schema/doc-good.out index XXXXXXX..XXXXXXX 100644 --- a/tests/qapi-schema/doc-good.out +++ b/tests/qapi-schema/doc-good.out @@ -XXX,XX +XXX,XX @@ Examples: - *verbatim* - {braces} doc symbol=Enum + Intro + Member=one The _one_ {and only}, description on the same line Member=two @@ -XXX,XX +XXX,XX @@ a member feature Plain @two is undocumented doc symbol=Base + Intro + Member=base1 description starts on a new line, minimally indented doc symbol=Variant1 + Intro + Plain A paragraph @@ -XXX,XX +XXX,XX @@ a feature Feature=member-feat a member feature doc symbol=Variant2 + Intro + doc symbol=Object + Intro + Feature=union-feat1 a feature doc symbol=Alternate + Intro + Member=i description starts on the same line remainder indented the same @@ -XXX,XX +XXX,XX @@ doc freeform Another subsection ================== doc symbol=cmd + Intro + Member=arg1 description starts on a new line, indented @@ -XXX,XX +XXX,XX @@ Note:: Since 2.10 doc symbol=cmd-boxed + Intro + Plain If you're bored enough to read this, go see a video of boxed cats Feature=cmd-feat1 @@ -XXX,XX +XXX,XX @@ another feature <- ... has no title ... doc symbol=EVT_BOXED + Intro + Feature=feat3 a feature -- 2.54.0 Signed-off-by: John Snow <jsnow@redhat.com> --- qapi/accelerator.json | 18 ++++++------------ 1 file changed, 6 insertions(+), 12 deletions(-) diff --git a/qapi/accelerator.json b/qapi/accelerator.json index XXXXXXX..XXXXXXX 100644 --- a/qapi/accelerator.json +++ b/qapi/accelerator.json @@ -XXX,XX +XXX,XX @@ ## # @KvmInfo: -# -# Information about support for KVM acceleration +# Information about support for KVM acceleration # # @enabled: true if KVM acceleration is active # @@ -XXX,XX +XXX,XX @@ ## # @query-kvm: -# -# Return information about KVM acceleration +# Return information about KVM acceleration # # Features: # @@ -XXX,XX +XXX,XX @@ ## # @x-accel-stats: -# -# Query accelerator statistics +# Query accelerator statistics # # Features: # @@ -XXX,XX +XXX,XX @@ ## # @Accelerator: -# -# Information about support for MSHV acceleration +# Information about support for MSHV acceleration # # @hvf: Apple Hypervisor.framework # @@ -XXX,XX +XXX,XX @@ ## # @AcceleratorInfo: -# -# Information about support for various accelerators +# Information about support for various accelerators # # @enabled: the accelerator that is in use # @@ -XXX,XX +XXX,XX @@ ## # @query-accelerators: -# -# Return information about accelerators +# Return information about accelerators # # Returns: @AcceleratorInfo # -- 2.54.0 Signed-off-by: John Snow <jsnow@redhat.com> --- qapi/acpi-hest.json | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/qapi/acpi-hest.json b/qapi/acpi-hest.json index XXXXXXX..XXXXXXX 100644 --- a/qapi/acpi-hest.json +++ b/qapi/acpi-hest.json @@ -XXX,XX +XXX,XX @@ ## # @inject-ghes-v2-error: -# -# Inject an error with additional ACPI 6.1 GHESv2 error information +# Inject an error with additional ACPI 6.1 GHESv2 error +# information # # @cper: contains a base64 encoded string with raw data for a single # CPER record with Generic Error Status Block, Generic Error Data -- 2.54.0 Signed-off-by: John Snow <jsnow@redhat.com> --- qapi/acpi.json | 18 +++++++----------- 1 file changed, 7 insertions(+), 11 deletions(-) diff --git a/qapi/acpi.json b/qapi/acpi.json index XXXXXXX..XXXXXXX 100644 --- a/qapi/acpi.json +++ b/qapi/acpi.json @@ -XXX,XX +XXX,XX @@ ## # @AcpiTableOptions: -# -# Specify an ACPI table on the command line to load. +# Specify an ACPI table on the command line to load. # # At most one of @file and @data can be specified. The list of files # specified by any one of them is loaded and concatenated in order. @@ -XXX,XX +XXX,XX @@ ## # @ACPIOSTInfo: -# -# OSPM Status Indication for a device. For description of possible -# values of @source and @status fields see "_OST (OSPM Status -# Indication)" chapter of ACPI5.0 spec. +# OSPM Status Indication for a device. For description +# of possible values of @source and @status fields see "_OST (OSPM +# Status Indication)" chapter of ACPI5.0 spec. # # @device: device ID associated with slot # @@ -XXX,XX +XXX,XX @@ ## # @query-acpi-ospm-status: -# -# Return a list of `ACPIOSTInfo` for devices that support status -# reporting via ACPI _OST method. +# Return a list of `ACPIOSTInfo` for devices +# that support status reporting via ACPI _OST method. # # Since: 2.1 # @@ -XXX,XX +XXX,XX @@ ## # @ACPI_DEVICE_OST: -# -# Emitted when guest executes ACPI _OST method. +# Emitted when guest executes ACPI _OST method. # # @info: OSPM Status Indication # -- 2.54.0 Signed-off-by: John Snow <jsnow@redhat.com> --- qapi/audio.json | 88 ++++++++++++++++++------------------------------- 1 file changed, 32 insertions(+), 56 deletions(-) diff --git a/qapi/audio.json b/qapi/audio.json index XXXXXXX..XXXXXXX 100644 --- a/qapi/audio.json +++ b/qapi/audio.json @@ -XXX,XX +XXX,XX @@ ## # @AudiodevPerDirectionOptions: -# -# General audio backend options that are used for both playback and -# recording. +# General audio backend options that are used for both playback +# and recording. # # @mixing-engine: use QEMU's mixing engine to mix all streams inside # QEMU and convert audio formats when not supported by the @@ -XXX,XX +XXX,XX @@ ## # @AudiodevGenericOptions: -# -# Generic driver-specific options. +# Generic driver-specific options. # # @in: options of the capture stream # @@ -XXX,XX +XXX,XX @@ ## # @AudiodevDBusOptions: -# -# Options of the D-Bus audio backend. +# Options of the D-Bus audio backend. # # @in: options of the capture stream # @@ -XXX,XX +XXX,XX @@ ## # @AudiodevAlsaPerDirectionOptions: -# -# Options of the ALSA backend that are used for both playback and -# recording. +# Options of the ALSA backend that are used for both playback and +# recording. # # @dev: the name of the ALSA device to use (default 'default') # @@ -XXX,XX +XXX,XX @@ ## # @AudiodevAlsaOptions: -# -# Options of the ALSA audio backend. +# Options of the ALSA audio backend. # # @in: options of the capture stream # @@ -XXX,XX +XXX,XX @@ ## # @AudiodevSndioOptions: -# -# Options of the sndio audio backend. +# Options of the sndio audio backend. # # @in: options of the capture stream # @@ -XXX,XX +XXX,XX @@ ## # @AudiodevCoreaudioPerDirectionOptions: -# -# Options of the Core Audio backend that are used for both playback -# and recording. +# Options of the Core Audio backend that are used for both +# playback and recording. # # @buffer-count: number of buffers # @@ -XXX,XX +XXX,XX @@ ## # @AudiodevCoreaudioOptions: -# -# Options of the coreaudio audio backend. +# Options of the coreaudio audio backend. # # @in: options of the capture stream # @@ -XXX,XX +XXX,XX @@ ## # @AudiodevDsoundOptions: -# -# Options of the DirectSound audio backend. +# Options of the DirectSound audio backend. # # @in: options of the capture stream # @@ -XXX,XX +XXX,XX @@ ## # @AudiodevJackPerDirectionOptions: -# -# Options of the JACK backend that are used for both playback and -# recording. +# Options of the JACK backend that are used for both playback and +# recording. # # @server-name: select from among several possible concurrent server # instances (default: environment variable $JACK_DEFAULT_SERVER if @@ -XXX,XX +XXX,XX @@ ## # @AudiodevJackOptions: -# -# Options of the JACK audio backend. +# Options of the JACK audio backend. # # @in: options of the capture stream # @@ -XXX,XX +XXX,XX @@ ## # @AudiodevOssPerDirectionOptions: -# -# Options of the OSS backend that are used for both playback and -# recording. +# Options of the OSS backend that are used for both playback and +# recording. # # @dev: file name of the OSS device (default '/dev/dsp') # @@ -XXX,XX +XXX,XX @@ ## # @AudiodevOssOptions: -# -# Options of the OSS audio backend. +# Options of the OSS audio backend. # # @in: options of the capture stream # @@ -XXX,XX +XXX,XX @@ ## # @AudiodevPaPerDirectionOptions: -# -# Options of the Pulseaudio backend that are used for both playback -# and recording. +# Options of the Pulseaudio backend that are used for both +# playback and recording. # # @name: name of the sink/source to use # @@ -XXX,XX +XXX,XX @@ ## # @AudiodevPaOptions: -# -# Options of the PulseAudio audio backend. +# Options of the PulseAudio audio backend. # # @in: options of the capture stream # @@ -XXX,XX +XXX,XX @@ ## # @AudiodevPipewirePerDirectionOptions: -# -# Options of the PipeWire backend that are used for both playback and -# recording. +# Options of the PipeWire backend that are used for both playback +# and recording. # # @name: name of the sink/source to use # @@ -XXX,XX +XXX,XX @@ ## # @AudiodevPipewireOptions: -# -# Options of the PipeWire audio backend. +# Options of the PipeWire audio backend. # # @in: options of the capture stream # @@ -XXX,XX +XXX,XX @@ ## # @AudiodevSdlPerDirectionOptions: -# -# Options of the SDL audio backend that are used for both playback and -# recording. +# Options of the SDL audio backend that are used for both playback +# and recording. # # @buffer-count: number of buffers (default 4) # @@ -XXX,XX +XXX,XX @@ ## # @AudiodevSdlOptions: -# -# Options of the SDL audio backend. +# Options of the SDL audio backend. # # @in: options of the recording stream # @@ -XXX,XX +XXX,XX @@ ## # @AudiodevWavOptions: -# -# Options of the wav audio backend. +# Options of the wav audio backend. # # @in: options of the capture stream # @@ -XXX,XX +XXX,XX @@ ## # @AudioFormat: -# -# An enumeration of possible audio formats. +# An enumeration of possible audio formats. # # @u8: unsigned 8 bit integer # @@ -XXX,XX +XXX,XX @@ ## # @AudiodevDriver: -# -# An enumeration of possible audio backend drivers. +# An enumeration of possible audio backend drivers. # # @jack: JACK audio backend (since 5.1) # @@ -XXX,XX +XXX,XX @@ ## # @Audiodev: -# -# Options of an audio backend. +# Options of an audio backend. # # @id: identifier of the backend # @@ -XXX,XX +XXX,XX @@ ## # @query-audiodevs: -# -# Return information about audiodev configuration +# Return information about audiodev configuration # # Since: 8.0 ## -- 2.54.0 Hiya, this is a series that explores a potential syntax for a designated "Intro" section. Markus knows why I want this, but for everyone else: a designated "Introduction" section is useful for the desired "inliner" feature for the new QAPI doc system. Commits explain a bit more. This is prep work and doesn't really change anything tangibly except source code syntax for the QAPI docs. It is designed so that this conversion can happen incrementally with no actual difference to the rendered manuals, so each QAPI module can be converted one at a time for easier review and merging in an arbitrary order. V4: A few minor comment touch-ups; omitted module conversions. V3: changed intro section syntax, made various fixes and changes; I'm going to be honest Markus I did not keep a strict account this time ;_; V2: punt intro sections to the subsequent line and adjust the parser accordingly. Left as an RFC patch that undoes some things this series does instead of properly factoring into the series. John Snow (13): python: temporarily restrict max mypy version tests/qapi: generate output in source order qapi/docs: remove unused QAPIDoc subsection members qapi/docs: add has_features property qapi/docs: make remaining subsection members "private" qapi/docs: fix comment phrasing qapi/docs: add "Intro" section qapi/parser: move _insert_near_kind() method qapi/parser: add mea culpa comment for ensure_returns qapi/docs: adjust stub member insertion algorithm qapi/docs: remove implicit Plain section qapi/docs: add rendering for INTRO sections qapi/docs: add "Intro" section parsing docs/devel/qapi-code-gen.rst | 16 ++-- docs/sphinx/qapidoc.py | 61 +++++++----- python/setup.cfg | 3 +- scripts/qapi/parser.py | 167 +++++++++++++++++++-------------- tests/qapi-schema/doc-good.out | 86 +++++++++-------- tests/qapi-schema/test-qapi.py | 14 +-- 6 files changed, 197 insertions(+), 150 deletions(-) -- 2.54.0 The newest versions of mypy do not support targeting Python 3.9, which we still support. I want to address that soon, but in the meantime it's nice if the tests pass. Signed-off-by: John Snow <jsnow@redhat.com> --- python/setup.cfg | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/python/setup.cfg b/python/setup.cfg index XXXXXXX..XXXXXXX 100644 --- a/python/setup.cfg +++ b/python/setup.cfg @@ -XXX,XX +XXX,XX @@ devel = flake8 >= 5.0.4 fusepy >= 2.0.4 isort >= 5.6.0 - mypy >= 1.4.0 + # Max ver is temporary until we drop 3.9 + mypy >= 1.4.0, <2.0.0 pylint >= 2.17.3 pylint != 3.2.4; python_version<"3.9" pytest >= 6.0.2 -- 2.54.0 Rewrite the test doc generator to produce output in source order instead of arbitrarily by section name. This patch removes our last use of the "body" field, which has an effect on how the sections of each test documention block are printed. We now print the name of the section followed by the section text for all sections except Members and Features, which are printed as "Member=%s" or "Feature=%s" followed by the section text, respectively. This patch is motivated by a desire to move the QAPIDoc API away from named fields for specific sections in a bid to force all users to simply iterate through all_sections in order, instead - and to remove the named subsections. Signed-off-by: John Snow <jsnow@redhat.com> --- tests/qapi-schema/doc-good.out | 82 +++++++++++++++++----------------- tests/qapi-schema/test-qapi.py | 14 +++--- 2 files changed, 48 insertions(+), 48 deletions(-) diff --git a/tests/qapi-schema/doc-good.out b/tests/qapi-schema/doc-good.out index XXXXXXX..XXXXXXX 100644 --- a/tests/qapi-schema/doc-good.out +++ b/tests/qapi-schema/doc-good.out @@ -XXX,XX +XXX,XX @@ event EVT_BOXED Object boxed=True feature feat3 doc freeform - body= + Plain ******* Section ******* doc freeform - body= + Plain Just text, no heading. doc freeform - body= + Plain Subsection ========== @@ -XXX,XX +XXX,XX @@ Examples: - *verbatim* - {braces} doc symbol=Enum - body= + Plain - arg=one + Member=one The _one_ {and only}, description on the same line - arg=two + Member=two - feature=enum-feat + Feature=enum-feat Also _one_ {and only} - feature=enum-member-feat + Feature=enum-member-feat a member feature - section=Plain + Plain @two is undocumented doc symbol=Base - body= + Plain - arg=base1 + Member=base1 description starts on a new line, minimally indented doc symbol=Variant1 - body= + Plain A paragraph Another paragraph @var1 is undocumented - arg=var1 + Member=var1 - feature=variant1-feat + Feature=variant1-feat a feature - feature=member-feat + Feature=member-feat a member feature doc symbol=Variant2 - body= + Plain doc symbol=Object - body= + Plain - feature=union-feat1 + Feature=union-feat1 a feature doc symbol=Alternate - body= + Plain - arg=i + Member=i description starts on the same line remainder indented the same @b is undocumented - arg=b + Member=b - feature=alt-feat + Feature=alt-feat a feature doc freeform - body= + Plain Another subsection ================== doc symbol=cmd - body= + Plain - arg=arg1 + Member=arg1 description starts on a new line, indented - arg=arg2 + Member=arg2 description starts on the same line remainder indented differently - arg=arg3 + Member=arg3 - feature=cmd-feat1 + Feature=cmd-feat1 a feature - feature=cmd-feat2 + Feature=cmd-feat2 another feature - section=Plain + Plain .. note:: @arg3 is undocumented - section=Returns + Returns @Object - section=Errors + Errors some - section=Todo + Todo frobnicate - section=Plain + Plain .. admonition:: Notes - Lorem ipsum dolor sit amet @@ -XXX,XX +XXX,XX @@ Examples:: Note:: Ceci n'est pas une note - section=Since + Since 2.10 doc symbol=cmd-boxed - body= + Plain If you're bored enough to read this, go see a video of boxed cats - feature=cmd-feat1 + Feature=cmd-feat1 a feature - feature=cmd-feat2 + Feature=cmd-feat2 another feature - section=Plain + Plain .. qmp-example:: -> "this example" <- ... has no title ... doc symbol=EVT_BOXED - body= + Plain - feature=feat3 + Feature=feat3 a feature diff --git a/tests/qapi-schema/test-qapi.py b/tests/qapi-schema/test-qapi.py index XXXXXXX..XXXXXXX 100755 --- a/tests/qapi-schema/test-qapi.py +++ b/tests/qapi-schema/test-qapi.py @@ -XXX,XX +XXX,XX @@ from io import StringIO from qapi.error import QAPIError +from qapi.parser import QAPIDoc from qapi.schema import QAPISchema, QAPISchemaVisitor @@ -XXX,XX +XXX,XX @@ def test_frontend(fname): print('doc symbol=%s' % doc.symbol) else: print('doc freeform') - print(' body=\n%s' % doc.body.text) - for arg, section in doc.args.items(): - print(' arg=%s\n%s' % (arg, section.text)) - for feat, section in doc.features.items(): - print(' feature=%s\n%s' % (feat, section.text)) - for section in doc.sections: - print(' section=%s\n%s' % (section.kind, section.text)) + for section in doc.all_sections: + if isinstance(section, QAPIDoc.ArgSection): + print(' %s=%s' % (section.kind, section.name)) + else: + print(' %s' % section.kind) + print(section.text) def open_test_result(dir_name, file_name, update): -- 2.54.0 All users of the QAPIDoc object should be iterating over all_sections and not grabbing arbitrary subsections, if possible. Remove the 'body' and 'sections' members, as they are no longer used. Signed-off-by: John Snow <jsnow@redhat.com> Reviewed-by: Markus Armbruster <armbru@redhat.com> --- scripts/qapi/parser.py | 6 ------ 1 file changed, 6 deletions(-) diff --git a/scripts/qapi/parser.py b/scripts/qapi/parser.py index XXXXXXX..XXXXXXX 100644 --- a/scripts/qapi/parser.py +++ b/scripts/qapi/parser.py @@ -XXX,XX +XXX,XX @@ def __init__(self, info: QAPISourceInfo, symbol: Optional[str] = None): self.all_sections: List[QAPIDoc.Section] = [ QAPIDoc.Section(info, QAPIDoc.Kind.PLAIN) ] - # the body section - self.body: Optional[QAPIDoc.Section] = self.all_sections[0] # dicts mapping parameter/feature names to their description self.args: Dict[str, QAPIDoc.ArgSection] = {} self.features: Dict[str, QAPIDoc.ArgSection] = {} @@ -XXX,XX +XXX,XX @@ def __init__(self, info: QAPISourceInfo, symbol: Optional[str] = None): self.errors: Optional[QAPIDoc.Section] = None # "Since" section self.since: Optional[QAPIDoc.Section] = None - # sections other than .body, .args, .features - self.sections: List[QAPIDoc.Section] = [] def end(self) -> None: for section in self.all_sections: @@ -XXX,XX +XXX,XX @@ def ensure_untagged_section(self, info: QAPISourceInfo) -> None: # start new section section = self.Section(info, kind) - self.sections.append(section) self.all_sections.append(section) def new_tagged_section( @@ -XXX,XX +XXX,XX @@ def new_tagged_section( raise QAPISemError( info, "duplicated '%s' section" % kind) self.since = section - self.sections.append(section) self.all_sections.append(section) def _new_description( -- 2.54.0 Begin preparing to remove public access to the .features member by introducing a semantic "has features" property instead. Signed-off-by: John Snow <jsnow@redhat.com> --- scripts/qapi/parser.py | 8 ++++++-- 1 file changed, 6 insertions(+), 2 deletions(-) diff --git a/scripts/qapi/parser.py b/scripts/qapi/parser.py index XXXXXXX..XXXXXXX 100644 --- a/scripts/qapi/parser.py +++ b/scripts/qapi/parser.py @@ -XXX,XX +XXX,XX @@ def get_doc(self) -> 'QAPIDoc': break # Non-blank line, first of a section if line == 'Features:': - if doc.features: + if doc.has_features: raise QAPIParseError( self, "duplicated 'Features:' line") self.accept(False) @@ -XXX,XX +XXX,XX @@ def get_doc(self) -> 'QAPIDoc': if text: doc.append_line(text) line = self.get_doc_indented(doc) - if not doc.features: + if not doc.has_features: raise QAPIParseError( self, 'feature descriptions expected') no_more_args = True @@ -XXX,XX +XXX,XX @@ def __init__(self, info: QAPISourceInfo, symbol: Optional[str] = None): # "Since" section self.since: Optional[QAPIDoc.Section] = None + @property + def has_features(self) -> bool: + return bool(self.features) + def end(self) -> None: for section in self.all_sections: section.text = section.text.strip('\n') -- 2.54.0 These fields are used to provide error checking and internal logistics and should not be used by a user of the library to directly access documentation sections, so make them private. The "since" field alone is left public, as the qapidoc generator does use this field to pull that section out of the regular flow of the document. Signed-off-by: John Snow <jsnow@redhat.com> --- scripts/qapi/parser.py | 48 +++++++++++++++++++++--------------------- 1 file changed, 24 insertions(+), 24 deletions(-) diff --git a/scripts/qapi/parser.py b/scripts/qapi/parser.py index XXXXXXX..XXXXXXX 100644 --- a/scripts/qapi/parser.py +++ b/scripts/qapi/parser.py @@ -XXX,XX +XXX,XX @@ def __init__(self, info: QAPISourceInfo, symbol: Optional[str] = None): QAPIDoc.Section(info, QAPIDoc.Kind.PLAIN) ] # dicts mapping parameter/feature names to their description - self.args: Dict[str, QAPIDoc.ArgSection] = {} - self.features: Dict[str, QAPIDoc.ArgSection] = {} + self._args: Dict[str, QAPIDoc.ArgSection] = {} + self._features: Dict[str, QAPIDoc.ArgSection] = {} # a command's "Returns" and "Errors" section - self.returns: Optional[QAPIDoc.Section] = None - self.errors: Optional[QAPIDoc.Section] = None + self._returns: Optional[QAPIDoc.Section] = None + self._errors: Optional[QAPIDoc.Section] = None # "Since" section self.since: Optional[QAPIDoc.Section] = None @property def has_features(self) -> bool: - return bool(self.features) + return bool(self._features) def end(self) -> None: for section in self.all_sections: @@ -XXX,XX +XXX,XX @@ def new_tagged_section( ) -> None: section = self.Section(info, kind) if kind == QAPIDoc.Kind.RETURNS: - if self.returns: + if self._returns: raise QAPISemError( info, "duplicated '%s' section" % kind) - self.returns = section + self._returns = section elif kind == QAPIDoc.Kind.ERRORS: - if self.errors: + if self._errors: raise QAPISemError( info, "duplicated '%s' section" % kind) - self.errors = section + self._errors = section elif kind == QAPIDoc.Kind.SINCE: if self.since: raise QAPISemError( @@ -XXX,XX +XXX,XX @@ def _new_description( desc[name] = section def new_argument(self, info: QAPISourceInfo, name: str) -> None: - self._new_description(info, name, QAPIDoc.Kind.MEMBER, self.args) + self._new_description(info, name, QAPIDoc.Kind.MEMBER, self._args) def new_feature(self, info: QAPISourceInfo, name: str) -> None: - self._new_description(info, name, QAPIDoc.Kind.FEATURE, self.features) + self._new_description(info, name, QAPIDoc.Kind.FEATURE, self._features) def append_line(self, line: str) -> None: self.all_sections[-1].append_line(line) def connect_member(self, member: 'QAPISchemaMember') -> None: - if member.name not in self.args: + if member.name not in self._args: assert member.info if self.symbol not in member.info.pragma.documentation_exceptions: raise QAPISemError(member.info, @@ -XXX,XX +XXX,XX @@ def connect_member(self, member: 'QAPISchemaMember') -> None: section = QAPIDoc.ArgSection( self.info, QAPIDoc.Kind.MEMBER, member.name) - self.args[member.name] = section + self._args[member.name] = section # Determine where to insert stub doc - it should go at the # end of the members section(s), if any. Note that index 0 @@ -XXX,XX +XXX,XX @@ def connect_member(self, member: 'QAPISchemaMember') -> None: index += 1 self.all_sections.insert(index, section) - self.args[member.name].connect(member) + self._args[member.name].connect(member) def connect_feature(self, feature: 'QAPISchemaFeature') -> None: - if feature.name not in self.features: + if feature.name not in self._features: raise QAPISemError(feature.info, "feature '%s' lacks documentation" % feature.name) - self.features[feature.name].connect(feature) + self._features[feature.name].connect(feature) def ensure_returns(self, info: QAPISourceInfo) -> None: @@ -XXX,XX +XXX,XX @@ def _insert_near_kind( def check_expr(self, expr: QAPIExpression) -> None: if 'command' in expr: - if self.returns and 'returns' not in expr: + if self._returns and 'returns' not in expr: raise QAPISemError( - self.returns.info, + self._returns.info, "'Returns' section, but command doesn't return anything") else: - if self.returns: + if self._returns: raise QAPISemError( - self.returns.info, + self._returns.info, "'Returns' section is only valid for commands") - if self.errors: + if self._errors: raise QAPISemError( - self.errors.info, + self._errors.info, "'Errors' section is only valid for commands") def check(self) -> None: @@ -XXX,XX +XXX,XX @@ def check_args_section( "do" if len(bogus) > 1 else "does" )) - check_args_section(self.args, 'member') - check_args_section(self.features, 'feature') + check_args_section(self._args, 'member') + check_args_section(self._features, 'feature') -- 2.54.0 Plaintext sections can be one or more paragraphs, the original comment was a mistake. Signed-off-by: John Snow <jsnow@redhat.com> --- scripts/qapi/parser.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/scripts/qapi/parser.py b/scripts/qapi/parser.py index XXXXXXX..XXXXXXX 100644 --- a/scripts/qapi/parser.py +++ b/scripts/qapi/parser.py @@ -XXX,XX +XXX,XX @@ def get_doc(self) -> 'QAPIDoc': line = self.get_doc_indented(doc) no_more_args = True else: - # plain paragraph + # plain paragraph(s) doc.ensure_untagged_section(self.info) doc.append_line(line) line = self.get_doc_paragraph(doc) -- 2.54.0 This patch adds an "Intro" doc section, intended to eventually replace the "Plain" doc section alongside a forthcoming "Details" section. For now, this section is not actually instantiated or used, but subsequent patches will slowly convert the leading introductory sections of QAPIDoc documentation to use this new section. A main motivation of this series of changes is to more explicitly delineate the "Introductory" documentation for each QAPI definition for the sake of the inliner. When inlining members, examples, and details from multiple QAPIDoc sections, we will want to omit the "Introductory" text from inlined definitions while keeping notes, caution boxes, examples, and so on. This new section facilitates that. Signed-off-by: John Snow <jsnow@redhat.com> --- docs/sphinx/qapidoc.py | 2 +- scripts/qapi/parser.py | 3 ++- 2 files changed, 3 insertions(+), 2 deletions(-) diff --git a/docs/sphinx/qapidoc.py b/docs/sphinx/qapidoc.py index XXXXXXX..XXXXXXX 100644 --- a/docs/sphinx/qapidoc.py +++ b/docs/sphinx/qapidoc.py @@ -XXX,XX +XXX,XX @@ def visit_sections(self, ent: QAPISchemaDefinition) -> None: for i, section in enumerate(sections): section.text = self.reformat_arobase(section.text) - if section.kind == QAPIDoc.Kind.PLAIN: + if section.kind.name in ("PLAIN", "INTRO"): self.visit_paragraph(section) elif section.kind == QAPIDoc.Kind.MEMBER: assert isinstance(section, QAPIDoc.ArgSection) diff --git a/scripts/qapi/parser.py b/scripts/qapi/parser.py index XXXXXXX..XXXXXXX 100644 --- a/scripts/qapi/parser.py +++ b/scripts/qapi/parser.py @@ -XXX,XX +XXX,XX @@ class Kind(enum.Enum): ERRORS = 4 SINCE = 5 TODO = 6 + INTRO = 7 @staticmethod def from_string(kind: str) -> 'QAPIDoc.Kind': @@ -XXX,XX +XXX,XX @@ def has_features(self) -> bool: def end(self) -> None: for section in self.all_sections: section.text = section.text.strip('\n') - if section.kind != QAPIDoc.Kind.PLAIN and section.text == '': + if not (section.kind.name in ("INTRO", "PLAIN") or section.text): raise QAPISemError( section.info, "text required after '%s:'" % section.kind) -- 2.54.0 Move this function out from underneath `ensure_returns` and make it available for general purpose use as an object method instead. This is purely a scope-level patch with no functional changes. Signed-off-by: John Snow <jsnow@redhat.com> --- scripts/qapi/parser.py | 33 +++++++++++++++++---------------- 1 file changed, 17 insertions(+), 16 deletions(-) diff --git a/scripts/qapi/parser.py b/scripts/qapi/parser.py index XXXXXXX..XXXXXXX 100644 --- a/scripts/qapi/parser.py +++ b/scripts/qapi/parser.py @@ -XXX,XX +XXX,XX @@ def new_feature(self, info: QAPISourceInfo, name: str) -> None: def append_line(self, line: str) -> None: self.all_sections[-1].append_line(line) + def _insert_near_kind( + self, + kind: 'QAPIDoc.Kind', + new_sect: 'QAPIDoc.Section', + after: bool = False, + ) -> bool: + """Insert or append a new doc section at a specific point.""" + for idx, sect in enumerate(reversed(self.all_sections)): + if sect.kind == kind: + pos = len(self.all_sections) - idx - 1 + if after: + pos += 1 + self.all_sections.insert(pos, new_sect) + return True + return False + def connect_member(self, member: 'QAPISchemaMember') -> None: if member.name not in self._args: assert member.info @@ -XXX,XX +XXX,XX @@ def connect_feature(self, feature: 'QAPISchemaFeature') -> None: self._features[feature.name].connect(feature) def ensure_returns(self, info: QAPISourceInfo) -> None: - - def _insert_near_kind( - kind: QAPIDoc.Kind, - new_sect: QAPIDoc.Section, - after: bool = False, - ) -> bool: - for idx, sect in enumerate(reversed(self.all_sections)): - if sect.kind == kind: - pos = len(self.all_sections) - idx - 1 - if after: - pos += 1 - self.all_sections.insert(pos, new_sect) - return True - return False - if any(s.kind == QAPIDoc.Kind.RETURNS for s in self.all_sections): return # Stub "Returns" section for undocumented returns value stub = QAPIDoc.Section(info, QAPIDoc.Kind.RETURNS) - if any(_insert_near_kind(kind, stub, after) for kind, after in ( + if any(self._insert_near_kind(kind, stub, after) for kind, after in ( # 1. If arguments, right after those. (QAPIDoc.Kind.MEMBER, True), # 2. Elif errors, right *before* those. -- 2.54.0 These algorithms are quite a mess currently, but they are temporary until we add the inliner which will address these issues more holistically. For now, add the "mea culpa". Signed-off-by: John Snow <jsnow@redhat.com> --- scripts/qapi/parser.py | 7 +++++++ 1 file changed, 7 insertions(+) diff --git a/scripts/qapi/parser.py b/scripts/qapi/parser.py index XXXXXXX..XXXXXXX 100644 --- a/scripts/qapi/parser.py +++ b/scripts/qapi/parser.py @@ -XXX,XX +XXX,XX @@ def connect_feature(self, feature: 'QAPISchemaFeature') -> None: self._features[feature.name].connect(feature) def ensure_returns(self, info: QAPISourceInfo) -> None: + # This is more complicated than it ought to be. The doc + # parser should already know where a generated RETURNS section + # should go. It currently doesn't, mostly because it accepts + # tagged sections in any order. + # + # TODO: Tighten doc syntax and simplify. + if any(s.kind == QAPIDoc.Kind.RETURNS for s in self.all_sections): return -- 2.54.0 A forthcoming patch removes the implicit PLAIN section that always starts a QAPIDoc section list. Further future changes begin converting "PLAIN" sections to "INTRO" sections. This will affect the code that inserts "Not documented" descriptions for undocumented members ("stub sections") and the dummy section that marks the spot for "The members of ..." references. Adjust the algorithm to cope with not only the finished state, but temporary intermediate states while the series is merged. This algorithm can handle zero-or-more PLAIN *or* INTRO sections at the beginning of a QAPIDoc object, in contrast to the previous algorithm which assumed and relied upon there being always one PLAIN section at the beginning of every QAPIDoc section list. In other words: (PLAIN | INTRO)* <EverythingElse> This does not impact what the parser itself will actually produce. As of this patch, the parser will still always generate QAPIDoc section lists that start with precisely one PLAIN section (whether or not it is empty), followed by the remaining sections. Those remaining sections may or may not include additional PLAIN sections, but never two such sections contiguously as the parser will always treat that layout as one PLAIN section consisting of multiple paragraph(s). In other other words: This insertion algorithm is more lenient than the parser, but this is on purpose for flexibility mid-stream as we convert QAPI to using explicit introductory sections. The allowed order of sections will eventually become strictly enforced in the parser, which will in turn allow dramatic simplifications to the insertion algorithm. This only exists as transitory code until we are able to enforce that order. Fear not: the intermediate ReST output before and after this patch are byte identical, so failing all else, we at least know it doesn't make anything worse. Lastly, because we have three places in the code that need to insert stub/dummy sections, we take the opportunity to consolidate this code to handle all three cases with one function. This winds up necessitating the qapidoc.py generator actually modify the section list to insert a "dummy" member that acts as a placeholder for "The members of ..." text. While it looks like a code smell to modify the caller's argument, it is ultimately safe because the QAPI Schema object is re-parsed and re-constructed in memory for each individual process that needs to operate on it. In other words, the Sphinx document generator already does have "its own copy" of the section lists, so it is "safe" to modify here without regards to other consumers of the QAPIDoc objects. It only *looks* like it smells bad. Ultimately, this code will also be removed once the inliner is merged, so it is only a temporary aesthetic issue regardless. That's my story and I'm sticking to it. Signed-off-by: John Snow <jsnow@redhat.com> --- docs/sphinx/qapidoc.py | 42 ++++++++++++++++-------------- scripts/qapi/parser.py | 58 +++++++++++++++++++++++++++++------------- 2 files changed, 64 insertions(+), 36 deletions(-) diff --git a/docs/sphinx/qapidoc.py b/docs/sphinx/qapidoc.py index XXXXXXX..XXXXXXX 100644 --- a/docs/sphinx/qapidoc.py +++ b/docs/sphinx/qapidoc.py @@ -XXX,XX +XXX,XX @@ def _get_target( ) def visit_sections(self, ent: QAPISchemaDefinition) -> None: + # Generate a placeholder right after the member section(s) which + # may be used to generate documentation for "The members of..." + # pointers in the rendered document. + # + # This is a temporary hack until the inliner is merged. Note + # that although we modify the caller's section list, the + # Sphinx document generator has its own copy of the parsed + # schema in memory, so this action does not interfere with + # other users of the QAPISchema or QAPIDoc objects outside of + # the document generator. Fishy, but not harmful. + if ent.doc: + ent.doc.append_member_stub( + QAPIDoc.ArgSection( + ent.doc.info, QAPIDoc.Kind.MEMBER, "q_dummy" + ) + ) + sections = ent.doc.all_sections if ent.doc else [] - # Determine the index location at which we should generate - # documentation for "The members of ..." pointers. This should - # go at the end of the members section(s) if any. Note that - # index 0 is assumed to be a plain intro section, even if it is - # empty; and that a members section if present will always - # immediately follow the opening PLAIN section. - gen_index = 1 - if len(sections) > 1: - while sections[gen_index].kind == QAPIDoc.Kind.MEMBER: - gen_index += 1 - if gen_index >= len(sections): - break - # Add sections in source order: - for i, section in enumerate(sections): + for section in sections: section.text = self.reformat_arobase(section.text) if section.kind.name in ("PLAIN", "INTRO"): self.visit_paragraph(section) elif section.kind == QAPIDoc.Kind.MEMBER: assert isinstance(section, QAPIDoc.ArgSection) - self.visit_member(section) + if section.name == "q_dummy": + # Generate "The members of ..." entries if necessary + self._insert_member_pointer(ent) + else: + self.visit_member(section) elif section.kind == QAPIDoc.Kind.FEATURE: assert isinstance(section, QAPIDoc.ArgSection) self.visit_feature(section) @@ -XXX,XX +XXX,XX @@ def visit_sections(self, ent: QAPISchemaDefinition) -> None: else: assert False - # Generate "The members of ..." entries if necessary: - if i == gen_index - 1: - self._insert_member_pointer(ent) - self.ensure_blank_line() # Transmogrification core methods diff --git a/scripts/qapi/parser.py b/scripts/qapi/parser.py index XXXXXXX..XXXXXXX 100644 --- a/scripts/qapi/parser.py +++ b/scripts/qapi/parser.py @@ -XXX,XX +XXX,XX @@ def _insert_near_kind( return True return False + def _insert_after_intro( + self, + section: 'QAPIDoc.Section', + ) -> None: + """ + Insert a section immediately after the intro section. + + While we convert PLAIN sections to INTRO sections, all + contiguous INTRO/PLAIN sections at the start of a QAPIDoc + section list are treated as "the intro". + + Once INTRO conversion is complete, this helper will no longer be + needed and ``_insert_near_kind(QAPIDoc.Kind.INTRO, ...)`` will + be sufficient. + """ + index = 0 + for index, ref_section in enumerate(self.all_sections): + if ref_section.kind.name in ("PLAIN", "INTRO"): + continue + break + else: + index += 1 + + self.all_sections.insert(index, section) + + def append_member_stub(self, stub: 'QAPIDoc.Section') -> None: + + """ + Append a stub section after any Member sections. + """ + if self._insert_near_kind(QAPIDoc.Kind.MEMBER, stub, True): + return + + # No MEMBER sections present. Insert after INTRO/PLAIN sections. + self._insert_after_intro(stub) + def connect_member(self, member: 'QAPISchemaMember') -> None: if member.name not in self._args: assert member.info @@ -XXX,XX +XXX,XX @@ def connect_member(self, member: 'QAPISchemaMember') -> None: % (member.role, member.name)) # Insert stub documentation section for missing member docs. # TODO: drop when undocumented members are outlawed - - section = QAPIDoc.ArgSection( + stub_section = QAPIDoc.ArgSection( self.info, QAPIDoc.Kind.MEMBER, member.name) - self._args[member.name] = section - - # Determine where to insert stub doc - it should go at the - # end of the members section(s), if any. Note that index 0 - # is assumed to be an untagged intro section, even if it is - # empty. - index = 1 - if len(self.all_sections) > 1: - while self.all_sections[index].kind == QAPIDoc.Kind.MEMBER: - index += 1 - self.all_sections.insert(index, section) + self._args[member.name] = stub_section + self.append_member_stub(stub_section) self._args[member.name].connect(member) @@ -XXX,XX +XXX,XX @@ def ensure_returns(self, info: QAPISourceInfo) -> None: )): return - # Otherwise, it should go right after the intro. The intro - # is always the first section and is always present (even - # when empty), so we can insert directly at index=1 blindly. - self.all_sections.insert(1, stub) + # Otherwise, it should go right after the intro. + self._insert_after_intro(stub) def check_expr(self, expr: QAPIExpression) -> None: if 'command' in expr: -- 2.54.0 Prior to this patch, we always create an empty Plain section. Removing this allows us to gradually phase out the "Plain" section in favor of "Intro" and "Details" sections while keeping "Plain" around for the interim churn during the series - meaning that we don't actually know at __init__ time which type of section we'll have first. Signed-off-by: John Snow <jsnow@redhat.com> --- scripts/qapi/parser.py | 4 +--- tests/qapi-schema/doc-good.out | 14 -------------- 2 files changed, 1 insertion(+), 17 deletions(-) diff --git a/scripts/qapi/parser.py b/scripts/qapi/parser.py index XXXXXXX..XXXXXXX 100644 --- a/scripts/qapi/parser.py +++ b/scripts/qapi/parser.py @@ -XXX,XX +XXX,XX @@ def __init__(self, info: QAPISourceInfo, symbol: Optional[str] = None): # definition doc's symbol, None for free-form doc self.symbol: Optional[str] = symbol # the sections in textual order - self.all_sections: List[QAPIDoc.Section] = [ - QAPIDoc.Section(info, QAPIDoc.Kind.PLAIN) - ] + self.all_sections: List[QAPIDoc.Section] = [] # dicts mapping parameter/feature names to their description self._args: Dict[str, QAPIDoc.ArgSection] = {} self._features: Dict[str, QAPIDoc.ArgSection] = {} diff --git a/tests/qapi-schema/doc-good.out b/tests/qapi-schema/doc-good.out index XXXXXXX..XXXXXXX 100644 --- a/tests/qapi-schema/doc-good.out +++ b/tests/qapi-schema/doc-good.out @@ -XXX,XX +XXX,XX @@ Examples: - *verbatim* - {braces} doc symbol=Enum - Plain - Member=one The _one_ {and only}, description on the same line Member=two @@ -XXX,XX +XXX,XX @@ a member feature Plain @two is undocumented doc symbol=Base - Plain - Member=base1 description starts on a new line, minimally indented @@ -XXX,XX +XXX,XX @@ a feature Feature=member-feat a member feature doc symbol=Variant2 - Plain - doc symbol=Object - Plain - Feature=union-feat1 a feature doc symbol=Alternate - Plain - Member=i description starts on the same line remainder indented the same @@ -XXX,XX +XXX,XX @@ doc freeform Another subsection ================== doc symbol=cmd - Plain - Member=arg1 description starts on a new line, indented @@ -XXX,XX +XXX,XX @@ another feature <- ... has no title ... doc symbol=EVT_BOXED - Plain - Feature=feat3 a feature -- 2.54.0 Amend the qapidoc generator to handle and render INTRO sections. The only real difference here from other sections is that we need to dedent the text so it renders correctly. Members and Features are also indented, but do not require a dedent() because they are always used in tandem with an rST construct that forms the start of a new indented block; there is coincidental harmony. Plaintext sections, however, do not start their own block and thus need to be dedented to prevent accidentally rendering them as a blockquote or a syntax error. This dedent transformation on the text does not reflow the text, so source line information remains accurate, and the "blame" chain of custody for sphinx rST parsing error messages continues to be correct even through this transformation. Signed-off-by: John Snow <jsnow@redhat.com> --- docs/sphinx/qapidoc.py | 17 ++++++++++++++--- 1 file changed, 14 insertions(+), 3 deletions(-) diff --git a/docs/sphinx/qapidoc.py b/docs/sphinx/qapidoc.py index XXXXXXX..XXXXXXX 100644 --- a/docs/sphinx/qapidoc.py +++ b/docs/sphinx/qapidoc.py @@ -XXX,XX +XXX,XX @@ from pathlib import Path import re import sys +import textwrap from typing import TYPE_CHECKING from docutils import nodes @@ -XXX,XX +XXX,XX @@ def add_lines( self, content: str, info: QAPISourceInfo, + dedent: bool = False, ) -> None: lines = content.splitlines(True) + + if dedent: + lines = textwrap.dedent(content).splitlines(True) + else: + lines = content.splitlines(True) + for i, line in enumerate(lines): self.add_line_raw(line, info.fname, info.line + i) @@ -XXX,XX +XXX,XX @@ def reformat_arobase(text: str) -> str: # Transmogrification helpers - def visit_paragraph(self, section: QAPIDoc.Section) -> None: + def visit_plaintext(self, section: QAPIDoc.Section) -> None: # Squelch empty paragraphs. if not section.text: return + # Intro sections, which are indented in QAPI source, need to + # be dedented to avoid accidental block quotes in ReST syntax. + dedent = bool(section.kind == QAPIDoc.Kind.INTRO) self.ensure_blank_line() - self.add_lines(section.text, section.info) + self.add_lines(section.text, section.info, dedent) self.ensure_blank_line() def visit_member(self, section: QAPIDoc.ArgSection) -> None: @@ -XXX,XX +XXX,XX @@ def visit_sections(self, ent: QAPISchemaDefinition) -> None: section.text = self.reformat_arobase(section.text) if section.kind.name in ("PLAIN", "INTRO"): - self.visit_paragraph(section) + self.visit_plaintext(section) elif section.kind == QAPIDoc.Kind.MEMBER: assert isinstance(section, QAPIDoc.ArgSection) if section.name == "q_dummy": -- 2.54.0 Add parsing for explicit Intro section syntax. A side effect of this patch is that we will (currently) always create an empty Intro section, similar to how we used to have an empty Plain section. The tests are adjusted accordingly, rendered document output does not change at all. Signed-off-by: John Snow <jsnow@redhat.com> --- docs/devel/qapi-code-gen.rst | 16 +++++++--------- scripts/qapi/parser.py | 4 ++-- tests/qapi-schema/doc-good.out | 18 ++++++++++++++++++ 3 files changed, 27 insertions(+), 11 deletions(-) diff --git a/docs/devel/qapi-code-gen.rst b/docs/devel/qapi-code-gen.rst index XXXXXXX..XXXXXXX 100644 --- a/docs/devel/qapi-code-gen.rst +++ b/docs/devel/qapi-code-gen.rst @@ -XXX,XX +XXX,XX @@ definition it documents. When documentation is required (see pragma_ 'doc-required'), every definition must have documentation. -Definition documentation starts with a line naming the definition, -followed by an optional overview, a description of each argument (for -commands and events), member (for structs and unions), branch (for -alternates), or value (for enums), a description of each feature (if -any), and finally optional tagged sections. +Definition documentation starts with a description naming the +definition with an optional indented overview, a description of each +argument (for commands and events), member (for structs and unions), +branch (for alternates), or value (for enums), a description of each +feature (if any), and finally optional tagged sections. Descriptions start with '\@name:'. The description text must be indented like this:: @@ -XXX,XX +XXX,XX @@ Examples of complete definition documentation:: ## # @BlockStats: - # - # Statistics of a virtual block device or a block backing device. + # Statistics of a virtual block device or a block backing device. # # @device: If the stats are for a virtual block device, the name # corresponding to the virtual block device. @@ -XXX,XX +XXX,XX @@ Examples of complete definition documentation:: ## # @query-blockstats: - # - # Query the @BlockStats for all virtual block devices. + # Query the @BlockStats for all virtual block devices. # # @query-nodes: If true, the command will query all the block nodes # ... explain, explain ... diff --git a/scripts/qapi/parser.py b/scripts/qapi/parser.py index XXXXXXX..XXXXXXX 100644 --- a/scripts/qapi/parser.py +++ b/scripts/qapi/parser.py @@ -XXX,XX +XXX,XX @@ def get_doc(self) -> 'QAPIDoc': if not symbol: raise QAPIParseError(self, "name required after '@'") doc = QAPIDoc(info, symbol) - self.accept(False) - line = self.get_doc_line() + doc.all_sections.append(QAPIDoc.Section(info, QAPIDoc.Kind.INTRO)) + line = self.get_doc_indented(doc) no_more_args = False while line is not None: diff --git a/tests/qapi-schema/doc-good.out b/tests/qapi-schema/doc-good.out index XXXXXXX..XXXXXXX 100644 --- a/tests/qapi-schema/doc-good.out +++ b/tests/qapi-schema/doc-good.out @@ -XXX,XX +XXX,XX @@ Examples: - *verbatim* - {braces} doc symbol=Enum + Intro + Member=one The _one_ {and only}, description on the same line Member=two @@ -XXX,XX +XXX,XX @@ a member feature Plain @two is undocumented doc symbol=Base + Intro + Member=base1 description starts on a new line, minimally indented doc symbol=Variant1 + Intro + Plain A paragraph @@ -XXX,XX +XXX,XX @@ a feature Feature=member-feat a member feature doc symbol=Variant2 + Intro + doc symbol=Object + Intro + Feature=union-feat1 a feature doc symbol=Alternate + Intro + Member=i description starts on the same line remainder indented the same @@ -XXX,XX +XXX,XX @@ doc freeform Another subsection ================== doc symbol=cmd + Intro + Member=arg1 description starts on a new line, indented @@ -XXX,XX +XXX,XX @@ Note:: Since 2.10 doc symbol=cmd-boxed + Intro + Plain If you're bored enough to read this, go see a video of boxed cats Feature=cmd-feat1 @@ -XXX,XX +XXX,XX @@ another feature <- ... has no title ... doc symbol=EVT_BOXED + Intro + Feature=feat3 a feature -- 2.54.0