Re: [PATCH v4 04/18] tests/qapi/doc-good.json: Clean up markup

[Peter Maydell]

On Wed, 5 Aug 2020 at 14:03, Markus Armbruster <armbru@redhat.com> wrote:
>
> The subject is a bit misleading.  The markup doesn't need cleanup.  It
> purposefully tests corner cases of the doc comment parser.  For some of
> them, the conversion to rST will change the meaning.  This commit tweaks
> the test so it passes before and after the conversion.  Makes it a worse
> test for the doc comment parser, which doesn't matter, because the code
> in question is about to be deleted.
>
> Peter Maydell <peter.maydell@linaro.org> writes:
>
> > doc-good.json tests some oddities of markup that we don't want to
> > accept.  Make them match the more restrictive rST syntax:
> >
> >  * in a single list the bullet types must all match
> >  * lists must have leading and following blank lines
> >  * indentation is important
>
> Actually, indentation has always been important, but the conversion to
> rST changes where and how it matters.

Mmm. More specifically, indentation was previously unimportant
within a multiline block of text, and is now important there.

> >  * the '|' example syntax is going to go away entirely, so stop
> >    testing it
>
> Before the series, we (try to) cover all doc markup here.
>
> The series replaces the doc markup language by rST + a little extra.
> The test must still cover the little extra then, and covering a bit of
> rST to ensure basic sanity won't hurt.
>
> Correct?
>
> I'm asking because a "yes" explains why we can drop coverage without
> replacement: it's appropriate when the markup in question is replaced by
> rST.

I guess so. We still want some test coverage of the stuff the
doc-comment parser is doing that's actually looking for
arguments and so on; we don't really need to check that
rst is rst.

It's been a while since I wrote this patch, but basically IIRC
it's "make the minimal changes necessary so that the test does
not start failing for the parser changes that will follow".

> > This will avoid the tests spuriously breaking when we tighten up the
> > parser code in the following commits.

> > @@ -62,7 +62,7 @@
> >  ##
> >  # @Base:
> >  # @base1:
>
> Here, indentation is relevant even before the series: @base: is only
> recognized as an argument section when it's not indented.
>
> > -#   the first member
> > +# the first member
>
> Why do you need to unindent this line?

Because in the tightened syntax, you can either have:

@foo: line 1
      line 2

(the definition part of the argument is multiple lines of
rST, which all must be lined up to start at the same column)

or

@foo:
line1
line2

(the definition is multiple lines of rST, which all start
in column 1)

But
@foo:
   line1

is neither of the above, and will be invalid.

The old parser simply stripped all the leading whitespace
from this kind of multiline lump-of-doc-text. That doesn't
work for rST because we want to be able to have doc-text
where leading whitespace matters. So we need to know how
many characters of whitespace to delete from each line. The
two options above basically correspond to the two major
pre-existing kinds of doc-comment.

Compare commit 26ec4e53f2bf0a381, which fixed up the
doc comments in qapi/ proper to follow this "one of
these two indent models, but not anything else" pattern.

> >  ##
> >  { 'struct': 'Base', 'data': { 'base1': 'Enum' } }
> >
> > @@ -101,7 +101,7 @@
> >  ##
> >  # @Alternate:
> >  # @i: an integer
> > -# @b is undocumented
> > +#     @b is undocumented
>
> Why do you need to indent this line?

Again, because it needs to follow one of the two
indent patterns described above. Either no text
following the "@i:" and all lines start in column 1,
or all lines have to start in the same column as
the text following the "@i:". In this case I went for
option 2.

The test input is a bit odd because it's talking
about @b in the description-text for @i, but there
you go. You could alternatively add a newline
after the @i: line to put the text "@b is undocumented"
into the doc-text for the whole @Alternate struct
rather than into the doc-text for the @i member.

> >  { 'alternate': 'Alternate',
> >    'data': { 'i': 'int', 'b': 'bool' } }
> > @@ -115,7 +115,7 @@
> >  # @arg1: the first argument
> >  #
> >  # @arg2: the second
> > -# argument
> > +#        argument
>
> Why do you need to indent this line?

As above, beacuse it's not in either of the
two standard "this is what indent for a multi-line
bit of doc text for an argument can be" patterns.

thanks
-- PMM