[PATCH v3 wayland 3/3] doc: generate doxygen html output from the scanner

Tue Mar 1 22:21:03 UTC 2016

On Tue, Mar 01, 2016 at 11:01:50AM -0800, Bill Spitzak wrote:
> What exactly is this trying to solve?
> 
> It seems entirely redundant with the protocol documentation. I think it
> would be better to try to improve the protocol documentation, perhaps
> inserting the C prototype as a one-line addition to each request and event
> (as it is true that people are not so good at translating the protocol
> description into exactly what they need to type into the program). I don't
> like the idea that the same comment from the xml file will be repeated
> three times in the docs.

I'm looking forward to your patches.

> I do think inserting the doxygen comments, as long as they are clean and
> short, into the .h files is a good idea. C programmers are going to look
> there. Please don't describe obvious arguments, or at least compress that
> to a single line, and see if you can use per-enum-value comments rather
> than repeating the entire enumeration. The main problem with the current
> one is vertical size.
> 
> However I would refrain from actually putting this Doxygen output into the
> Wayland docs. The current output that you linked to is pretty horrific and
> I would be ashamed to show it. 

I'm looking forward to your CSS styling attempts to change the default
doxygen theme. Hint - you can get doxygen themes ready-made. Pick one that
you don't consider horrific and send us patches, we'll review them provided
they bikeshed is the right colour.

Cheers,
   Peter

