Re: [PATCH v2 3/3] docs: remove special parsing for freeform sections

[Markus Armbruster <armbru@redhat.com>]

John Snow <jsnow@redhat.com> writes:

> This change removes special parsing for freeform sections and allows
> them to simply be unmodified rST syntax. The existing headings in the
> QAPI schema are adjusted to reflect the new paradigm.
>
> Signed-off-by: John Snow <jsnow@redhat.com>

[...]

> diff --git a/docs/devel/qapi-code-gen.rst b/docs/devel/qapi-code-gen.rst
> index 231cc0fecf7..dfdbeac5a5a 100644
> --- a/docs/devel/qapi-code-gen.rst
> +++ b/docs/devel/qapi-code-gen.rst
> @@ -876,25 +876,35 @@ structuring content.
>  Headings and subheadings
>  ~~~~~~~~~~~~~~~~~~~~~~~~
>  
> -A free-form documentation comment containing a line which starts with
> -some ``=`` symbols and then a space defines a section heading::
> +Free-form documentation does not start with ``@SYMBOL`` and can contain
> +arbitrary rST markup. Headings can be marked up using the standard rST

Two spaces between sentences for consistency, please.

> +syntax::
>  
>      ##
> -    # = This is a top level heading
> +    # *************************
> +    # This is a level 2 heading
> +    # *************************
>      #
>      # This is a free-form comment which will go under the
>      # top level heading.
>      ##
>  
>      ##
> -    # == This is a second level heading
> +    # This is a third level heading
> +    # ==============================
> +    #
> +    # Level 4
> +    # _______
> +    #
> +    # Level 5
> +    # ^^^^^^^
> +    #
> +    # Level 6
> +    # """""""
>      ##
>  
> -A heading line must be the first line of the documentation
> -comment block.
> -
> -Section headings must always be correctly nested, so you can only
> -define a third-level heading inside a second-level heading, and so on.
> +Level 1 headings are reserved for use by the generated documentation
> +page itself, leaving level 2 as the highest level that should be used.
>  
>  
>  Documentation markup
> diff --git a/docs/interop/firmware.json b/docs/interop/firmware.json
> index 745d21d8223..f46fdedfa89 100644
> --- a/docs/interop/firmware.json
> +++ b/docs/interop/firmware.json
> @@ -11,7 +11,9 @@
>  # later. See the COPYING file in the top-level directory.
>  
>  ##
> -# = Firmware
> +# ********
> +# Firmware
> +# ********
>  ##
>  
>  { 'pragma': {
> diff --git a/docs/interop/vhost-user.json b/docs/interop/vhost-user.json
> index b6ade9e4931..095eb99cf79 100644
> --- a/docs/interop/vhost-user.json
> +++ b/docs/interop/vhost-user.json
> @@ -10,7 +10,9 @@
>  # later. See the COPYING file in the top-level directory.
>  
>  ##
> -# = vhost user backend discovery & capabilities
> +# *******************************************
> +# vhost user backend discovery & capabilities
> +# *******************************************
>  ##
>  
>  ##

I dislike reST headings.  Sticking to plain reST makes sense regardless.

> diff --git a/docs/sphinx/qapidoc.py b/docs/sphinx/qapidoc.py
> index c9c477378f5..cdf556c2a3c 100644
> --- a/docs/sphinx/qapidoc.py
> +++ b/docs/sphinx/qapidoc.py
> @@ -391,44 +391,9 @@ def visit_module(self, path: str) -> None:
>          self.ensure_blank_line()
>  
>      def visit_freeform(self, doc: QAPIDoc) -> None:
> -        # TODO: Once the old qapidoc transformer is deprecated, freeform
> -        # sections can be updated to pure rST, and this transformed removed.
> -        #
> -        # For now, translate our micro-format into rST. Code adapted
> -        # from Peter Maydell's freeform().
> -
>          assert len(doc.all_sections) == 1, doc.all_sections
>          body = doc.all_sections[0]
> -        text = body.text
> -        info = doc.info
> -
> -        if re.match(r"=+ ", text):
> -            # Section/subsection heading (if present, will always be the
> -            # first line of the block)
> -            (heading, _, text) = text.partition("\n")
> -            (leader, _, heading) = heading.partition(" ")
> -            # Implicit +1 for heading in the containing .rst doc
> -            level = len(leader) + 1
> -
> -            # https://www.sphinx-doc.org/en/master/usage/restructuredtext/basics.html#sections
> -            markers = ' #*=_^"'
> -            overline = level <= 2
> -            marker = markers[level]
> -
> -            self.ensure_blank_line()
> -            # This credits all 2 or 3 lines to the single source line.
> -            if overline:
> -                self.add_line(marker * len(heading), info)
> -            self.add_line(heading, info)
> -            self.add_line(marker * len(heading), info)
> -            self.ensure_blank_line()
> -
> -            # Eat blank line(s) and advance info
> -            trimmed = text.lstrip("\n")
> -            text = trimmed
> -            info = info.next_line(len(text) - len(trimmed) + 1)
> -
> -        self.add_lines(text, info)
> +        self.add_lines(body.text, doc.info)
>          self.ensure_blank_line()
>  
>      def visit_entity(self, ent: QAPISchemaDefinition) -> None:

Lovely.

Left in scripts/qapi/parser.py:

                elif line.startswith('='):
                    raise QAPIParseError(
                        self,
                        "unexpected '=' markup in definition documentation")

with test case tests/qapi-schema/doc-bad-section.json.

We might want to reject '=' markup anywhere for a while, to catch
outmoded headings.

[...]

Fails "make check".  Fixup:

diff --git a/tests/qapi-schema/doc-good.out b/tests/qapi-schema/doc-good.out
index 3711cf5480..f60a24d1b7 100644
--- a/tests/qapi-schema/doc-good.out
+++ b/tests/qapi-schema/doc-good.out
@@ -55,13 +55,16 @@ event EVT_BOXED Object
     feature feat3
 doc freeform
     body=
-= Section
+*******
+Section
+*******
 doc freeform
     body=
 Just text, no heading.
 doc freeform
     body=
-== Subsection
+Subsection
+==========
 
 *with emphasis*
 @var {in braces}
@@ -155,7 +158,8 @@ description starts on the same line
 a feature
 doc freeform
     body=
-== Another subsection
+Another subsection
+==================
 doc symbol=cmd
     body=