Re: [Qemu-devel] [PATCH v2 05/11] docs: add qapi texi template

[Markus Armbruster <armbru@redhat.com>]

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

> The qapi2texi scripts uses a template for the texi file. Since we are
> going to generate the documentation in multiple formats, move qmp-intro
> to qemu-qapi template. (it would be nice to write something similar for
> qemu-ga, but this is left for a future patch)
>
> Signed-off-by: Marc-André Lureau <marcandre.lureau@redhat.com>

I'm not exactly a Texinfo expert, but I can compare to the Texinfo
manual.

Lots of comments below.  Please don't let them discourage you!  Your two
manuals are pretty slick already, and a most welcome step forward.

> ---
>  docs/qemu-ga-qapi.template.texi |  58 ++++++++++++++++
>  docs/qemu-qapi.template.texi    | 148 ++++++++++++++++++++++++++++++++++++++++
>  docs/qmp-intro.txt              |  87 -----------------------
>  3 files changed, 206 insertions(+), 87 deletions(-)
>  create mode 100644 docs/qemu-ga-qapi.template.texi
>  create mode 100644 docs/qemu-qapi.template.texi
>  delete mode 100644 docs/qmp-intro.txt
>
> diff --git a/docs/qemu-ga-qapi.template.texi b/docs/qemu-ga-qapi.template.texi
> new file mode 100644
> index 0000000..3ddbf56
> --- /dev/null
> +++ b/docs/qemu-ga-qapi.template.texi
> @@ -0,0 +1,58 @@
> +\input texinfo

The Texinfo manual uses

   \input texinfo   @c -*-texinfo-*-

but my version of Emacs seems to be fine without this.

> +@setfilename qemu-ga-qapi

Not wrong, but the Texinfo manual recommends to replace the extension
(here: .texi) with .info, so let's do that:

   @setfilename qemu-ga-qapi.info

> +@documentlanguage en

This overrides the default en_US to just en.  Is that what we want?

> +@exampleindent 0
> +@paragraphindent 0
> +
> +@settitle QEMU-GA QAPI Reference Manual

What is "QAPI", and why would the reader care?  I think the manual is
about the QEMU Guest Agent Protocol.  The fact that its implementation
relies on QAPI is immaterial here.  What about:

   @settitle QEMU Guest Agent Protocol Reference

But then the filenames are off.  Rename to qemu-ga-ref.*.

> +

I think we need a copyright note.  Something like:

   @copying
   This is the QEMU Guest Agent QAPI reference manual.

   Copyright @copyright{} 2016 The QEMU Project developers

   @quotation
   Permission is granted to ...
   @end quotation
   @end copying

> +@ifinfo

   @dircategory QEMU

Should be added to qemu-doc.texi as well.

> +@direntry
> +* QEMU-GA-QAPI: (qemu-doc).    QEMU-GA QAPI Reference Manual

Pasto: (qemu-doc)

The description should start at column 32, not 31.

If we retitle and rename as suggested, this becomes:

   * QEMU-GA-Ref: (qemu-ga-ref):   QEMU Guest Agent Protocol Reference

> +@end direntry
> +@end ifinfo

Are you sure we need @ifinfo?

> +
> +@iftex
> +@titlepage
> +@sp 7
> +@center @titlefont{{QEMU Guest Agent {version}}}

{version} seems to get replaced by qapi2texi.py.  Worth a comment.

> +@sp 1
> +@center @titlefont{{QAPI Reference Manual}}

Protocol Reference Manual

> +@sp 3

Isn't @sp right before @end titlepage redundant?

We need to include the copyright notice:

   @page
   @vskip 0pt plus 1filll
   @insertcopying

> +@end titlepage
> +@end iftex

Are you sure we need @iftex?

We can also let Texinfo do the spacing, like this:

   @titlepage
   @title QEMU Guest Agent {version}
   @subtitle Protocol Reference Manual
   @page
   @vskip 0pt plus 1filll
   @insertcopying
   @end titlepage

The @title isn't really the title, though.  Could reshuffle things a
bit, e.g.

   @title QEMU Guest Agent Protocol Reference Manual
   @subtitle for QEMU {version}

Your choice.

The examples in Texinfo manual Appendix C "Sample Texinfo Files" have
@contents right here, after the title page.

> +
> +@ifnottex
> +@node Top
> +@top

   @top QEMU Guest Agent QAPI reference

> +
> +This is the QEMU Guest Agent QAPI reference for QEMU {version}.

"protocol reference manual for"

According to the Texinfo manual's examples, the @end ifnottex goes
here...

> +
> +@menu
> +* API Reference::
> +* Commands and Events Index::
> +* Data Types Index::
> +@end menu
> +
> +@end ifnottex

... and not here.

> +
> +@contents

Suggest to move this up, as mentioned above.

