[RFC wayland] doc: generate doxygen html output from the scanner

Bill Spitzak spitzak at gmail.com
Thu Oct 29 11:00:51 PDT 2015


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.

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.

- 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++.

- 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.

- 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.


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
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.freedesktop.org/archives/wayland-devel/attachments/20151029/a2ce8ccb/attachment-0001.html>


More information about the wayland-devel mailing list