Re: libegg/recentchooser
Hi Morten, On Wed, 2005-11-30 at 19:58 -0500, Morten Welinder wrote: Uhm, I'll put that code under #idef G_OS_UNIX/#endif guards for the time being, but the getenv(TZ)/setenv(TZ) timezone trick should work on any sufficiently recent POSIX-like system; users of other operating systems might just drop me an email. setenv isn't POSIX. You can probably just use g_setenv although that will leak. But really, just do it as ((days_since_19700101 * 24 + h) * 60 + m) * 60 + s where days_since_19700101 comes out of GDate. I've already updated the timestamp_from_iso8601 function using libsoup's equivalent code, after James pointed that out. The fixes are in libegg HEAD (libegg/bookmarkfile). Thanks, Emmanuele. -- Emmanuele Bassi - [EMAIL PROTECTED] Log: http://log.emmanuelebassi.net ___ gtk-devel-list mailing list gtk-devel-list@gnome.org http://mail.gnome.org/mailman/listinfo/gtk-devel-list
Re: libegg/recentchooser
Emmanuele Bassi wrote: Hi Morten, On Wed, 2005-11-30 at 19:58 -0500, Morten Welinder wrote: Uhm, I'll put that code under #idef G_OS_UNIX/#endif guards for the time being, but the getenv(TZ)/setenv(TZ) timezone trick should work on any sufficiently recent POSIX-like system; users of other operating systems might just drop me an email. setenv isn't POSIX. You can probably just use g_setenv although that will leak. But really, just do it as ((days_since_19700101 * 24 + h) * 60 + m) * 60 + s where days_since_19700101 comes out of GDate. I've already updated the timestamp_from_iso8601 function using libsoup's equivalent code, after James pointed that out. The fixes are in libegg HEAD (libegg/bookmarkfile). Perhaps the right solution is to move some of libsoup's date handling code into glib as official APIs. Parsing the various date formats is a useful feature. James. ___ gtk-devel-list mailing list gtk-devel-list@gnome.org http://mail.gnome.org/mailman/listinfo/gtk-devel-list
Re: libegg/recentchooser
Hi James, On Wed, 2005-11-30 at 11:23 +0800, James Henstridge wrote: Emmanuele Bassi wrote: Uhm, I'll put that code under #idef G_OS_UNIX/#endif guards for the time being, but the getenv(TZ)/setenv(TZ) timezone trick should work on any sufficiently recent POSIX-like system; users of other operating systems might just drop me an email. We could just define a GDate function for this date transformation, by the way, since every project that has to deal with ISO8601 dates might eventually benefit of this code. There is an ISO8601 date parser in libsoup that might be worth looking at. If you just want an mktime() equivalent that handles UTC, see soup_mktime_utc(). Thank you, I'll have a look at it immediately. Ciao, Emmanuele. -- Emmanuele Bassi - [EMAIL PROTECTED] Log: http://log.emmanuelebassi.net ___ gtk-devel-list mailing list gtk-devel-list@gnome.org http://mail.gnome.org/mailman/listinfo/gtk-devel-list
Re: libegg/recentchooser
Uhm, I'll put that code under #idef G_OS_UNIX/#endif guards for the time being, but the getenv(TZ)/setenv(TZ) timezone trick should work on any sufficiently recent POSIX-like system; users of other operating systems might just drop me an email. setenv isn't POSIX. You can probably just use g_setenv although that will leak. But really, just do it as ((days_since_19700101 * 24 + h) * 60 + m) * 60 + s where days_since_19700101 comes out of GDate. M. ___ gtk-devel-list mailing list gtk-devel-list@gnome.org http://mail.gnome.org/mailman/listinfo/gtk-devel-list
libegg/recentchooser
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. ___ gtk-devel-list mailing list gtk-devel-list@gnome.org http://mail.gnome.org/mailman/listinfo/gtk-devel-list
Re: libegg/recentchooser
That code also needs to be run through Valgrind or Purify. egg_recent_chooser_item_activated_cb, for example, seems to have a double free. timestamp_from_iso8601 manipulates the environment in a way that does not look po[r]table. (And it's not that hard to do using mktime-in-any-timezone, basic arithmetic, or GDate.) M. ___ gtk-devel-list mailing list gtk-devel-list@gnome.org http://mail.gnome.org/mailman/listinfo/gtk-devel-list
Re: libegg/recentchooser
Hi Matthias, On Tue, 2005-11-29 at 12:18 -0500, Matthias Clasen wrote: 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. Thanks. I'll try and do my best to answer. Matthias * eggdesktopbookmarks.[hc]: - Where is the GMarkup parser ? libegg still seems to have libxml It's in libegg/bookmarkfile - I didn't remove the old libxml-based parser for reference checking and because CVS removal always scare the hell out of me. :-) - 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 ?! The name should be BookmarkFile/bookmark_file since it retains the same semantics as the KeyFile/key_file object. Also, the BookmarkFile code should not be limited to recently used resources: the shortcuts for the FileChooser could use it - both internally and as a public API for manipulating the shortcuts. Right now, the only way to access this data is to copy the code from the FileSystem object or to replicate it (see bug #147434 [1]); also, the same meta-data about the application could make the shortcuts application-specific (see bug #157377 [2]). - 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. The BookmarkFile code has this function - it was a later addition. - 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 ? I was trying to mimic the KeyFile API. We have non-mandatory meta-data, though; returning %NULL could either mean that the attribute was unset *or* the item was missing. I prefer to keep the former case possible, while the latter an error. * eggrecentchooser.[hc], eggrecentchooserprivate.h - I wonder why there is both EggRecentChooserSortType and EggRecentSortType, and why they are not the same. Uhm, never quite convinced me either - but in design phase I preferred to keep them separated, as the first affects the RecentManager, while the second affects the RecentChooser implementations. I could merge them, though, and place the SortType into a common enum-only header. - 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 ? The show-tips and show-numbers properties were left-overs from the adaptation of the EggRecent API; show-tips might just go, while it was pointed out (on IRC) that a mnemonic for the menu would be in order, in order to navigate with keyboards. Also, the show-icon could be usefule: while menus have a style property, the RecentChooserWidget should listen to that property, and that would violate a bit the separation between the menu and the widget. Also the documentation for these is a bit weak. I know. Not being a non-english speaker makes writing good documentation more difficult, for me at least. 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. For this, I'll need more testing. I'll see what I can do. Also, it seems that most of these properties are only implemented for some of the EggRecentChooser implementations. Some of them make more sense in some implementations, e.g. the show-tips would be useless on the RecentChooserWidget without the TreeView supporting them. I was planning to add tips to the filters, though, and those would be controlled by the show-tips property. Other properties are partially implemented because I was still considering to have them. As I said, the show-numbers property might go away: we could just number the items ourselves (even though I don't really much like it); the show-tips property is useful only if every widget supports tooltips, which should happen when the RecentChooser code hits GTK+. - 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 You're right - I'll export the RecentChooser interface. * 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 prefer being a little more
Re: libegg/recentchooser
Hi, On Tue, 2005-11-29 at 14:56 -0500, Morten Welinder wrote: That code also needs to be run through Valgrind or Purify. egg_recent_chooser_item_activated_cb, for example, seems to have a double free. I'll check it out ASAP. I'm not that good with Valgrind, so any help there would be much appreciated. :-) timestamp_from_iso8601 manipulates the environment in a way that does not look po[r]table. (And it's not that hard to do using mktime-in-any-timezone, basic arithmetic, or GDate.) Uhm, I'll put that code under #idef G_OS_UNIX/#endif guards for the time being, but the getenv(TZ)/setenv(TZ) timezone trick should work on any sufficiently recent POSIX-like system; users of other operating systems might just drop me an email. We could just define a GDate function for this date transformation, by the way, since every project that has to deal with ISO8601 dates might eventually benefit of this code. Thanks, Emmanuele. -- Emmanuele Bassi - [EMAIL PROTECTED] Log: http://log.emmanuelebassi.net ___ gtk-devel-list mailing list gtk-devel-list@gnome.org http://mail.gnome.org/mailman/listinfo/gtk-devel-list
