Re: [Qemu-devel] [PATCH v6 13/15] qapi: add 'If:' section to generated documentation

[Markus Armbruster]

Marc-André Lureau <address@hidden> writes:

> The documentation is generated only once, and doesn't know C
> pre-conditions. Add 'If:' sections for top-level entities.
>
> Signed-off-by: Marc-André Lureau <address@hidden>
> ---
>  scripts/qapi/doc.py             | 22 ++++++++++++----------
>  tests/qapi-schema/doc-good.json |  2 +-
>  tests/qapi-schema/doc-good.out  |  1 +
>  tests/qapi-schema/doc-good.texi |  2 ++
>  4 files changed, 16 insertions(+), 11 deletions(-)
>
> diff --git a/scripts/qapi/doc.py b/scripts/qapi/doc.py
> index 4db6674dc3..987fd3c943 100755
> --- a/scripts/qapi/doc.py
> +++ b/scripts/qapi/doc.py
> @@ -174,7 +174,7 @@ def texi_members(doc, what, base, variants, member_func):
>      return 'address@hidden:address@hidden @address@hidden table\n' % (what, 
> items)
>  
>  
> -def texi_sections(doc):
> +def texi_sections(doc, ifcond):
>      """Format additional sections following arguments"""
>      body = ''
>      for section in doc.sections:
> @@ -185,14 +185,16 @@ def texi_sections(doc):
>              body += texi_example(section.text)
>          else:
>              body += texi_format(section.text)
> +    if ifcond:
> +        body += 'address@hidden:} @code{%s}' % ", ".join(ifcond)
>      return body
>  
>  
> -def texi_entity(doc, what, base=None, variants=None,
> +def texi_entity(doc, what, ifcond, base=None, variants=None,
>                  member_func=texi_member):
>      return (texi_body(doc)
>              + texi_members(doc, what, base, variants, member_func)
> -            + texi_sections(doc))
> +            + texi_sections(doc, ifcond))
>  
>  
>  class QAPISchemaGenDocVisitor(qapi.common.QAPISchemaVisitor):
> @@ -208,7 +210,7 @@ class 
> QAPISchemaGenDocVisitor(qapi.common.QAPISchemaVisitor):
>          doc = self.cur_doc
>          self._gen.add(TYPE_FMT(type='Enum',
>                                 name=doc.symbol,
> -                               body=texi_entity(doc, 'Values',
> +                               body=texi_entity(doc, 'Values', ifcond,
>                                                  
> member_func=texi_enum_value)))
>  
>      def visit_object_type(self, name, info, ifcond, base, members, variants):
> @@ -217,14 +219,14 @@ class 
> QAPISchemaGenDocVisitor(qapi.common.QAPISchemaVisitor):
>              base = None
>          self._gen.add(TYPE_FMT(type='Object',
>                                 name=doc.symbol,
> -                               body=texi_entity(doc, 'Members',
> +                               body=texi_entity(doc, 'Members', ifcond,
>                                                  base, variants)))
>  
>      def visit_alternate_type(self, name, info, ifcond, variants):
>          doc = self.cur_doc
>          self._gen.add(TYPE_FMT(type='Alternate',
>                                 name=doc.symbol,
> -                               body=texi_entity(doc, 'Members')))
> +                               body=texi_entity(doc, 'Members', ifcond)))
>  
>      def visit_command(self, name, info, ifcond, arg_type, ret_type, gen,
>                        success_response, boxed, allow_oob, allow_preconfig):
> @@ -233,9 +235,9 @@ class 
> QAPISchemaGenDocVisitor(qapi.common.QAPISchemaVisitor):
>              body = texi_body(doc)
>              body += ('address@hidden:} the members of @code{%s}\n'
>                       % arg_type.name)
> -            body += texi_sections(doc)
> +            body += texi_sections(doc, ifcond)
>          else:
> -            body = texi_entity(doc, 'Arguments')
> +            body = texi_entity(doc, 'Arguments', ifcond)
>          self._gen.add(MSG_FMT(type='Command',
>                                name=doc.symbol,
>                                body=body))
> @@ -244,7 +246,7 @@ class 
> QAPISchemaGenDocVisitor(qapi.common.QAPISchemaVisitor):
>          doc = self.cur_doc
>          self._gen.add(MSG_FMT(type='Event',
>                                name=doc.symbol,
> -                              body=texi_entity(doc, 'Arguments')))
> +                              body=texi_entity(doc, 'Arguments', ifcond)))
>  
>      def symbol(self, doc, entity):
>          if self._gen._body:
> @@ -257,7 +259,7 @@ class 
> QAPISchemaGenDocVisitor(qapi.common.QAPISchemaVisitor):
>          assert not doc.args
>          if self._gen._body:
>              self._gen.add('\n')
> -        self._gen.add(texi_body(doc) + texi_sections(doc))
> +        self._gen.add(texi_body(doc) + texi_sections(doc, None))
>  
>  
>  def gen_doc(schema, output_dir, prefix):
> diff --git a/tests/qapi-schema/doc-good.json b/tests/qapi-schema/doc-good.json
> index 97ab4625ff..984cd8ed06 100644
> --- a/tests/qapi-schema/doc-good.json
> +++ b/tests/qapi-schema/doc-good.json
> @@ -55,7 +55,7 @@
>  #
>  # @two is undocumented
>  ##
> -{ 'enum': 'Enum', 'data': [ 'one', 'two' ] }
> +{ 'enum': 'Enum', 'data': [ 'one', 'two' ], 'if': 'defined(IFCOND)' }
>  
>  ##
>  # @Base:
> diff --git a/tests/qapi-schema/doc-good.out b/tests/qapi-schema/doc-good.out
> index 9c8a4838e1..35f3f1164c 100644
> --- a/tests/qapi-schema/doc-good.out
> +++ b/tests/qapi-schema/doc-good.out
> @@ -3,6 +3,7 @@ enum QType ['none', 'qnull', 'qnum', 'qstring', 'qdict', 
> 'qlist', 'qbool']
>      prefix QTYPE
>  module doc-good.json
>  enum Enum ['one', 'two']
> +    if ['defined(IFCOND)']
>  object Base
>      member base1: Enum optional=False
>  object Variant1
> diff --git a/tests/qapi-schema/doc-good.texi b/tests/qapi-schema/doc-good.texi
> index 0aed2300a5..e42eace474 100644
> --- a/tests/qapi-schema/doc-good.texi
> +++ b/tests/qapi-schema/doc-good.texi
> @@ -89,6 +89,8 @@ Not documented
>  @end table
>  @code{two} is undocumented
>  
> +
> address@hidden:} @code{defined(IFCOND)}
>  @end deftp

One blank line would be better, but that's as pervasive as it is minor.

Reviewed-by: Markus Armbruster <address@hidden>

For the record, wanted follow-up work, from v5's review:

* The big comment in qapi-schema.json should explain the documentation
  format, including the meaning of tags like 'If:', 'Since:' and so
  forth

* Better test coverage