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 <[email protected]> > 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 <[email protected]> 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 <[email protected]> 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 <[email protected]> > > > > > > > > > > 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 > > [email protected] > > https://lists.freedesktop.org/mailman/listinfo/wayland-devel > > _______________________________________________ wayland-devel mailing list [email protected] https://lists.freedesktop.org/mailman/listinfo/wayland-devel
