Marc-André Lureau <marcandre.lur...@gmail.com> writes: > Hi > > On Mon, Mar 13, 2017 at 10:23 AM Markus Armbruster <arm...@redhat.com> > wrote: > >> I'm proposing this is 2.9 because it fixes a documentation regression. >> It affects only documentation; generated C code is unchanged except >> for the removal of trailing space in PATCH 46. >> >> Based on my qapi-next branch, which contains Marc-André's PATCH 1/2. >> >> Marc-André's work to merge qmp-commands.txt and qmp-events.txt into >> the QAPI schema and generate their replacements from the schema >> (commit b6af8ea..56e8bdd) was a big step forward. As committed, it >> also was a step back: the documentation lost information on JSON >> types, because I didn't like Marc-André's patch to add it. He >> reposted it for further review afterwards: >> >> Subject: [PATCH 0/2] qapi2texi: add type information >> Message-Id: <20170125130308.16104-1-marcandre.lur...@redhat.com> >> https://lists.gnu.org/archive/html/qemu-devel/2017-01/msg05432.html >> >> His PATCH 1/2 is a straightforward cleanup. His PATCH 2/2 adds type >> descriptions in a new formal language to the generated documentation. >> Quoting the commit message: >> >> Array types have the following syntax: type[]. Ex: str[]. >> >> - Struct, commands and events use the following members syntax: >> >> { 'member': type, ('foo': str), ... } >> >> Optional members are under parentheses. >> >> A structure with a base type will have 'BaseStruct +' prepended. >> >> - Alternates use the following syntax: >> >> [ 'foo': type, 'bar': type, ... ] >> >> - Simple unions use the following syntax: >> >> { 'type': str, 'data': 'type' = [ 'foo': type, 'bar': type... ] } >> >> - Flat unions use the following syntax: >> >> BaseStruct + 'discriminator' = [ 'foo': type, 'bar': type... ] >> >> End quote. Looks like this in generated documentation: >> >> -- Event: VNC_CONNECTED {'server': VncServerInfo, 'client': >> VncBasicInfo} >> >> Emitted when a VNC client establishes a connection >> ''server'' >> server information >> ''client'' >> client information >> >> Note: This event is emitted before any authentication takes place, >> thus the authentication ID is not provided >> [...] >> >> -- Struct: VncServerInfo VncBasicInfo + {('auth': str)} >> >> The network connection information for server >> ''auth'' (optional) >> authentication method used for the plain (non-websocket) VNC >> server >> >> Since: 2.1 >> >> -- Simple Union: SocketAddress { 'type': str, 'data': 'type' = ['inet': >> InetSocketAddress, 'unix': UnixSocketAddress, 'vsock': >> VsockSocketAddress, 'fd': String] } >> >> Captures the address of a socket, which could also be a named file >> descriptor >> >> Since: 1.3 >> >> Here's my counter-proposal: instead of inventing a formal language, >> fix the natural language documentation to actually mention *all* >> members, and add type information in a plain, easy-to-understand way. >> Looks like this: >> >> -- Event: VNC_CONNECTED >> >> Emitted when a VNC client establishes a connection >> >> Arguments: >> 'server: VncServerInfo' >> server information >> 'client: VncBasicInfo' >> client information >> >> Note: This event is emitted before any authentication takes place, >> thus the authentication ID is not provided >> [...] >> >> -- Object: VncServerInfo >> >> The network connection information for server >> >> Members: >> 'auth: string' (optional) >> authentication method used for the plain (non-websocket) VNC >> server >> The members of 'VncBasicInfo' >> >> Since: 2.1 >> >> -- Object: SocketAddress >> >> Captures the address of a socket, which could also be a named file >> descriptor >> >> Members: >> 'type' >> One of "inet", "unix", "vsock", "fd" >> 'data: InetSocketAddress' when 'type' is "inet" >> 'data: UnixSocketAddress' when 'type' is "unix" >> 'data: VsockSocketAddress' when 'type' is "vsock" >> 'data: String' when 'type' is "fd" >> >> Since: 1.3 >> >> > I like both, to me they serve different purposes. I like to have a short > overview / signature and then a more detailed documentation for each field.
I sympathize with the argument. Unfortunately, the "short" signatures are anything but for real-world QAPI: -- Flat Union: BlockdevOptions {'driver': BlockdevDriver, ('node-name': str), ('discard': BlockdevDiscardOptions), ('cache': BlockdevCacheOptions), ('read-only': bool), ('detect-zeroes': BlockdevDetectZeroesOptions)} + 'driver' = ['archipelago': BlockdevOptionsArchipelago, 'blkdebug': BlockdevOptionsBlkdebug, 'blkverify': BlockdevOptionsBlkverify, 'bochs': BlockdevOptionsGenericFormat, 'cloop': BlockdevOptionsGenericFormat, 'dmg': BlockdevOptionsGenericFormat, 'file': BlockdevOptionsFile, 'ftp': BlockdevOptionsCurl, 'ftps': BlockdevOptionsCurl, 'gluster': BlockdevOptionsGluster, 'host_cdrom': BlockdevOptionsFile, 'host_device': BlockdevOptionsFile, 'http': BlockdevOptionsCurl, 'https': BlockdevOptionsCurl, 'iscsi': BlockdevOptionsIscsi, 'luks': BlockdevOptionsLUKS, 'nbd': BlockdevOptionsNbd, 'nfs': BlockdevOptionsNfs, 'null-aio': BlockdevOptionsNull, 'null-co': BlockdevOptionsNull, 'parallels': BlockdevOptionsGenericFormat, 'qcow2': BlockdevOptionsQcow2, 'qcow': BlockdevOptionsGenericCOWFormat, 'qed': BlockdevOptionsGenericCOWFormat, 'quorum': BlockdevOptionsQuorum, 'raw': BlockdevOptionsRaw, 'rbd': BlockdevOptionsRbd, 'replication': BlockdevOptionsReplication, 'sheepdog': BlockdevOptionsSheepdog, 'ssh': BlockdevOptionsSsh, 'vdi': BlockdevOptionsGenericFormat, 'vhdx': BlockdevOptionsGenericFormat, 'vmdk': BlockdevOptionsGenericCOWFormat, 'vpc': BlockdevOptionsGenericFormat, 'vvfat': BlockdevOptionsVVFAT] Options for creating a block device. Many options are available for all block devices, independent of the block driver: ''driver'' block driver name ''node-name'' (optional) the node name of the new node (Since 2.0). This option is required on the top level of blockdev-add. ''discard'' (optional) discard-related options (default: ignore) ''cache'' (optional) cache-related options ''read-only'' (optional) whether the block device should be read-only (default: false) ''detect-zeroes'' (optional) detect and optimize zero writes (Since 2.1) (default: off) Remaining options are determined by the block driver. Since: 1.7 >> Additionally, my series fixes a number of bugs and cleans up along the >> way. In particular, it converts qapi2texi.py from parse trees to the >> visitor interface the other generators use. >> >> > Your series failed to apply in patchew, and I can't find the branch in your > repo. Could you publish it? Done: branch qapi-doc at http://repo.or.cz/w/qemu/armbru.git >> Future generated documentation work includes eliding types that aren't >> visible in QMP (like introspection does), and making uses of type >> names links in HTML. >> >> > Yes, links would be really nice. Your work brought them into reach, let's grab them :)