Re: gtkmm API freeze on July 30th

[Murray Cumming <murrayc murrayc com>]

On Wed, 2007-07-25 at 10:07 -0500, Jonathon Jongsma wrote:
> Here's my brief review.  Overall, I didn't find too many API issues,
> most of it is a matter of documentation accidentally carrying over
> GTK+-isms.
> 
> Builder:
> - get_type_from_name(): what is the use case for this?  Is it
> necessary to wrap this?

Yes, this does not seem useful. I have removed it from gtkmm and 
added a documentation bug for GTK+:
http://bugzilla.gnome.org/show_bug.cgi?id=461222

> - A new interface has been introduced in GTK+: GtkBuildable, that
> nearly all widgets now implement.  Do we need to implement this in
> gtkmm as well?  Is it even possible without breaking ABI?

We can't add base classes without breaking ABI in C++. And this does not
have methods that applications need to call.

We should probably provide it as a class for people who want to
implement the GtkBuildable interface, to make their C++ widgets
available to Glade. I guess that should work.

> ScaleButton constructor takes a GtkIconSize (GTK+) type instead of a
> Gtk::IconSize (gtkmm) type.

Fixed.

> Widget::set_has_tooltip(): When would a user explicitly call this, and
> why?  Doesn't the has-tooltip property automatically get set when you
> call the Widget::set_tooltip_text() and similar functions?

Yes, it's set automatically, according to the documentation for
gtk_widget_set_tooltip_text(). But maybe it's used when implementing
other widgets. I've asked on gtk-devel-list.

> Documentation + functionality:
> - PageSetup::save_to_key_file(), PrintSettings::to_key_file():
> documentation says groupname can be 0 to use default group name, but
> it doesn't look like the implementation is customized to send 0 when
> the user passes an empty Glib::ustring() to this function

I added a method overload to both classes, without the group_name
paramter, and corrected the documentation. I also added the save_ prefix
to the PrintSettings methods, for consistency.

> - TreeView::set_tooltip_cell(): the 'cell' parameter is of type
> CellRenderer*, but the documentation says that this is a
> 'CellRendererText'.  must it be?

I don't know. I filed a GTK+ bug:
http://bugzilla.gnome.org/show_bug.cgi?id=461225

> - Widget::modify_cursor(): documentation says 'or 0 to undo the effect
> of previous calls to of modify_cursor()", but the function takes
> references.  Is there a way to undo this in gtkmm?

I had already added unset_cursor(). I have added a "@see unset_cursor()"
to the documentation for modify_cursor(), though I must check that that 
documentation is being used.

> Documentation:
> - AboutDialog::get_program_name() and others: refers to the dialog
> 'owning' the string and warning the user not to modify it
> - Action::create_menu(): does the user need to free the menu?
> - EntryCompletion::get_completion_prefix(): "remove 'or 0 if...'"
> - IconTheme::choose_icon(): "icon_names 0-terminated array of icon
> names to lookup.", also "Returns: A Gtk::IconInfo structure containing
> information about the icon, or 0 if the icon wasn't found. Free with
> gtk_icon_info_free()"
> - IconTheme::list_contexts(): I have no idea what 'contexts' are, it'd
> be nice to explain it somewhere.  Also: "Returns: A G::List list
> holding the names of all the contexts in the theme. You must first
> free each element in the list with Glib::free(), then free the list
> itself with Glib::list_free()"
> IconView::set_tooltip_cell() & set_tooltip_item(): "See also
> gtk_tooltip_set_tip_area()."
> - PageSetup::save_to_file(), PrintSettings::to_file(): has
> documentation for non-existant 'error' parameter
> - Printer::request_details(): documentation refers to the GTK+ signal
> name "Gtk::Printer::details-acquired"
> - CellLayout::get_cells() contains the following text in
> documentation: "The list, but not the renderers has been newly
> allocated and should be freed with Glib::list_free() when no longer
> needed"
> - Widget::error_bell(): refers to Gtk::Settings:gtk-error-bell GTK+
> property, which isn't wrapped in gtkmm.  Documentation also refers to
> the C function gdk_window_beep()
> - Widget::get_tooltip_markup(), Widget::get_tooltip_text(): incorrect
> text "The tooltip text, or 0. You should free the returned string with
> Glib::free() when done."

I'll fix these next.

-- 
Murray Cumming
murrayc murrayc com
www.murrayc.com
www.openismus.com