A closer look at the recent files proposal
- From: Matthias Clasen <mclasen redhat com>
- To: gtk-devel-list gnome org
- Cc:
- Subject: A closer look at the recent files proposal
- Date: Tue, 20 Sep 2005 14:29:26 -0400
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
[
Date Prev][
Date Next] [
Thread Prev][
Thread Next]
[
Thread Index]
[
Date Index]
[
Author Index]