Re: [PATCH] NM DBUS api documentation build system

[Dan Williams <dcbw redhat com>]

On Fri, 2008-02-08 at 18:43 +0100, Will Stephenson wrote:
> On Friday 08 February 2008 17:52:22 Dan Williams wrote:
> > On Tue, 2008-02-05 at 15:46 +0100, Will Stephenson wrote:
> > > Here's a patch that adds api documentation generation to the NM build
> > > system. The code is taken from the Telepathy project.
> >
> > I think as others have expressed, I think that process adds an extra,
> > unnecessary step.  I think I'd rather see the following:
> >
> > 1) annotate the introspection/*.xml files using the many-times-proposed
> > "org.freedesktop.DBus.Doc" annotation name which was designed just for
> > this.  You'd end up with stuff looking like this:
> >
> > <interface name="org.freedesktop.NetworkManager.Device">
> >   <method name="Deactivate">
> >     <annotation name="org.freedesktop.DBus.GLib.CSymbol"
> > value="impl_device_deactivate"/> <annotation
> > name="org.freedesktop.DBus.Doc">
> >       Deactivates the device, removing applicable routes, IP addresses, and
> > marking the device down. </annotation>
> >   </method>
> >
> >   <property name="Udi" type="s" access="read">
> >     <annotation name="org.freedesktop.DBus.Doc">
> >       The HAL Unique Device Identifier for this device.
> >     </annotation>
> >   </property>
> > ...
> > </interface>
> 
> Google knows exactly one hit for "org.freedesktop.DBus.Doc", that doesn't say 
> much.  Where is this documented?  From this example it seems to be less 
> descriptive than Telepathy's format, but I'll reserve judgement until I've 
> had a look at it.
> 
> If you've got an archive of the dbus list, look for the thread "documentation 
> and introspection" from Nov 2006 where Simon McVittie compares the tp doc 
> format and another namespaced doc proposal.

So a simple patch to dbus-binding-tool to make it ignore namespaced
nodes and attributes will allow us to use the TP namespace within the
existing introspection XML files and avoid the extra step of spec/*.

If you're still willing to work on this, I'd love a patch that would run
the XSLT stuff over the introspection/*.xml (feel free to add all.xml to
introspection/* if you like) and generate the HTML docs.

I don't think it would be to much work to modify the makefile stuff to
rip out the bits that convert spec/* -> introspection/* and just
preserve the bits that do introspection/* -> HTML, right?

Thanks!
Dan

> 
> > 2) Assuming dbus-glib's binding tool ignores annotations it doesn't
> > recognize (and if it doesn't, we fix it to do so), the binding tool will
> > just continue as normal creating the glue and bindings file.
> 
> It should ignore namespaces it doesn't recognise too, making tp: namespaced 
> introspection xml safe even without the (IMO trivial) extra xslt step that 
> removes them.
> 
> > 3) The optional document generation step (enabled at configure-time with
> > --enable-docs or something) can run xslt or something on the
> > introspection XML and generate the HTML documentation.
> 
> Right, this is how the tp system works too.
> 
> > There are some bits of the code that could stand extra documentation
> > that aren't part of the D-Bus API directly, like the NMSettings
> > subclasses.  They are of course D-Bus dictionaries, and the key/value
> > pairs and types are firmly defined.  While that spec doesn't live in
> > SVN, it really should live there, and it really should be pulled into
> > the autogenerated docs for consistency.
> 
> tp uses similar settings dictionaries, and defines their key ranges in these 
> types' docstrings.  Have a look at http://telepathy.freedesktop.org/spec.html 
> for its output.
> 
> Will

diff --git a/dbus/dbus-gparser.c b/dbus/dbus-gparser.c
index f296f96..cdd4e53 100644
--- a/dbus/dbus-gparser.c
+++ b/dbus/dbus-gparser.c
@@ -128,13 +128,17 @@ locate_attributes (const char  *element_name,
 
       if (!found)
         {
-          g_set_error (error,
-                       G_MARKUP_ERROR,
-                       G_MARKUP_ERROR_PARSE,
-                       _("Attribute \"%s\" is invalid on <%s> element in this context"),
-                       attribute_names[i], element_name);
-          retval = FALSE;
-          goto out;
+          /* We want to passthrough namespaced XML nodes that we don't know anything about. */
+          if (strchr (attribute_names[i], ':') == NULL)
+            {
+              g_set_error (error,
+                           G_MARKUP_ERROR,
+                           G_MARKUP_ERROR_PARSE,
+                           _("Attribute \"%s\" is invalid on <%s> element in this context"),
+                           attribute_names[i], element_name);
+              retval = FALSE;
+              goto out;
+            }
         }
 
       ++i;
@@ -177,6 +181,7 @@ struct Parser
   PropertyInfo *property;
   ArgInfo *arg;
   gboolean in_annotation;
+  guint unknown_namespaced_depth;
 };
 
 Parser*
@@ -791,10 +796,14 @@ parser_start_element (Parser      *parser,
     }
   else
     {
-      g_set_error (error, G_MARKUP_ERROR,
-                   G_MARKUP_ERROR_PARSE,
-                   _("Element <%s> not recognized"),
-                   element_name);
+      if (strchr (element_name, ':') != NULL)
+        /* Passthrough XML-namespaced nodes */
+        parser->unknown_namespaced_depth += 1;
+      else
+        g_set_error (error, G_MARKUP_ERROR,
+                     G_MARKUP_ERROR_PARSE,
+                     _("Element <%s> not recognized"),
+                     element_name);
     }
   
   return TRUE;
@@ -844,6 +853,11 @@ parser_end_element (Parser      *parser,
       if (parser->node_stack == NULL)
         parser->result = top; /* We are done, store the result */      
     }
+  else if (strchr (element_name, ':') != NULL)
+    {
+      /* Passthrough XML-namespaced nodes */
+      parser->unknown_namespaced_depth -= 1;
+    }
   else
     g_assert_not_reached (); /* should have had an error on start_element */