Marc-André Lureau <mlur...@redhat.com> writes:
> Hi
>
> ----- Original Message -----
>> Marc-André Lureau <marcandre.lur...@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.lur...@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.
Let's touch it up once this series is in.
>> > ---
>> > 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
I don't think redundant conditionals buy us anything.
>> > +
>> > +@iftex
>> > +@titlepage
>> > +@sp 7
>> > +@center @titlefont{{QEMU Guest Agent {version}}}
>>
>> {version} seems to get replaced by qapi2texi.py. Worth a comment.
>>
>
> ok
Peeking at your v3, I see you found a better solution :)
>> > +@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?
We define several types in the QAPI schema that are used only
internally. Useful trick to see what's external:
$ python -B scripts/qapi-introspect.py -cu -p scratch- qapi-schema.json
This generates scratch-qmp-introspect.c describing the external QMP
interface with unmasked type names (-u). Anything not mentioned there
is not externally visible.
To see how qapi-introspect.py finds the externally visible part of the
schema, search it for _use_type.
>> > +
>> > +@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.
Replacing qmp-commands.txt by a QMP reference manual is a
straightforward task: we don't have to worry about scope or structure,
only about not losing valuable contents.
But as soon you draw in qmp-intro.txt, we're making a QMP manual.
That's a more complex task, posing additional questions. For starters,
why is qmp-spec.txt not part of it? Should the QGA manual get similar
contents?
I'm not saying that task should not be tackled. But let's control this
series' scope, and make just a QMP reference manual. We can expand it
into a QMP manual with a reference part later if we like.
> Any suggestion for the the naming? (tbh, I don't mind it being called
> reference manual or not, even if it includes that text).
We could call it "QMP User Manual".
