libegg/recentchooser
- From: Matthias Clasen <mclasen redhat com>
- To: gtk-devel-list gnome org
- Cc:
- Subject: libegg/recentchooser
- Date: Tue, 29 Nov 2005 12:18:30 -0500
Hey Emanuele,
in an attempt to get this moving forward again, I spent some time
looking at the code thats currently in libegg, and wrote down some
random notes.
Matthias
* eggdesktopbookmarks.[hc]:
- Where is the GMarkup parser ? libegg still seems to have libxml
- Should the name be egg_desktop_bookmarks_... ? Considering it does
not contain a single bookmark, but a whole list of resources. But
then maybe it should not be named "bookmark" at all, since this is
about recently used resources ?!
- Is there any particular reason why all the load functions return the
item count as well, when there is egg_desktop_bookmark_get_count() ?
Might be nice to avoid that out parameter.
- Is EGG_DESKTOP_BOOKMARK_ERROR_DUMP just a catch-all for bugs which
have no other code ? If so, you may want to move it to the first
position. Or is it a generic code for all error having to do with
writing/modifying bookmarks ? No, that would be _WRITE...
- I'm not convinced that having GErrors in all the getters is adding
a lot of value. In most cases, a NULL return value already tells
the whole story, no ?
* eggrecentchooser.[hc], eggrecentchooserprivate.h
- I wonder why there is both EggRecentChooserSortType and
EggRecentSortType, and why they are not the same.
- Why do we need to add so many "UI tweak" properties ?
I can see that show-private, show-not-found and local-only
make sense, but what is the rationale for show-tips,
show-numbers and show-icons ? Also the documentation for these
is a bit weak. It does not mention that e.g. the visibility
of icons in menus still depends on global settings. I also
wonder how well the numbering code works in RTL locales.
Also, it seems that most of these properties are only implemented
for some of the EggRecentChooser implementations.
- The item-activated signal is in eggrecentchooserprivate.h, yet
it seems to be required api to make menus work, like in
test-recent-menu.c
* eggrecentmanager.[hc]
- EGG_RECENT_MANAGER_ERROR_NOT_REGISTERED
EGG_RECENT_MANAGER_ERROR_BAD_EXEC_STRING
Should these really be treated as hard error conditions ? I think
it would make sense to interpret data->app_name == NULL as use
g_get_application_name(), and I think not setting a command line
should be fine. Why does every file have to have an associated
commandline ?
* Random coding style comments
- Please don't use boolean == TRUE
- Please use G_DEFINE_TYPE where appropriate
When running test-recent-menu I noticed that the widget makes
not-found items insensitive, while the menu does not.
[
Date Prev][
Date Next] [
Thread Prev][
Thread Next]
[
Thread Index]
[
Date Index]
[
Author Index]