<div dir="ltr"><div><div><div><div><div><div><div><div>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. </div>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: </div>- 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. </div>- 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. </div><div>- 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. </div></div>Problems I see: </div>- 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++. </div>- 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. </div>- 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. <div><div><div><div><div><div><div><div><div><div><div> </div></div></div></div></div></div></div></div></div></div></div></div><div class="gmail_extra"> <div class="gmail_quote">On Wed, Oct 28, 2015 at 6:48 PM, Peter Hutterer <<a href="mailto:peter.hutterer@who-t.net" target="_blank">peter.hutterer@who-t.net</a>> wrote: <blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">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/<a href="http://wayland.doxygen.in" rel="noreferrer" target="_blank">wayland.doxygen.in</a> | 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 = <a href="http://wayland.doxygen.in" rel="noreferrer" target="_blank">wayland.doxygen.in</a> 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/<a href="http://wayland.doxygen.in" rel="noreferrer" target="_blank">wayland.doxygen.in</a> b/doc/doxygen/<a href="http://wayland.doxygen.in" rel="noreferrer" target="_blank">wayland.doxygen.in</a> index fb76b12..9cd7a64 100644 --- a/doc/doxygen/<a href="http://wayland.doxygen.in" rel="noreferrer" target="_blank">wayland.doxygen.in</a> +++ b/doc/doxygen/<a href="http://wayland.doxygen.in" rel="noreferrer" target="_blank">wayland.doxygen.in</a> @@ -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 <a href="mailto:wayland-devel@lists.freedesktop.org">wayland-devel@lists.freedesktop.org</a> <a href="http://lists.freedesktop.org/mailman/listinfo/wayland-devel" rel="noreferrer" target="_blank">http://lists.freedesktop.org/mailman/listinfo/wayland-devel</a> </blockquote></div> </div>