I finally got around to looking in more detail at the eggrecentchooser stuff.
My first comment is that 150+ functions and 10000+ lines of code feel a bit large just for recent-files support. This is partially due to the 3000 lines of XBEL parser, but also due to copying the file chooser approach of having a EggRecentChooser interface with three implementations (EggRecentChooserMenu for menus, EggRecentChooserDialog for a standalone dialog, and EggRecentChooserWidget for an embedded list) and a separate filter object. Are there any use cases for the standalone dialog ? Who uses filters on recent files ? I find it a bit confusing that there are two levels of filtering. In particular I wonder if the filter function set on the manager affects what items get written to disk ? Why are the show-tips and show-numbers properties on the menu chooser and not on the chooser interface ? Apart from the fact the we don't exactly support tooltips on menus, don't they make sense in a dialog as well ? Obviously, you would run into the problem that we don't currently support tooltips on treeviews either, but I think it would make sense to move these properties to the interface and just ignore them if the implementation doesn't support them. Apart from that, I just have a long list of small stylistic comments: - Why is egg_recent_manager_changed() exported, the changed signal is not an action signal ? - Naming, item vs info. The egg_recent_manager_functions are all named _item (), but the object is called Info. - Should use G_DEFINE_TYPE() to save on boilerplate. - No point in using atomic refcounting for EggRecentInfo, since this will become part of GTK+, so will go under the GDK lock. - We should problably use the GLib stdio wrappers for stat() et al, even if Win32 will get a native implementation of recent files. - The previous point reminds me that someone needs to investigate this point: Can the recent manager api be implemented using the native Win32 API ? - Just use if (!...) instead of if (FALSE == ...). Similarly for == TRUE. - Omit {} around if branches with single statements. Can you commit your GMarkup XBEL parser to libegg ? Regards, Matthias _______________________________________________ gtk-devel-list mailing list gtk-devel-list@gnome.org http://mail.gnome.org/mailman/listinfo/gtk-devel-list