[RFC wayland] doc: generate doxygen html output from the scanner
Peter Hutterer
peter.hutterer at who-t.net
Sun Nov 1 16:38:37 PST 2015
On Thu, Oct 29, 2015 at 11:00:51AM -0700, Bill Spitzak wrote:
> Protocol documentation is produced directly from the xml currently. If it
> is merged with doxygen-produced comments then there will be duplication and
> possible confusion, so these header files must somehow be skipped when
> running doxygen.
the current protocol documentation documents the protocol but not the
C-level API. Two differenent items. The reason we have docbook (and
publican) is the prose that you can pack into it and the "ease" of producing
a single piece of output, be it html or pdf. The struggle with docbook comes
from converting bits that arent docbook to docbook.
In theory you could make the scanner spit out docbook in which case many of
the makefile/xslt hacks go away.
> That said, it may make sense to just use this and doxygen to produce the
> documentation, skipping all the docbook and xml conversions, and this would
> be part of such a patch. Positive reasons I can see:
>
> - Eliminates the use of docbook. Current combination of docbook and doxygen
> is pretty confusing. Outside of gnu I can assure you that doxygen is vastly
> more popular and familiar to programmers.
again, two different documentation approaches. Both are good for some things
and worse for others, neither fully replaces the other.
> - doxygen produces links easily, allowing the client and server api docs to
> refer to messages/events, and messages/event documentation to refer to
> functions in the client and server apis, with working cross-links.
> Descriptive chapters, if added, can easily link to the items in the
> documentation.
>
> - Documentation is specific to a backend api, removing any ambiguity about
> how that backend translated messages and event descriptions. The
> documentation will list exactly what the user types into their software.
>
> Problems I see:
>
> - Unless a lot of \relates and pages comments are added, the C backend
> documentation will lose all the object-oriented aspects of the protocol.
> Doxygen is not really good at this and is heavily biased toward C++.
That's exactly what the patch does, adding a bunch of @ingroup to keep
things together. We generate the source code, so we can write anything into
it to make it do what we need it to.
> - It is a real pain to get Doxygen to produce introductory and
> instructional chapters that look remotely attractive. This may lead to
> running the doxygen output through docbook to add these, just like the
> current version does.
Depends what "remotely attractive" means. I'd say for visual effects we
'just' need a better CSS. @page, @subpage, @section and @subsection give you
a relatively deep nesting, enough for our current docs. doxygen does support
markdown too.
> - Ambiguity may be a lot worse if somebody has to use the C documentation
> to figure out how to use their backend in a different language.
as I said in the other email, the overlap between the output generated by
this patch and the current documentation is almost nil, so there's little
ambiguity at this point. tbh, instead of the doxygen-style HTML nothing
really stops us from adding all the actual protocol C-level API to the
current publican output in docbook format. Then we'd have ambiguity, but at
least we'd have information too :)
Cheers,
Peter
>
> On Wed, Oct 28, 2015 at 6:48 PM, 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.
> >
> > For the wayland protocol, this generates a mainpage with the copyright
> > information, all interfaces are separated by doxygen groups and thus
> > listed in
> > "Modules" in the generated output.
> >
> > Function, struct and #defines are documented in doxygen style and
> > associated
> > with the matching interface.
> > ---
> > This is an RFC for now, we need to agree on whether we want to switch to
> > doxygen style first. Other changes still missing here are:
> > * afaict, the summary can be dropped for most entries, it doesn't seem
> > to add any value if a long description is there
> > * currently parsing too many header files, we should only parse the
> > protocol
> > ones for a cleaner documentation.
> > * future work for wayland-protocols is to add the various protocols at the
> > @page level
> > * a couple of things don't have the doxygen tag yet (mostly #defines)
> >
> > doc/doxygen/Makefile.am | 22 +++++-
> > doc/doxygen/wayland.doxygen.in | 4 +-
> > src/scanner.c | 165
> > ++++++++++++++++++++++++++---------------
> > 3 files changed, 130 insertions(+), 61 deletions(-)
> >
> > diff --git a/doc/doxygen/Makefile.am b/doc/doxygen/Makefile.am
> > index 5520d39..eaa6ce4 100644
> > --- a/doc/doxygen/Makefile.am
> > +++ b/doc/doxygen/Makefile.am
> > @@ -1,7 +1,11 @@
> >
> > .SUFFIXES = .gv .png .map
> >
> > -noinst_DATA = xml/Client/index.xml xml/Server/index.xml
> > +noinst_DATA = \
> > + xml/Client/index.xml \
> > + xml/Server/index.xml \
> > + html/Client/index.html \
> > + html/Server/index.html
> > dist_noinst_DATA = wayland.doxygen.in
> >
> > scanned_src_files_shared = \
> > @@ -27,6 +31,12 @@ scanned_src_files_man =
> > \
> > $(top_srcdir)/src/wayland-client.h \
> > $(top_srcdir)/src/wayland-client-core.h
> >
> > +extra_doxygen_Server = \
> > + $(top_builddir)/protocol/wayland-server-protocol.h
> > +
> > +extra_doxygen_Client = \
> > + $(top_builddir)/protocol/wayland-client-protocol.h
> > +
> > diagramsdir := $(srcdir)/dot
> > diagramssrc := $(wildcard $(diagramsdir)/*.gv)
> > diagrams := $(patsubst $(diagramsdir)/%,xml/%,$(diagramssrc:.gv=.png))
> > @@ -38,7 +48,7 @@ diagram_maps := $(patsubst
> > $(diagramsdir)/%,xml/%,$(diagramssrc:.gv=.map))
> > dist_man3_MANS = $(shell test -d man && find man/man3 -name "wl_*.3"
> > -printf "man/man3/%P\n")
> >
> > # Listing various directories that might need to be created.
> > -alldirs := xml xml/Client xml/Server man/man3
> > +alldirs := xml xml/Client xml/Server man/man3 html/Client html/Server
> >
> > $(diagrams): $(diagramssrc)
> >
> > @@ -51,6 +61,13 @@ xml/%/index.xml: $(scanned_src_files_%) wayland.doxygen
> > $(diagrams) $(diagram_ma
> > echo "INPUT= $(scanned_src_files_$*)"; \
> > ) | $(DOXYGEN) -
> >
> > +html/%/index.html: $(scanned_src_files_%) wayland.doxygen $(diagrams)
> > $(diagram_maps) | html/%
> > + $(AM_V_GEN)(cat wayland.doxygen; \
> > + echo "GENERATE_HTML=YES"; \
> > + echo "HTML_OUTPUT=html/$*"; \
> > + echo "INPUT= $(scanned_src_files_$*) $(extra_doxygen_$*)"; \
> > + ) | $(DOXYGEN) -
> > +
> > man/man3/wl_display.3: $(scanned_src_files_man) wayland.doxygen | man/man3
> > $(AM_V_GEN)(cat wayland.doxygen; \
> > echo "GENERATE_MAN=YES"; \
> > @@ -74,6 +91,7 @@ all-local: man/man3/wl_display.3
> >
> > clean-local:
> > rm -rf xml/
> > + rm -rf html/
> > rm -rf man/
> >
> > EXTRA_DIST = $(diagramssrc)
> > diff --git a/doc/doxygen/wayland.doxygen.in b/doc/doxygen/
> > wayland.doxygen.in
> > index fb76b12..9cd7a64 100644
> > --- a/doc/doxygen/wayland.doxygen.in
> > +++ b/doc/doxygen/wayland.doxygen.in
> > @@ -13,4 +13,6 @@ MACRO_EXPANSION = YES
> > EXPAND_ONLY_PREDEF = YES
> > DOT_MULTI_TARGETS = YES
> > ALIASES += comment{1}="/* \1 *<!-- -->/"
> > -GENERATE_HTML = NO
> > +OPTIMIZE_OUTPUT_FOR_C = YES
> > +EXTRACT_ALL = YES
> > +EXTRACT_STATIC = YES
> > diff --git a/src/scanner.c b/src/scanner.c
> > index f456aa5..f7780d6 100644
> > --- a/src/scanner.c
> > +++ b/src/scanner.c
> > @@ -762,9 +762,11 @@ emit_opcode_versions(struct wl_list *message_list,
> > struct interface *interface)
> > {
> > struct message *m;
> >
> > - wl_list_for_each(m, message_list, link)
> > + wl_list_for_each(m, message_list, link) {
> > + printf("/**\n * @ingroup iface_%s\n */\n",
> > interface->name);
> > printf("#define %s_%s_SINCE_VERSION\t%d\n",
> > interface->uppercase_name, m->uppercase_name,
> > m->since);
> > + }
> >
> > printf("\n");
> > }
> > @@ -804,6 +806,7 @@ emit_stubs(struct wl_list *message_list, struct
> > interface *interface)
> > struct arg *a, *ret;
> > int has_destructor, has_destroy;
> >
> > + printf("/** @ingroup iface_%s */\n", interface->name);
> > printf("static inline void\n"
> > "%s_set_user_data(struct %s *%s, void *user_data)\n"
> > "{\n"
> > @@ -812,6 +815,7 @@ emit_stubs(struct wl_list *message_list, struct
> > interface *interface)
> > interface->name, interface->name, interface->name,
> > interface->name);
> >
> > + printf("/** @ingroup iface_%s */\n", interface->name);
> > printf("static inline void *\n"
> > "%s_get_user_data(struct %s *%s)\n"
> > "{\n"
> > @@ -838,6 +842,7 @@ emit_stubs(struct wl_list *message_list, struct
> > interface *interface)
> > }
> >
> > if (!has_destroy && strcmp(interface->name, "wl_display") != 0)
> > + printf("/** @ingroup iface_%s */\n", interface->name);
> > printf("static inline void\n"
> > "%s_destroy(struct %s *%s)\n"
> > "{\n"
> > @@ -865,6 +870,7 @@ emit_stubs(struct wl_list *message_list, struct
> > interface *interface)
> > ret = a;
> > }
> >
> > + printf("/** @ingroup iface_%s */\n", interface->name);
> > if (ret && ret->interface_name == NULL)
> > printf("static inline void *\n");
> > else if (ret)
> > @@ -950,6 +956,16 @@ emit_event_wrappers(struct wl_list *message_list,
> > struct interface *interface)
> > return;
> >
> > wl_list_for_each(m, message_list, link) {
> > + printf("/**\n"
> > + " * @ingroup iface_%s\n"
> > + " * Sends an %s event to the client owning the
> > resource.\n",
> > + interface->name,
> > + m->name);
> > + wl_list_for_each(a, &m->arg_list, link) {
> > + if (a->summary)
> > + printf("@param %s %s\n", a->name,
> > a->summary);
> > + }
> > + printf(" */\n");
> > printf("static inline void\n"
> > "%s_send_%s(struct wl_resource *resource_",
> > interface->name, m->name);
> > @@ -981,6 +997,30 @@ emit_event_wrappers(struct wl_list *message_list,
> > struct interface *interface)
> > }
> >
> > static void
> > +format_text_to_comment(const char *text, bool standalone_comment)
> > +{
> > + int bol = 1, start = 0, i;
> > +
> > + for (i = 0; text[i]; i++) {
> > + if (bol && (text[i] == ' ' || text[i] == '\t')) {
> > + continue;
> > + } else if (bol) {
> > + bol = 0;
> > + start = i;
> > + }
> > +
> > + if (text[i] == '\n' || text[i] == '\0') {
> > + printf("%s %.*s\n",
> > + i == 0 && standalone_comment ? "/*" : " *",
> > + i - start, text + start);
> > + bol = 1;
> > + }
> > + }
> > + if (standalone_comment)
> > + printf(" */\n\n");
> > +}
> > +
> > +static void
> > emit_enumerations(struct interface *interface)
> > {
> > struct enumeration *e;
> > @@ -996,28 +1036,23 @@ emit_enumerations(struct interface *interface)
> >
> > if (desc) {
> > printf("/**\n");
> > - desc_dump(desc->summary,
> > - " * %s_%s - ",
> > - interface->name, e->name);
> > - wl_list_for_each(entry, &e->entry_list, link) {
> > - desc_dump(entry->summary,
> > - " * @%s_%s_%s: ",
> > - interface->uppercase_name,
> > - e->uppercase_name,
> > - entry->uppercase_name);
> > - }
> > - if (desc->text) {
> > - printf(" *\n");
> > - desc_dump(desc->text, " * ");
> > - }
> > + printf(" * @ingroup iface_%s\n", interface->name);
> > + format_text_to_comment(desc->summary, false);
> > + if (desc->text)
> > + format_text_to_comment(desc->text, false);
> > printf(" */\n");
> > }
> > printf("enum %s_%s {\n", interface->name, e->name);
> > - wl_list_for_each(entry, &e->entry_list, link)
> > + wl_list_for_each(entry, &e->entry_list, link) {
> > + if (entry->summary)
> > + printf("\t/**\n"
> > + "\t * %s\n"
> > + "\t */\n", entry->summary);
> > printf("\t%s_%s_%s = %s,\n",
> > interface->uppercase_name,
> > e->uppercase_name,
> > entry->uppercase_name, entry->value);
> > + }
> > printf("};\n");
> > printf("#endif /* %s_%s_ENUM */\n\n",
> > interface->uppercase_name, e->uppercase_name);
> > @@ -1034,9 +1069,10 @@ emit_structs(struct wl_list *message_list, struct
> > interface *interface, enum sid
> > if (wl_list_empty(message_list))
> > return;
> >
> > + printf("/**\n");
> > +#if 0
> > if (interface->description) {
> > struct description *desc = interface->description;
> > - printf("/**\n");
> > desc_dump(desc->summary, " * %s - ", interface->name);
> > wl_list_for_each(m, message_list, link) {
> > struct description *mdesc = m->description;
> > @@ -1046,8 +1082,12 @@ emit_structs(struct wl_list *message_list, struct
> > interface *interface, enum sid
> > }
> > printf(" *\n");
> > desc_dump(desc->text, " * ");
> > - printf(" */\n");
> > }
> > +#endif
> > + printf(" * @ingroup iface_%s\n", interface->name);
> > + printf(" * @struct %s_%s\n", interface->name,
> > + (side == SERVER) ? "interface" : "listener");
> > + printf(" */\n");
> > printf("struct %s_%s {\n", interface->name,
> > (side == SERVER) ? "interface" : "listener");
> >
> > @@ -1055,24 +1095,24 @@ emit_structs(struct wl_list *message_list, struct
> > interface *interface, enum sid
> > struct description *mdesc = m->description;
> >
> > printf("\t/**\n");
> > - desc_dump(mdesc ? mdesc->summary : "(none)",
> > - "\t * %s - ", m->name);
> > - wl_list_for_each(a, &m->arg_list, link) {
> > - if (side == SERVER && a->type == NEW_ID &&
> > - a->interface_name == NULL)
> > - printf("\t * @interface: name of the
> > objects interface\n"
> > - "\t * @version: version of the
> > objects interface\n");
> > -
> > -
> > - desc_dump(a->summary ? a->summary : "(none)",
> > - "\t * @%s: ", a->name);
> > - }
> > + if (mdesc->summary)
> > + printf("\t * %s\n", mdesc->summary);
> > if (mdesc) {
> > printf("\t *\n");
> > desc_dump(mdesc->text, "\t * ");
> > }
> > + wl_list_for_each(a, &m->arg_list, link) {
> > + if (side == SERVER && a->type == NEW_ID &&
> > + a->interface_name == NULL)
> > + printf("\t * @param interface name of the
> > objects interface\n"
> > + "\t * @param version version of the
> > objects interface\n");
> > +
> > + if (a->summary)
> > + printf("\t * @param %s %s\n", a->name,
> > + a->summary);
> > + }
> > if (m->since > 1) {
> > - printf("\t * @since: %d\n", m->since);
> > + printf("\t * @since %d\n", m->since);
> > }
> > printf("\t */\n");
> > printf("\tvoid (*%s)(", m->name);
> > @@ -1112,6 +1152,9 @@ emit_structs(struct wl_list *message_list, struct
> > interface *interface, enum sid
> > printf("};\n\n");
> >
> > if (side == CLIENT) {
> > + printf("/**\n"
> > + " * @ingroup %s_iface\n"
> > + " */\n", interface->name);
> > printf("static inline int\n"
> > "%s_add_listener(struct %s *%s,\n"
> > "%sconst struct %s_listener *listener, void *data)\n"
> > @@ -1128,29 +1171,6 @@ emit_structs(struct wl_list *message_list, struct
> > interface *interface, enum sid
> > }
> >
> > static void
> > -format_copyright(const char *copyright)
> > -{
> > - int bol = 1, start = 0, i;
> > -
> > - for (i = 0; copyright[i]; i++) {
> > - if (bol && (copyright[i] == ' ' || copyright[i] == '\t')) {
> > - continue;
> > - } else if (bol) {
> > - bol = 0;
> > - start = i;
> > - }
> > -
> > - if (copyright[i] == '\n' || copyright[i] == '\0') {
> > - printf("%s %.*s\n",
> > - i == 0 ? "/*" : " *",
> > - i - start, copyright + start);
> > - bol = 1;
> > - }
> > - }
> > - printf(" */\n\n");
> > -}
> > -
> > -static void
> > emit_types_forward_declarations(struct protocol *protocol,
> > struct wl_list *message_list,
> > struct wl_array *types)
> > @@ -1203,6 +1223,31 @@ get_include_name(bool core, enum side side)
> > }
> >
> > static void
> > +emit_mainpage_blurb(const struct protocol *protocol, enum side side)
> > +{
> > + printf("/**\n"
> > + " * @mainpage\n"
> > + " * Wayland protocol '%s' %s API documentation\n"
> > + " * For the list of available interfaces, please see the\n"
> > + " * <a href=\"modules.html\">modules</a> list.\n",
> > + protocol->name,
> > + (side == SERVER) ? "server" : "client");
> > +
> > + if (protocol->description) {
> > + printf(" * @section description Description\n");
> > + format_text_to_comment(protocol->description->text, false);
> > + }
> > +
> > + if (protocol->copyright) {
> > + printf(" * @section copyright Copyright\n");
> > + printf(" * <pre>\n");
> > + format_text_to_comment(protocol->copyright, false);
> > + printf(" * </pre>\n");
> > + }
> > + printf(" */\n");
> > +}
> > +
> > +static void
> > emit_header(struct protocol *protocol, enum side side)
> > {
> > struct interface *i, *i_next;
> > @@ -1210,9 +1255,6 @@ emit_header(struct protocol *protocol, enum side
> > side)
> > const char *s = (side == SERVER) ? "SERVER" : "CLIENT";
> > char **p, *prev;
> >
> > - if (protocol->copyright)
> > - format_copyright(protocol->copyright);
> > -
> > printf("#ifndef %s_%s_PROTOCOL_H\n"
> > "#define %s_%s_PROTOCOL_H\n"
> > "\n"
> > @@ -1229,6 +1271,8 @@ emit_header(struct protocol *protocol, enum side
> > side)
> > protocol->uppercase_name, s,
> > get_include_name(protocol->core_headers, side));
> >
> > + emit_mainpage_blurb(protocol, side);
> > +
> > wl_array_init(&types);
> > wl_list_for_each(i, &protocol->interface_list, link) {
> > emit_types_forward_declarations(protocol,
> > &i->request_list, &types);
> > @@ -1252,6 +1296,11 @@ emit_header(struct protocol *protocol, enum side
> > side)
> > printf("\n");
> >
> > wl_list_for_each(i, &protocol->interface_list, link) {
> > + printf(" /**\n"
> > + " * @defgroup iface_%s The %s interface\n"
> > + " * API provided by the %s interface\n"
> > + " */\n",
> > + i->name, i->name, i->name);
> > printf("extern const struct wl_interface "
> > "%s_interface;\n", i->name);
> > }
> > @@ -1395,7 +1444,7 @@ emit_code(struct protocol *protocol)
> > char **p, *prev;
> >
> > if (protocol->copyright)
> > - format_copyright(protocol->copyright);
> > + format_text_to_comment(protocol->copyright, true);
> >
> > printf("#include <stdlib.h>\n"
> > "#include <stdint.h>\n"
> > --
> > 2.4.3
> >
> > _______________________________________________
> > wayland-devel mailing list
> > wayland-devel at lists.freedesktop.org
> > http://lists.freedesktop.org/mailman/listinfo/wayland-devel
> >
More information about the wayland-devel
mailing list