GNOME Bugzilla – Bug 349694
port to GtkRecent
Last modified: 2006-08-07 17:22:10 UTC
We need to switch to GtkRecent and kill EggRecent for good. This has become partcularly urgent given that gnome-panel has switched. Attached is a first cut of a port. Things that still need doing: 1 - filtering of the recent lists (I have no idea how to do this... by mime? by group?) 2 - handle screen-safety: I am pretty confused about this I have to admit... EggRecentModel used to be a singleton while GtkRecentManager is per screen... the examples all simply use recent_manager_get_recent() but I fear that this doesn't cut for gedit since we can have windows on different screens and a screen can be closed while gedit is running (and the recent_manager goes away with the screen). We should do something similar to what we do with the icon-theme in gedit spinner. However I still don't understand what happens if I add a recent on a screen and remove it in another... are the different recent managers kept in sync? Related to this is the fact the I made recent_add/remove take a GeditWindow with the idea of storing the recent manager pointer in the window itself... though I have not made up my mind also because GeditTab has to jump through hoops to get the window... 3 - the gconf notification of the max recents still needs porting 4 - maybe other bugs or thing I now forgot 5 - tooltips on the toolbar menu recents are not properly ellipsized like they were before: this needs fixing in gtk. Anyway all in all the code is not that bad even if the inlined menu support is a bit of a hack it allows us to remove lots of hacks we had in place before and obviously also to kill the infamous eggrecent.
another thing I forgot: we should prolly use item_add_full to specify the mime type and maybe other stuff... suggestion about it would be welcome.
Created attachment 70083 [details] [review] patch
1. filtering can be done on mime type (I imagine text/*), group (TextEditor) and application (Gedit); 2. the safest course of action is to use a screen singleton for each window; since every GtkRecentManager reads and writes the same file synchronisation should not be an issue in case you have to switch to another screen. 5. I'll fix those ASAP; GTK 2.10.2 should be released in time for GNOME 2.16 so there are good chances that every fix that does not break API/ABI can go in this time frame. as for the add_full(): you should probably use that, since Gedit already uses gnome-vfs and supports non-local files.
Thanks for the clarification Emmanuele, > 1. filtering can be done on mime type (I imagine text/*), group (TextEditor) > and application (Gedit); I am not thrilled by the idea of using mime types... unfortunately there are plenty text files that do not have text/* like application/python etc and special casing them is a PITA. Someone recalls how did we filter with EggRecent? Emmanuele, are there examples of filtering and of add_full?
Created attachment 70120 [details] [review] patch Ok, here is a patch I consider ready for review/applying. - it has filtering/clamping: filtering is based on a group "gedit", like the old EggRecent code did... we can prolly switch to filter on the application name or even better to the mime type if it really supports mime type inheritance like the docs suggest. Btw, can we have two filters OR-ed (group gedit || mime text)? Anyway, this is a further enanchement which can be done after the initial port - it properly uses add_full specifying the mime type, group, etc. - it's screen safe (at least in theory). Needs to be tested - restores GConf notification, though I don't see a way to update the manually created inline menu... this is also untested beacuse gconf notification doesn't work here even on 2.14, may be my system though A thing still TODO is update the sensitivity of recents according to the state, but this can be done in a second step: it was marked TODO even with the old code since Egg didn't allow us to do that, now we can and should do it.
Created attachment 70260 [details] [review] patch updated patch which also takes care of sensitivity. Call for testing: the patch crashed on me once while opening a file, but I have not been able to reproduce and get a decent stack trace... can you guys test this a bit?
committed. We'll find out about the crash by unleashing this to the masses :)