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

["Marc-André Lureau" <mlureau@redhat.com>]

Hi

----- Original Message -----
> 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.

I based it on some older version of qemu-doc.texi. Many of your remarks are also valid for it.

> 
> > ---
> >  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.

Shouldn't be necessary imho (in general, I am not fond of modeline unless necessary)
> 
> > +@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

ok

> 
> > +@documentlanguage en
> 
> This overrides the default en_US to just en.  Is that what we want?

let's keep the default

> 
> > +@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.*.

fine by me

> 
> > +
> 
> 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

ok

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

I'll leave that for another series

> 
> > +@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
> 

ok

> > +@end direntry
> > +@end ifinfo
> 
> Are you sure we need @ifinfo?

Probably not, but it looks more explicit to me that this part is only for .info

> > +
> > +@iftex
> > +@titlepage
> > +@sp 7
> > +@center @titlefont{{QEMU Guest Agent {version}}}
> 
> {version} seems to get replaced by qapi2texi.py.  Worth a comment.
> 

ok

> > +@sp 1
> > +@center @titlefont{{QAPI Reference Manual}}
> 
> Protocol Reference Manual
> 
> > +@sp 3
> 
> Isn't @sp right before @end titlepage redundant?

ok

> 
> We need to include the copyright notice:
> 
>    @page
>    @vskip 0pt plus 1filll
>    @insertcopying
> 

ok

> > +@end titlepage
> > +@end iftex
> 
> Are you sure we need @iftex?

Same as qemu-doc.texi, I suppose it looks better with info.

> 
> 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
> 

That ends up with a different results than qapi-doc, but I also prefer it

> 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.

Yep, looks good, altough doesn't fit on a single line, I am dropping the leading QEMU

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

Ok

> > +
> > +@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...

Removing it, now redundant with @copying text

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

ok

> > +
> > +@contents
> 
> Suggest to move this up, as mentioned above.
> 

ok

> > +
> > +@node API Reference
> > +@chapter API Reference
> > +
> > +@c man begin DESCRIPTION
> 
> What does this @c man magic do?  Suggest to explain in a comment.

It's for texi2pod, I'll add a comment
> 
> > +{qapi}
> 
> This seems to get replaced with the generated reference documentation by
> qapi2texi.py.  Worth a comment.

ok

> 
> > +@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.
> 

ok

> > +@node Data Types Index
> > +@unnumbered Data Types Index
> > +@printindex tp
> 
> And here.

ok

> 
> > +@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.

What are the internal types? How is it filtered?

> 
> > +
> > +@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.

ok

> 
> > +@item
> > +Lightweight, text-based, easy to parse data format
> 
> Suggest "plain text" instead of "text-based".

ok

> 
> 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.

Imho it would be nice to include qmp-intro in the document, has there are more chances it gets read, be it online in html/pdf for, or with the info/man pages.

Any suggestion for the the naming? (tbh, I don't mind it being called reference manual or not, even if it includes that text).

thanks

> 
> > +
> > +@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
>