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', \&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
Attachment:
GtkInfoBar.t
Description: Troff document
Attachment:
InfoBar-patch.diff
Description: Binary data