Re: RecentChooserDialog

[Torsten Schoenfeld <kaffeetisch gmx de>]

On 14.01.2013 18:24, Dave M wrote:
> Here's a first stab at the Gtk3::RecentChooserDialog stuff.  Attached
> is a diff and the test file.

Looking good in general.  One minor style nibble:

+    $dialog->add_button( $buttons[$i], $buttons[$i+1] );

Please follow the style of the surrounding code, so in this case:

  $dialog->add_button ($buttons[$i], $buttons[$i+1]);

Similarly for "if" and "for".

> The test is currently doing two bad things:
>
> 1. Several "Gtk-WARNING **: Attempting to add a widget with type
> GtkRecentChooserDialog to a GtkWindow, but as a GtkBin subclass a
> GtkWindow can only contain one widget at a time; it already contains a
> widget of type GtkRecentChooserDialog" warnings.

This is due to setting the parent in

+  my $dialog = Glib::Object::new ($class, title => $title,
+    parent => $parent);

Simply remove "parent => $parent".

> 2. The tests fail:
> not ok 14
> #   Failed test at GtkRecentChooserDialog.t line 40.
> #          got: '1'
> #     expected: '2'
> # Looks like you failed 1 test of 14.
> I think this is because it uses a Box, and the "children" are held in
> the box (that sounds horrible - sorry).  Note that the original code
> was
> "$chooser -> action_area -> get_children();" but it doesn't like the
> "action_area" stuff, so I changed it to "get_action_area", and it's
> only returning the box.

This was actually a bug (or missing override, rather) in Gtk3.  I fixed 
it with 
<http://git.gnome.org/browse/perl-Gtk3/commit/?id=6363e69c9028380b238331e54e2307a7ca2524b1> 
last year.

> The code for both 'new' and 'new_for_manager' are very similar, so we
> should be able to write them in the same manner as the
> "Gtk3::RadioMenuItem constructors".  Probably wouldn't save many lines
> though, so I'm not sure it's worth it.

I agree; two separate functions are fine in this case.