> But even if it was cleaned up to a readable
> state it would be redundant. I would also worry that cleaning it up will
> insert so many page/module/relates comments into the .h file that it will
> make them unreadable.
> 
> 
> On Mon, Feb 29, 2016 at 3:26 PM, Peter Hutterer <peter.hutterer at who-t.net>
> wrote:
> 
> > On Mon, Feb 29, 2016 at 03:41:39PM +0200, Pekka Paalanen wrote:
> > > On Mon, 29 Feb 2016 17:08:29 +1000
> > > Peter Hutterer <peter.hutterer at who-t.net> wrote:
> > >
> > > > On Fri, Feb 26, 2016 at 05:00:15PM +0200, Pekka Paalanen wrote:
> > > > > On Mon, 22 Feb 2016 08:57:47 +1000
> > > > > Peter Hutterer <peter.hutterer at who-t.net> wrote:
> > > > >
> > > > > > This switches the scanner to generate doxygen-compatible tags for
> > the
> > > > > > generated protocol headers, and hooks up the doxygen build to
> > generate server
> > > > > > and client-side API documentation.
> > > > > >
> > > > > > Each protocol is a separate doxygen @page, with each interface a
> > @subpage.
> > > > > > Wayland only has one protocol, wayland-protocols will have these
> > nested.
> > > > > > Each protocol page has a list of interfaces and the copyright and
> > description
> > > > > > where available.
> > > > > > All interfaces are grouped by doxygen @defgroup and @ingroups and
> > appear in
> > > > > > "Modules" in the generated output. Each interface subpage has the
> > description
> > > > > > and a link to the actual API doc.
> > > > > > Function, struct and #defines are documented in doxygen style and
> > associated
> > > > > > with the matching interface.
> > > > > >
> > > > > > Note that pages and groups have fixed HTML file names and are
> > directly
> > > > > > linkable/bookmark-able.
> > > > > >
> > > > > > The @mainpage is a separate file that's included at build time. It
> > doesn't
> > > > > > contain much other than links to where the interesting bits are.
> > It's a static
> > > > > > file though that supports markdown, so we can extend it easily in
> > the future.
> > > > > >
> > > > > > For doxygen we need the new options EXTRACT_ALL and
> > OPTIMIZE_OUTPUT_FOR_C so
> > > > > > it scans C code properly. EXTRACT_STATIC is needed since most of
> > the protocol
> > > > > > hooks are static. WARN_IF_DOC_ERROR is required to silence the
> > warnings when
> > > > > > some parameters are documented but others aren't.
> > > > > >
> > > > > > Signed-off-by: Peter Hutterer <peter.hutterer at who-t.net>
> > > > >
> > > > > Hi Peter,
> > > > >
> > > > > I started looking at the output of this, and it took a while to
> > realize
> > > > > where all the output goes. Nothing much changes with the existing
> > > > > documentation we install, so I got puzzled.
> > > > >
> > > > > Then I looked at doc/doxygen/html/index.html and it didn't seem to
> > have
> > > > > what you described.
> > > > >
> > > > > Then re-reading the Makefile.am and which directory it is in, I
> > > > > realized I need to look at doc/doxygen/html/Client/index.html and
> > > > > doc/doxygen/html/Server/index.html. These are the main outputs of
> > this
> > > > > patch, right?
> > > >
> > > > yes, sorry, i should've pointed this out in the commit message.
> > > >
> > > > > I get the feeling the generated documentation is quite scattered and
> > > > > not easy to find, but you probably intend to fix that in future?
> > > > > I have quite forgot the earlier discussions, sorry.
> > > >
> > > > I'm not quite sure what you mean here. the documentation is in normal
> > > > doxygen format, all protocol interfaces are on the related page but
> > also in
> > > > the modules. One drawback is that each interface page is little more
> > than a
> > > > stop through to the module page but I don't know how to fix this within
> > > > doxygen. That's a bit of a roundabout way but it makes more sense for
> > > > wayland-protocols where we have more than one <protocol>
> > >
> > > Hi Peter,
> > >
> > > that was more about that you need to know to go to:
> > > - doc/doxygen/html/index.html for libwayland API docs
> >
> > These shouldn't be generated anymore, turns out this was a bug with the
> > makefile/doxygen. GENERATE_HTML is on by default so unless we turn it off,
> > the xml and man page targets produce that output. I've fixed that in the
> > v4.
> >
> > > - doc/doxygen/html/Client/index.html for client protocol API docs
> > > - doc/doxygen/html/Server/index.html for server protocol API docs
> > > - and then the "Wayland book" that gets installed is yet another
> > >   independent entity
> > >
> > > None of these contain links to the other that I could see, they are all
> > > completely independent, and mostly not installed.
> > >
> > > Even a top-level page that just contained links to these "books" would
> > > go a long way.
> > >
> > > That's all fine to sort out later.
> >
> > Patch in mid-flight, I kept this one separate though.
> >
> > > > > Looking at e.g.
> > > > >
> > file:///home/pq/git/wayland/doc/doxygen/html/Client/group__iface__wl__surface.html
> > > > > it seems all the function documentations are missing. Well, they were
> > > > > missing also before.
> > > >
> > > > weird. This should be part of the output but it somehow got lost. New
> > > > patch coming up that reinstates this.
> > >
> > > Nice.
> > >
> > > > > I have opposed silencing the warnings about undocumented parameters
> > > > > before. Is there something here that makes documenting all parameters
> > > > > not possible, rather than just not having written docs in XML?
> > > >
> > > > I've added @param resource_ where we do so, the rest would need to be
> > in the
> > > > xml. The other option would be to add a fake documentation, so you get
> > > > something like
> > > >    @param foo Undocumented parameter foo
> > > > or similar. Not a fan of this though, better to document it in the
> > protocol.
> > >
> > > Right, it would be good if the scanner fills in all the parameters you
> > > cannot document in the xml, and rely on the xml for the rest. Things
> > > like wl_registry.bind's C API have several arguments for a single XML
> > > <arg>.
> > >
> > > IMHO, undocumented parameters should cause noise and not be silenced by
> > > stub docs.
> >
> > I *think* at this point all leftover warnings are for things that could be
> > documented in the xml file.
> >
> > > > > When I compare old and new wayland-client-protocol.h, I see that in
> > the
> > > > > beginning with the list of extern const struct wl_interface
> > > > > wl_display_interface; etc. the documentation seems to be twice in the
> > > > > new comment. Is that necessary?
> > > >
> > > > Basically yes. the output is:
> > > >
> > > > /**
> > > >  * @page page_iface_wl_display wl_display
> > > >  * @section page_iface_wl_display_desc Description
> > > >  *
> > > >  * The core global object.  This is a special singleton object.  It
> > > >  * is used for internal Wayland protocol features.
> > > >  * @section page_iface_wl_display_api API
> > > >  * See @ref iface_wl_display.
> > > >  *
> > > >  * @defgroup iface_wl_display The wl_display interface
> > > >  *
> > > >  * The core global object.  This is a special singleton object.
> > > >  * It is used for internal Wayland protocol features.
> > > >  */
> > > >
> > > > Note the @page and @defgroup command. The first defines a page (see
> > Related
> > > > Pages which then links to the module), the second defines a group that
> > we can
> > > > then group the various commands in. It is trivial to separate
> > > > those two into two comment blocks to make it easier to recognise. I've
> > done
> > > > this in the new patch.
> > >
> > > Ok. What was the purpose to have a @page in addition to the group?
> >
> > this is the bit that makes more sense for wayland-protocols. modules are
> > flat, pages have a two-level hierarchy (page and subpage). In core wayland
> > we only have one page, in w-p we have one per protocol with the interfaces
> > being subpages. So the Related Pages gives us a tree:
> >
> > https://people.freedesktop.org/~whot/wayland-doxygen/wayland-protocols/Client/pages.html
> > (this is an older version of w-p, but still looks the same)
> >
> > Cheers,
> >    Peter
> > _______________________________________________
> > wayland-devel mailing list
> > wayland-devel at lists.freedesktop.org
> > https://lists.freedesktop.org/mailman/listinfo/wayland-devel
> >