Re: [PATCH v2 07/13] qapi/docs: add "Intro" section parsing

[Markus Armbruster <armbru@redhat.com>]

John Snow <jsnow@redhat.com> writes:

> Add parsing for explicit Intro section syntax.
>
> A side effect of this patch is that we will 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            | 17 +++++----
>  scripts/qapi/parser.py                  | 47 ++++++++++++++++++-------
>  tests/qapi-schema/doc-good.out          | 18 ++++++++++
>  tests/qapi-schema/doc-missing-colon.err |  2 +-
>  4 files changed, 64 insertions(+), 20 deletions(-)
>
> diff --git a/docs/devel/qapi-code-gen.rst b/docs/devel/qapi-code-gen.rst
> index 3a632b4a648..a8175934d52 100644
> --- a/docs/devel/qapi-code-gen.rst
> +++ b/docs/devel/qapi-code-gen.rst
> @@ -862,7 +862,7 @@ documentation comment.
>  If the documentation comment starts like ::
>  
>      ##
> -    # @SYMBOL:
> +    # @SYMBOL: [...]
>  
>  it documents the definition of SYMBOL, else it's free-form
>  documentation.
> @@ -984,11 +984,11 @@ 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 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::
> @@ -996,6 +996,11 @@ indented like this::
>   # @name: Lorem ipsum dolor sit amet, consectetur adipiscing elit, sed
>   #     do eiusmod tempor incididunt ut labore et dolore magna aliqua.
>  
> +Definition descriptions are special: the optional introductory overview
> +describing the definition will not be inlined when referenced by other
> +definitions (such as when using 'base' to include members from another
> +definition), while other descriptions and tagged sections will be.
> +

Isn't this premature?  We're not inlining anything just yet.

>  .. FIXME The parser accepts these things in almost any order.
>  
>  .. FIXME union branches should be described, too.
> diff --git a/scripts/qapi/parser.py b/scripts/qapi/parser.py
> index 6612f471bb8..c23fd26aaa7 100644
> --- a/scripts/qapi/parser.py
> +++ b/scripts/qapi/parser.py
> @@ -24,6 +24,7 @@
>      Match,
>      Optional,
>      Set,
> +    Tuple,
>      Union,
>  )
>  
> @@ -524,6 +525,30 @@ def get_doc_paragraph(self, doc: 'QAPIDoc') -> Optional[str]:
>                  return line
>              doc.append_line(line)
>  
> +    def _get_doc_intro(
> +        self,
> +        line: str,
> +        info: QAPISourceInfo
> +    ) -> Tuple['QAPIDoc', Optional[str]]:
> +        match = self._match_at_name_colon(line)
> +        if not match:
> +            raise QAPIParseError(self, "@name must end with ':'")
> +
> +        # Invalid names are not checked here, but the name
> +        # provided *must* match the following definition,
> +        # which *is* validated in expr.py.
> +        symbol = match.group(1)
> +        if not symbol:
> +            raise QAPIParseError(self, "name required after '@'")
> +        doc = QAPIDoc(info, symbol)
> +

Drop the blank line.  For what it's worth, you do it already in PATCH
13.

> +        doc.ensure_untagged_section(info, QAPIDoc.Kind.INTRO)

Hmm.

.ensure_untagged_section() extends the current section if we have one
and it is of kind .INTRO, else creates a new one of kind .INTRO.

But there is no current section here!  Why not simply create one?