> +
> +@node API Reference
> +@chapter API Reference
> +
> +@c man begin DESCRIPTION

What does this @c man magic do?  Suggest to explain in a comment.

> +{qapi}

This seems to get replaced with the generated reference documentation by
qapi2texi.py.  Worth a comment.

> +@c man end
> +
> +@c man begin SEEALSO
> +The HTML documentation of QEMU for more precise information.
> +@c man end

More @c man magic.

> +
> +@node Commands and Events Index
> +@unnumbered Commands and Events Index
> +@printindex fn

Blank line here, please.

> +@node Data Types Index
> +@unnumbered Data Types Index
> +@printindex tp

And here.

> +@bye
> diff --git a/docs/qemu-qapi.template.texi b/docs/qemu-qapi.template.texi
> new file mode 100644
> index 0000000..102c8d9
> --- /dev/null
> +++ b/docs/qemu-qapi.template.texi

The comments above largely apply; I won't repeat them.

> @@ -0,0 +1,148 @@
> +\input texinfo
> +@setfilename qemu-qapi
> +@documentlanguage en
> +@exampleindent 0
> +@paragraphindent 0
> +
> +@settitle QEMU QAPI Reference Manual

Again, QAPI is detail; it's the QEMU QMP reference manual.  Except it
has more than just QMP, because we choose to use qapi-schema.json for
purely internal types in addition to QMP.

Options:

* Claim this is the QMP reference manual, include the internal types
  anyway.

* Filter out the internal types automatically, similar to
  qmp-introspect.py.

* Filter out the internal types manually, by annotating them in the
  schema.  Feels error-prone.

* Split the QAPI schema.

* Reflect the curious mix of QMP protocol and internal data type
  reference in the title.

We don't need a perfect solution to commit this, but an understanding of
what we want to do would be nice.

