Re: RecentChooserDialog

On Sun, Jan 20, 2013 at 9:48 AM, Torsten Schoenfeld <kaffeetisch 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',

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


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.

Dave M

Attachment: GtkInfoBar.t
Description: Troff document

Attachment: InfoBar-patch.diff
Description: Binary data

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