Created attachment 206141 [details] [review]
GtkBuilder: change format of menus

Change the format of GtkBuilder <menu> to be more in-line with the style
of the rest of GtkBuilder so that we can do translation in a consistent
way.

The format is now substantially more difficult to hand-write, but tools
should be along soon.

There is an xslt program attached to the bug to help you convert your
existing .ui files from the old format to the new one.

Created attachment 206142 [details]
xslt transform for converting menus to the new format

Review of attachment 206141 [details] [review]:

Otherwise, looks fine to me. Any idea what to do with the xslt - just post it to the list, maybe ?

::: gtk/gtkbuilder-menus.c
@@ +341,3 @@
+  g_free (state->context);
+
+  g_slice_free (GtkBuilderMenuState, state);

Should this print out some error message ?
What does the rest of gtkbuilder do ?

Review of attachment 206141 [details] [review]:

::: gtk/gtkbuilder-menus.c
@@ +341,3 @@
+  g_free (state->context);
+
+  g_slice_free (GtkBuilderMenuState, state);

This is the subparser error handler.  Its job is to clean up the data allocated by the subparser.  The error is still reported to the error function of the main parser and returned from g_markup_parse_context_parse() in the usual way.

Attachment 206141 [details] pushed as eed3077 - GtkBuilder: change format of menus

(In reply to comment #3)
> Otherwise, looks fine to me. Any idea what to do with the xslt - just post it
> to the list, maybe ?

I wrote a mail to the list: https://mail.gnome.org/archives/gtk-devel-list/2012-January/msg00118.html

Since we are changing the format and moving the code to gtk, can you also consider the use of <object type=menu> instead of <menu>? When we chatted on irc you said that the main reason to use a different tag not consistent with gtk_builder_get_object() was that the code was in glib...

I was going to do it like:

  <object type='GMenu'>

but I didn't like that for two reasons.

The first is that GMenu doesn't (really, can't) implement GtkBuildable, so it would still be an exceptional case without looking like one.

The second issue is that it would clash with the nesting of menus as submenus or sections -- <submenu> <section> or <link> can all result in the creation of a new GMenu.

The content of the section is also syntactically distinct, which implies (at least to me) that there should be a different tag to introduce it -- from the standpoint of trying to specify the syntax of the file format in a DTD, for example.  I don't consider this to be a substantial point since other uses of GtkBuildable are already breaking this rule...

glade is going to have to be taught about the new format as it is, so I don't see any harm in having a slightly different way of introducing the section.  The main difficulty with the old format (life being miserable for translators) has been overcome with this change.

(In reply to comment #8)
> I was going to do it like:
> 
>   <object type='GMenu'>
> 
> but I didn't like that for two reasons.
> 
> The first is that GMenu doesn't (really, can't) implement GtkBuildable, so it
> would still be an exceptional case without looking like one.

Firstly, objects don't have to be buildable to be built by the builder
(I know everyone knows that already anyway) and secondly, this would be
a perfect time to pull back GtkBuilder/GtkBuildable interface into
the glib stack.

I don't have the irc log on hand, but I distinctly recall that an
excuse for rushing GtkBuilder apis into the GTK+ (2.10 ?) release
was that "even though we all agree this api belongs in glib, we
can always pull it back into glib and wrap these apis with the
GtkBuilder/GtkBuildable apis", GtkBuildable could essentially
just be a forward declaration/typedef.

Pulling these apis back into say, libgio or libgobject constitutes
no real coding work anyway, it's really just a matter of simply wrapping
the GtkBuilder api from GTK+ and editing some makefiles...

> 
> The second issue is that it would clash with the nesting of menus as submenus
> or sections -- <submenu> <section> or <link> can all result in the creation of
> a new GMenu.

This sounds like perfectly valid GtkBuilder syntax to me, this type of
case is exactly why we introduced <child type="keyword"> apis at the
GtkBuilder level (and handle that with the
GtkBuildable->add_child (buildable, child_type, child) method).

In normal GtkBuilder terms you would simply define <child type="section">
or <child type="submenu"> to distinguish in what context the child GMenu
would be added to it's parent menu.

> The content of the section is also syntactically distinct, which implies (at
> least to me) that there should be a different tag to introduce it -- from the
> standpoint of trying to specify the syntax of the file format in a DTD, for
> example.  I don't consider this to be a substantial point since other uses of
> GtkBuildable are already breaking this rule...
> 
> glade is going to have to be taught about the new format as it is, so I don't
> see any harm in having a slightly different way of introducing the section. 
> The main difficulty with the old format (life being miserable for translators)
> has been overcome with this change.

Integrating GMenu into Glade using normal builder syntax will probably cost
25% of the effort of integrating GMenu into Glade using a custom syntax
(that's modest, it could be more like 10%).

The hard parts consist of:
  a.) Special casing the different contexts in which to add submenus,
      at least we have machinery to handle the regular <child type="foo">
      in place already.

  b.) What object is created by this builder fragment ? is it just some
      kind of metadata that is "there" ? in any case from Glade's POV
      there must be an object type involve which it's building, possibly
      this is the custom content of a GtkMenuBar ?

      Even so, managing the whole "menu hierarchy" as a custom hierarchy
      from Glade is a very nonsensical thing to to, instead handling
      the addition and removal of individual GMenu objects, handling
      their own custom syntaxes when they come up, makes much more sense
      and constitutes less code to write.

It's not possible to use the normal syntax because the children of a GMenu are not GObjects.