John Snow <[email protected]> writes:
> Enforce the relative ordering of "Intro", "Members", "Features",
> "Errors" and "Returns" sections.
>
> Signed-off-by: John Snow <[email protected]>
> ---
> docs/devel/qapi-code-gen.rst | 19 +++++++++++------
> scripts/qapi/parser.py | 41 ++++++++++++++++++++++++++++++------
> 2 files changed, 48 insertions(+), 12 deletions(-)
>
> diff --git a/docs/devel/qapi-code-gen.rst b/docs/devel/qapi-code-gen.rst
> index 3ac1c2dae18..29e9c5a3da6 100644
> --- a/docs/devel/qapi-code-gen.rst
> +++ b/docs/devel/qapi-code-gen.rst
> @@ -985,12 +985,19 @@ When documentation is required (see pragma_
> 'doc-required'), every
> definition must have documentation.
>
> Definition documentation starts with a description naming the
> -definition with an optional indented overview (the "intro"),
> -:ref:`descriptions <doc-descrip>` of each argument (for commands and
> -events), member (for structs and unions), branch (for alternates), or
> -value (for enums), :ref:`feature descriptions <doc-feat>` of each
> -feature (if any), and finally optional :ref:`tagged sections
> -<doc-tagged>`.
> +definition with an optional indented overview (the "intro").
> +
> +The introduction is followed by :ref:`descriptions <doc-descrip>` of
> +each argument (for commands and events), member (for structs and
> +unions), branch (for alternates), or value (for enums).
> +
> +:ref:`Tagged sections <doc-tagged>` follow in order: Returns (for
> +commands), Errors (for commands), and lastly :ref:`feature
> +descriptions <doc-feat>` if applicable.
*Some* of the tagged sections do.
Does this document still need the concept "tagged section"?
> +
> +Definition documentation concludes with rST-formatted detail
> +paragraphs (including examples, notes, &c), and finally an optional
> +"Since" tagged section at the very end.
>
> .. _doc-descrip:
>
> diff --git a/scripts/qapi/parser.py b/scripts/qapi/parser.py
> index fe284a3f866..94a60a23482 100644
> --- a/scripts/qapi/parser.py
> +++ b/scripts/qapi/parser.py
> @@ -545,6 +545,28 @@ def get_doc(self) -> 'QAPIDoc':
> doc.all_sections.append(QAPIDoc.Section(info,
> QAPIDoc.Kind.INTRO))
> line = self.get_doc_indented(doc)
> no_more_args = False
> + last_ordered_section = QAPIDoc.Kind.INTRO
> +
> + def _tag_check(this: Union['QAPIDoc.Kind', str]) -> None:
> + nonlocal last_ordered_section
> + if isinstance(this, str):
> + this = QAPIDoc.Kind.from_string(this)
Simpler, I think: require a .Kind argument, and do the conversion from
string in the one caller that needs it.
> +
> + if this in (QAPIDoc.Kind.TODO, QAPIDoc.Kind.SINCE,
> + QAPIDoc.Kind.DETAILS):
> + return
> +
> + if this.value < last_ordered_section.value:
> + raise QAPIParseError(
> + self,
> + f"'{this}' section cannot appear after "
> + f"'{last_ordered_section}' section, please re-order "
> + "the sections and adjust phrasing as necessary to "
> + "ensure consistent documentation flow between the "
> + "source code and the rendered HTML manual"
> + )
That's a loooong error message. It actually consists of the error
proper and advice on how to correct it. Please use
"<error>\n<advice>"
like we do in expr.py's check_keys(), parser.py's
._validate_doc_line_format(), and schema.py's ._def_definition(). Yes,
the embedded newline is a bit of a hack.
> +
> + last_ordered_section = this
>
> while line is not None:
> # Blank lines
> @@ -558,6 +580,7 @@ def get_doc(self) -> 'QAPIDoc':
> if doc.has_features:
> raise QAPIParseError(
> self, "duplicated 'Features:' line")
> + _tag_check(QAPIDoc.Kind.FEATURE)
> self.accept(False)
> line = self.get_doc_line()
> while line == '':
> @@ -581,6 +604,7 @@ def get_doc(self) -> 'QAPIDoc':
> self,
> "description of '@%s:' follows a section"
> % match.group(1))
> + _tag_check(QAPIDoc.Kind.MEMBER)
> while (line is not None
> and (match := self._match_at_name_colon(line))):
> doc.new_argument(self.info, match.group(1))
> @@ -621,6 +645,7 @@ def get_doc(self) -> 'QAPIDoc':
> )
> raise QAPIParseError(self, emsg)
>
> + _tag_check(match.group(1))
This is the caller that needs it.
> doc.new_tagged_section(
> self.info,
> QAPIDoc.Kind.from_string(match.group(1))
> @@ -632,6 +657,7 @@ def get_doc(self) -> 'QAPIDoc':
> no_more_args = True
> else:
> # plain paragraph(s)
> + _tag_check(QAPIDoc.Kind.DETAILS)
> doc.ensure_untagged_section(self.info)
> doc.append_line(line)
> line = self.get_doc_paragraph(doc)
> @@ -674,14 +700,17 @@ class QAPIDoc:
> """
>
> class Kind(enum.Enum):
> - DETAILS = 0
> + # The order here is the order in which sections must appear in
> + # source code; with the exception of 'TODO' which may appear
> + # anywhere but is treated as a comment.
> + INTRO = 0
> MEMBER = 1
> - FEATURE = 2
> - RETURNS = 3
> - ERRORS = 4
> - SINCE = 5
> + RETURNS = 2
> + ERRORS = 3
> + FEATURE = 4
> + DETAILS = 5
> TODO = 6
> - INTRO = 7
> + SINCE = 7
>
> @staticmethod
> def from_string(kind: str) -> 'QAPIDoc.Kind':
Missing: negative test for the new error.