> +
> +@ifinfo
> +@direntry
> +* QEMU: (qemu-doc).    QEMU QAPI Reference Manual
> +@end direntry
> +@end ifinfo
> +
> +@iftex
> +@titlepage
> +@sp 7
> +@center @titlefont{{QEMU Emulator {version}}}
> +@sp 1
> +@center @titlefont{{QAPI Reference Manual}}
> +@sp 3
> +@end titlepage
> +@end iftex
> +
> +@ifnottex
> +@node Top
> +@top
> +
> +This is the QMP QAPI reference for QEMU {version}.
> +
> +@menu
> +* Introduction::
> +* API Reference::
> +* Commands and Events Index::
> +* Data Types Index::
> +@end menu
> +
> +@end ifnottex
> +
> +@contents
> +
> +@node Introduction
> +@chapter Introduction
> +
> +The QEMU Machine Protocol (@acronym{{QMP}}) allows applications to
> +operate a QEMU instance.
> +
> +QMP is @uref{{http://www.json.org, JSON}} based and features the
> +following:
> +
> +@itemize @minus

@bullet is more common.  Matter of taste.

> +@item
> +Lightweight, text-based, easy to parse data format

Suggest "plain text" instead of "text-based".

JSON is "easy to parse" until you hit the potholes in its specification.
See "Parsing JSON is a Minefield" <http://seriot.ch/parsing_json.html>.

   QMP provides the following features:

   @itemize @bullet
   @item
   Simple @uref{{http://www.json.org, JSON}} syntax

> +@item
> +Asynchronous messages support (ie. events)

i.e.

But I'd say

   @item
   Synchronous commands and replies
   @item
   Asynchronous messages ("events")

> +@item
> +Capabilities Negotiation

I'd add

   @item
   Introspection

> +@end itemize
> +
> +For detailed information on QEMU Machine Protocol, the specification
> +is in @file{{qmp-spec.txt}}.
> +
> +@section Usage
> +
> +You can use the @option{{-qmp}} option to enable QMP. For example, the
> +following makes QMP available on localhost port 4444:
> +
> +@example
> +$ qemu [...] -qmp tcp:localhost:4444,server,nowait
> +@end example
> +
> +However, for more flexibility and to make use of more options, the
> +@option{{-mon}} command-line option should be used. For instance, the
> +following example creates one HMP instance (human monitor) on stdio
> +and one QMP instance on localhost port 4444:

This sounds a bit like we don't want people to use -qmp.  What about

   However, for more flexibility and to make use of more options, the
   @option{{-mon}} command-line option should be used. For instance, the
   following example creates one HMP instance (human monitor) on stdio
   and one QMP instance on localhost port 4444:
   

> +
> +@example
> +$ qemu [...] -chardev stdio,id=mon0 -mon chardev=mon0,mode=readline \
> +             -chardev socket,id=mon1,host=localhost,port=4444,server,nowait \
> +             -mon chardev=mon1,mode=control,pretty=on
> +@end example

Not sure we want to drag in HMP here.

> +
> +Please, refer to QEMU's manpage for more information.

Drop the comma.

Hrmmm, I just realized this is merely moved from qmp-intro.txt.  I guess
I should read your commit message before your patch %-)

I'm not sure a *reference* manual is a good home for an *introduction*
to use.  It's certainly not where I'd look first.

We can decide this isn't a reference manual after all, and change title,
file name and so forth accordingly.

Or we can stick to the reference manual idea, and include qmp-intro.txt
by reference.

> +
> +@section Simple testing
> +
> +To manually test QMP one can connect with telnet and issue commands by
> +hand:
> +
> +@example
> +$ telnet localhost 4444
> +Trying 127.0.0.1...
> +Connected to localhost.
> +Escape character is '^]'.
> +@{{
> +    "QMP": @{{
> +        "version": @{{
> +            "qemu": @{{
> +                "micro": 50,
> +                "minor": 6,
> +                "major": 1
> +            @}},
> +            "package": ""
> +        @}},
> +        "capabilities": [
> +        ]
> +    @}}
> +@}}
> +
> +@{{ "execute": "qmp_capabilities" @}}
> +@{{
> +    "return": @{{
> +    @}}
> +@}}
> +
> +@{{ "execute": "query-status" @}}
> +@{{
> +    "return": @{{
> +        "status": "prelaunch",
> +        "singlestep": false,
> +        "running": false
> +    @}}
> +@}}
> +@end example
> +
> +@section Wiki
> +
> +Please refer to the @uref{{http://wiki.qemu-project.org/QMP, QMP QEMU
> + wiki page}} for more details on QMP.
> +
> +@node API Reference
> +@chapter API Reference
> +
> +@c man begin DESCRIPTION
> +{qapi}
> +@c man end
> +
> +@c man begin SEEALSO
> +The HTML documentation of QEMU for more precise information.
> +@c man end
> +
> +@node Commands and Events Index
> +@unnumbered Commands and Events Index
> +@printindex fn
> +@node Data Types Index
> +@unnumbered Data Types Index
> +@printindex tp
> +@bye
> diff --git a/docs/qmp-intro.txt b/docs/qmp-intro.txt
> deleted file mode 100644
> index f6a3a03..0000000
> --- a/docs/qmp-intro.txt
> +++ /dev/null
> @@ -1,87 +0,0 @@
> -                          QEMU Machine Protocol
> -                          =====================
> -
> -Introduction
> -------------
> -
> -The QEMU Machine Protocol (QMP) allows applications to operate a
> -QEMU instance.
> -
> -QMP is JSON[1] based and features the following:
> -
> -- Lightweight, text-based, easy to parse data format
> -- Asynchronous messages support (ie. events)
> -- Capabilities Negotiation
> -
> -For detailed information on QMP's usage, please, refer to the following files:
> -
> -o qmp-spec.txt      QEMU Machine Protocol current specification
> -o qmp-commands.txt  QMP supported commands (auto-generated at build-time)
> -o qmp-events.txt    List of available asynchronous events
> -
> -[1] http://www.json.org
> -
> -Usage
> ------
> -
> -You can use the -qmp option to enable QMP. For example, the following
> -makes QMP available on localhost port 4444:
> -
> -$ qemu [...] -qmp tcp:localhost:4444,server,nowait
> -
> -However, for more flexibility and to make use of more options, the -mon
> -command-line option should be used. For instance, the following example
> -creates one HMP instance (human monitor) on stdio and one QMP instance
> -on localhost port 4444:
> -
> -$ qemu [...] -chardev stdio,id=mon0 -mon chardev=mon0,mode=readline \
> -             -chardev socket,id=mon1,host=localhost,port=4444,server,nowait \
> -             -mon chardev=mon1,mode=control,pretty=on
> -
> -Please, refer to QEMU's manpage for more information.
> -
> -Simple Testing
> ---------------
> -
> -To manually test QMP one can connect with telnet and issue commands by hand:
> -
> -$ telnet localhost 4444
> -Trying 127.0.0.1...
> -Connected to localhost.
> -Escape character is '^]'.
> -{
> -    "QMP": {
> -        "version": {
> -            "qemu": {
> -                "micro": 50, 
> -                "minor": 6, 
> -                "major": 1
> -            }, 
> -            "package": ""
> -        }, 
> -        "capabilities": [
> -        ]
> -    }
> -}
> -
> -{ "execute": "qmp_capabilities" }
> -{
> -    "return": {
> -    }
> -}
> -
> -{ "execute": "query-status" }
> -{
> -    "return": {
> -        "status": "prelaunch", 
> -        "singlestep": false, 
> -        "running": false
> -    }
> -}
> -
> -Please, refer to the qapi-schema.json file for a complete command reference.
> -
> -QMP wiki page
> --------------
> -
> -http://wiki.qemu-project.org/QMP