Re: simple markup format

["Dominic Ludlam" <dom recoil org>]

From: "Havoc Pennington" <hp@redhat.com>

Simple point first - in g_markup_node_get_attributes there is the following
loop:

>  i = 0;
>  tmp_list = node->attributes;
>  while (tmp_list)
>    {
>      GMarkupAttribute *attr = tmp_list->data;
>
>      g_assert (i < len + 1);
>      
>      if (namesp)
>        names[i] = g_strdup (attr->name);
>
>      if (valuesp)
>        values[i] = g_strdup (attr->value);
>      
>      tmp_list = g_list_next (tmp_list);
>    }

'i' doesn't seem to get incremented in here.

If this is a generic XML subset parser, then it may be nice to support the xml
shorthand for tags with no children

    <element foo="bar" />

This looks relatively simple to add:

> static GList*
> parse_attribute_list (const gchar *text,
>                       gint i,
>                       gint length,
>                       GMarkupParseFlags flags,
>                       gint *new_i,
>                       GError **error)
> {
>
> ...
>
>       i = skip_spaces (text, i, length);
>
>       T("after attr list trailing ws", i);
>
>       c = g_utf8_get_char (&text[i]);

This

>       if (c == '>')
>         break;

would be

        if (c == '>' || c == '/')
          break;

>     }
>
>   *new_i = i;
>
>   T("after attr list", i);
>
>   return list;
> }
>
> ...
>
> static GMarkupNode*
> parse_element (const gchar *text,
>                gint i,
>                gint length,
>                GMarkupParseFlags flags,
>                gint *new_i,
>                GError **error)
> {
>
> ...
>
>   err = NULL;
>   attr_list = parse_attribute_list (text, i, length,
>                                     flags, &j, &err);
>   i = j;
>
>   if (err)
>     {
>       if (error)
>         *error = err;
>       else
>         g_error_free (err);
>
>       return NULL;
>     }
>

And this

>   c = g_utf8_get_char (&text[i]);
>   if (c != '>')
>     {
>       set_error (text, i, length,
>                  error,
>                  G_MARKUP_ERROR_PARSE,
>                  _("Document ended just after attribute list, no '>' seen"));
>
>       free_attribute_list (attr_list);
>
>       return NULL;
>     }
>

would be

    if (c == '/')
      {
        i = next_char (text, i);

        if (i >= length)
          {
            set_error (text, i, length,
                       error,
                       G_MARKUP_ERROR_PARSE,
                       _("Document ends just after '/' in self-closing tag"));

            free_attribute_list (attr_list);

            return NULL;
          }

        c = g_utf8_get_char (&text[i]);

        if (c != '>')
          {
            set_error (text, i, length,
                       error,
                       G_MARKUP_ERROR_PARSE,
                       _("Self-closing tag must end with '/>', '>' character is missing"));

            free_attribute_list (attr_list);

            return NULL;
          }

        // completed this tag, create it and return
        gchar *open_name = g_strndup (&text[name_start],
                                      name_end - name_start);

        node = g_markup_node_new_element (open_name);

        g_free (open_name);

        // skip to char after '>'
        i = next_char (text, i);
        *new_i = i;

        node->children = child_list;
        node->attributes = attr_list;

        return (GMarkupNode*) node;
      }
    else if (c != '>')
      {
        set_error (text, i, length,
                   error,
                   G_MARKUP_ERROR_PARSE,
                   _("Document ended just after attribute list, no '>' seen"));

        free_attribute_list (attr_list);

        return NULL;
     }

>   i = next_char (text, i);
>
>   T("start of child list", i);
>   child_list = parse_child_list (text, i, length,
>                                  flags, &j, &err);
>   i = j;
>
> ...

Another point - at the end of parse_element there is

    g_markup_node_get_attributes (node, NULL, NULL, NULL);
  
which doesn't look like it does much so I didn't put it in the code
above - is it important?

Dom.