> +        text = line[match.end():]
> +        if text:
> +            doc.append_line(text)
> +
> +        return doc, self.get_doc_indented(doc)
> +
>      def get_doc(self) -> 'QAPIDoc':
>          if self.val != '##':
>              raise QAPIParseError(
> @@ -532,18 +557,9 @@ def get_doc(self) -> 'QAPIDoc':
>          self.accept(False)
>          line = self.get_doc_line()
>          if line is not None and line.startswith('@'):
> +

Drop the blank line, please.

>              # Definition documentation
> -            if not line.endswith(':'):
> -                raise QAPIParseError(self, "line should end with ':'")
> -            # Invalid names are not checked here, but the name
> -            # provided *must* match the following definition,
> -            # which *is* validated in expr.py.
> -            symbol = line[1:-1]
> -            if not symbol:
> -                raise QAPIParseError(self, "name required after '@'")
> -            doc = QAPIDoc(info, symbol)
> -            self.accept(False)
> -            line = self.get_doc_line()
> +            doc, line = self._get_doc_intro(line, info)
>              no_more_args = False
>  
>              while line is not None:
> @@ -751,8 +767,13 @@ def end(self) -> None:
>                  raise QAPISemError(
>                      section.info, "text required after '%s:'" % section.kind)
>  
> -    def ensure_untagged_section(self, info: QAPISourceInfo) -> None:
> -        kind = QAPIDoc.Kind.PLAIN
> +    def ensure_untagged_section(
> +        self,
> +        info: QAPISourceInfo,
> +        kind: Optional['QAPIDoc.Kind'] = None,

Why not = QAPIDoc.Kind.PLAIN here, so ...

> +    ) -> None:
> +        if kind is None:
> +            kind = QAPIDoc.Kind.PLAIN

... you don't need this?

>  
>          if self.all_sections and self.all_sections[-1].kind == kind:
>              # extend current section

Hmm.

QAPIDoc.ensure_untagged_section() extends the current section if we have
one and it's "untagged", else creates a new "untagged" section.

In the beginning, there was just one kind of "untagged" section.  Its
encoding eventually became "Section.kind is QAPIDoc.Kind.PLAIN".  So
what the function does was clear enough.

This patch adds a second kind: QAPIDoc.Kind.INTRO.
.ensure_untagged_section() now takes a kind argument.  It ensures *that*
kind of section.

I think the function name becomes misleading.  It can now ensure *any*
kind of section, not just "untagged".

Rename to ensure_section() and make the @kind argument mandatory?

> diff --git a/tests/qapi-schema/doc-good.out b/tests/qapi-schema/doc-good.out
> index 6fcc8175cfe..bc89853765f 100644
> --- a/tests/qapi-schema/doc-good.out
> +++ b/tests/qapi-schema/doc-good.out
> @@ -106,6 +106,8 @@ Examples:
>  - *verbatim*
>  - {braces}
>  doc symbol=Enum
> +    section=Intro
> +
>      arg=one
>  The _one_ {and only}, description on the same line
>      arg=two
> @@ -117,10 +119,14 @@ a member feature
>      section=Plain
>  @two is undocumented
>  doc symbol=Base
> +    section=Intro
> +
>      arg=base1
>   description starts on a new line,
>   minimally indented
>  doc symbol=Variant1
> +    section=Intro
> +
>      section=Plain
>  A paragraph
>  
> @@ -134,10 +140,16 @@ a feature
>      feature=member-feat
>  a member feature
>  doc symbol=Variant2
> +    section=Intro
> +
>  doc symbol=Object
> +    section=Intro
> +
>      feature=union-feat1
>  a feature
>  doc symbol=Alternate
> +    section=Intro
> +
>      arg=i
>  description starts on the same line
>      remainder indented the same
> @@ -151,6 +163,8 @@ doc freeform
>  Another subsection
>  ==================
>  doc symbol=cmd
> +    section=Intro
> +
>      arg=arg1
>      description starts on a new line,
>      indented
> @@ -198,6 +212,8 @@ Note::
>      section=Since
>  2.10
>  doc symbol=cmd-boxed
> +    section=Intro
> +
>      section=Plain
>  If you're bored enough to read this, go see a video of boxed cats
>      feature=cmd-feat1
> @@ -211,5 +227,7 @@ another feature
>  
>     <- ... has no title ...
>  doc symbol=EVT_BOXED
> +    section=Intro
> +
>      feature=feat3
>  a feature
> diff --git a/tests/qapi-schema/doc-missing-colon.err b/tests/qapi-schema/doc-missing-colon.err
> index cbcea007153..bd5862b30f3 100644
> --- a/tests/qapi-schema/doc-missing-colon.err
> +++ b/tests/qapi-schema/doc-missing-colon.err
> @@ -1 +1 @@
> -doc-missing-colon.json:4:1: line should end with ':'
> +doc-missing-colon.json:4:1: @name must end with ':'

Let's not change this error message in this patch.  For what it's worth,
PATCH 13 changes it right back.