Re: [Qemu-devel] [PATCH for-4.0 v7 22/27] qapi: add 'If:' condition to enum values documentation

[Markus Armbruster <armbru@redhat.com>]

Marc-André Lureau <marcandre.lureau@redhat.com> writes:

> Use a common function to generate the "If:..." line.
>
> While at it, get rid of the existing \n\n (no idea why it was
> there). Use a line-break in member description, this seems to look
> slightly better in the plaintext version.

Where exactly in the patch is this done?

> Signed-off-by: Marc-André Lureau <marcandre.lureau@redhat.com>
> ---
>  scripts/qapi/doc.py             | 24 +++++++++++++++---------
>  tests/qapi-schema/doc-good.json |  4 +++-
>  tests/qapi-schema/doc-good.out  |  1 +
>  tests/qapi-schema/doc-good.texi |  2 +-
>  4 files changed, 20 insertions(+), 11 deletions(-)
>
> diff --git a/scripts/qapi/doc.py b/scripts/qapi/doc.py
> index 76cb186ff9..2133ded47e 100755
> --- a/scripts/qapi/doc.py
> +++ b/scripts/qapi/doc.py
> @@ -126,19 +126,26 @@ def texi_body(doc):
>      return texi_format(doc.body.text)
>  
>  
> -def texi_enum_value(value):
> +def texi_if(ifcond, prefix='\n', suffix='\n'):
> +    """Format the #if condition"""
> +    return '%s@b{If:} @code{%s}%s' % (
> +        prefix, ', '.join(ifcond), suffix) if ifcond else ''

I like the ternary operator, but I nevertheless think this function is
easier to read as

       if not ifcond:
           return ''
       return '%s@b{If:} @code{%s}%s' % (prefix, ', '.join(ifcond), suffix)

> +
> +
> +def texi_enum_value(value, desc, suffix=''):
>      """Format a table of members item for an enumeration value"""
> -    return '@item @code{%s}\n' % value.name
> +    return '@item @code{%s}\n%s%s' % (
> +        value.name, desc, texi_if(value.ifcond, prefix='@*'))

Do you ignore suffix intentionally?

>  
>  
> -def texi_member(member, suffix=''):
> +def texi_member(member, desc='', suffix=''):
>      """Format a table of members item for an object type member"""
>      typ = member.type.doc_type()
>      membertype = ': ' + typ if typ else ''
> -    return '@item @code{%s%s}%s%s\n' % (
> +    return '@item @code{%s%s}%s%s\n%s' % (
>          member.name, membertype,
>          ' (optional)' if member.optional else '',
> -        suffix)
> +        suffix, desc)
>  
>  
>  def texi_members(doc, what, base, variants, member_func):
> @@ -155,7 +162,7 @@ def texi_members(doc, what, base, variants, member_func):
>              desc = 'One of ' + members_text + '\n'
>          else:
>              desc = 'Not documented\n'
> -        items += member_func(section.member) + desc
> +        items += member_func(section.member, desc)

Here, you pass a @desc argument, but no @suffix.

@member_func is either texi_enum_value() or texi_member().  Works.

>      if base:
>          items += '@item The members of @code{%s}\n' % base.doc_type()
>      if variants:
> @@ -165,7 +172,7 @@ def texi_members(doc, what, base, variants, member_func):
>              if v.type.is_implicit():
>                  assert not v.type.base and not v.type.variants
>                  for m in v.type.local_members:
> -                    items += member_func(m, when)
> +                    items += member_func(m, suffix=when)

Here, you pass a @suffix argument, but no @desc.

texi_enum_value() would choke on that, but it can't occur here, because
enums have no variants.

Still, I'd prefer texi_enum_value() and texi_member() to have the exact
same signature.

>              else:
>                  items += '@item The members of @code{%s}%s\n' % (
>                      v.type.doc_type(), when)
> @@ -185,8 +192,7 @@ def texi_sections(doc, ifcond):
>              body += texi_example(section.text)
>          else:
>              body += texi_format(section.text)
> -    if ifcond:
> -        body += '\n\n@b{If:} @code{%s}' % ", ".join(ifcond)
> +    body += texi_if(ifcond, suffix='')
>      return body
>  
>  
> diff --git a/tests/qapi-schema/doc-good.json b/tests/qapi-schema/doc-good.json
> index 984cd8ed06..c7fe08c530 100644
> --- a/tests/qapi-schema/doc-good.json
> +++ b/tests/qapi-schema/doc-good.json
> @@ -55,7 +55,9 @@
>  #
>  # @two is undocumented
>  ##
> -{ 'enum': 'Enum', 'data': [ 'one', 'two' ], 'if': 'defined(IFCOND)' }
> +{ 'enum': 'Enum', 'data':
> +  [ { 'name': 'one', 'if': 'defined(IFENUM)' }, 'two' ],
> +  'if': 'defined(IFCOND)' }

IFCOND applies to the enum, and IFENUM applies to one of its values.
Awkward.  Let's rename IFENUM to IFONE.

>  
>  ##
>  # @Base:
> diff --git a/tests/qapi-schema/doc-good.out b/tests/qapi-schema/doc-good.out
> index c2fc5c774a..a05535b69b 100644
> --- a/tests/qapi-schema/doc-good.out
> +++ b/tests/qapi-schema/doc-good.out
> @@ -11,6 +11,7 @@ enum QType
>  module doc-good.json
>  enum Enum
>      member one
> +        if ['defined(IFENUM)']
>      member two
>      if ['defined(IFCOND)']
>  object Base
> diff --git a/tests/qapi-schema/doc-good.texi b/tests/qapi-schema/doc-good.texi
> index e42eace474..5c0231e0e6 100644
> --- a/tests/qapi-schema/doc-good.texi
> +++ b/tests/qapi-schema/doc-good.texi
> @@ -84,12 +84,12 @@ Examples:
>  @table @asis
>  @item @code{one}
>  The @emph{one} @{and only@}
> +@*@b{If:} @code{defined(IFENUM)}

@* forces a line break.  Putting it at the end of the previous line
would be tidier, but let's ignore that for now.

>  @item @code{two}
>  Not documented
>  @end table
>  @code{two} is undocumented
>  
> -
>  @b{If:} @code{defined(IFCOND)}
>  @end deftp