Re: RecentChooserDialog

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

[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]