Re: RecentChooserDialog

[Dave M <dave nerd gmail com>]

On Tue, Jan 15, 2013 at 8:54 AM, Torsten Schoenfeld <kaffeetisch gmx de> wrote:
> 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".
My apologies, I should have that down by now.
> > 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".
Works.
> > 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.
Here it is, completely working and properly formatted.  Note that I
changed the test file to match the Gtk3 test formatting, which differs
from Gtk2 test formatting (e.g., single vs double quotes, different
cuddling, etc).  I'm not sure if I did the chaining format properly
though:
$chooser-> get_action_area-> get_children();

I'm going to submit a demo for this to my git repository as well.

Finally, I hope to submit InfoBar patches this weekend.

Thanks,
Dave M

Attachment: RecentChooserDialog2.tar.gz
Description: GNU Zip compressed data