On Sun, Jan 20, 2013 at 9:48 AM, Torsten Schoenfeld <kaffeeti...@gmx.de> wrote:
> On 20.01.2013 16:07, Dave M wrote:
>>
>> Just a few questions with the InfoBar stuff:
>
>
> It looks like something went wrong with the patch file. There seems to be
> stuff missing at the beginning and at the end. Also, "unified" diffs (via
> "diff -u") are easier to read. And for files of this size, there's no need
> to tar them up.
You're right - I forgot to use the "-u" on that one.
>
>
>> 1. The only ones I did not write in the format above for
>> Dialog/InfoBar are "new" and "new_with_buttons". When you combine
>> those, it looks like a train wreck - much more readable with those two
>> separate and in the same manner as Dialog. How does that sound? I
>> don't mind changing it, but I always prefer readability.
>
>
> The advantage of writing it generically of course is that it avoids
> duplicating logic. But I'll leave this up to you.
>
I still prefer separation, but just say the word and I'll re-write
them together.
>
>> 2. I actually figured out adding '::_gtk3_perl_response_converter' to
>> InfoBar by myself. No question here; just pointing it out.
>
>
> I think you could simply use
>
> ['Gtk3::InfoBar', 'response',
> \&Gtk3::Dialog::_gtk3_perl_response_converter]
>
> in @_GTK_USE_GENERIC_SIGNAL_MARSHALLER_FOR and thus avoid having to copy
> this function.
>
Much better than what I had.
>
>> 3. The copyright years show 2003-2012. Did you want to update that to
>> 2013?
>
>
> Yes.
>
> One final note: It would be more economical to wrap a single loop
>
> {
> no strict qw(refs);
>
> foreach my $dialog_package (qw/Dialog InfoBar/) {
> ...
> }
> }
>
> around all of the functions that are to be duplicated, instead of having
> separate loops for each function.
>
Right again. All changes are enclosed. Thanks for all the comments.Thanks, Dave M
GtkInfoBar.t
Description: Troff document
InfoBar-patch.diff
Description: Binary data
_______________________________________________ gtk-perl-list mailing list gtk-perl-list@gnome.org https://mail.gnome.org/mailman/listinfo/gtk-perl-list
