GNOME Bugzilla – Bug 307796
Ellipsis of the recent menu items
Last modified: 2005-08-21 19:09:22 UTC
I'll be pasting in a screenshot to show what's happening with some of my PDFs, but basically we need a decent default size for the menu items. Maybe making sure the menu items can't go beyond where the zoom level combobox is, try it out and let me know.
Created attachment 47818 [details] Screenshot of large menu items
Created attachment 47900 [details] [review] This should do it I need to get this in libegg though... Feel free to play with the max char define to see if it's appropriate...
a better approach would be creating an API for this inside EggRecentViewGtk, allowing a custom value for the label width, instead of an hardcoded one. marco, you should reassign this bug to the recent-files component of libegg.
Created attachment 47946 [details] [review] EggRecentViewGtk ellipsize API This adds an API for the max width of the label, in order to enable the ellipsization of the recent item name
Created attachment 47947 [details] [review] EggRecentViewGtk ellipsize API this time, the correct patch, with the set/get_property code
I am not sure you'd like to present it in api, moreover, it's better to calculate visible width in pixels instead of using chars.
I am not that sure about the api either. At very least I think we should have a default value different than -1. That way we get a consistent behavior for all applications and it's one thing less application writers has to worry about. Maybe some applications has different needs, but I dont quite see the use case for this, that's why I didnt introduce an api in first place. Emmanuele are you ok with changing the default value to 40 or something? About chars vs width I disagree. I think what's important here is to show to user a significative number of chars, independently of the font width. What would you pick as width? Also fwiw I think this api was introduced in gtk with this use case in mind.
marco: I used the default '-1' value for two reasons: mainatin consistency with GtkLabel API, and be backwards compatible with current code with regards to UI. not that I'm advocating against choosing a sane default (40 chars are a bit too much, IMHO; less would be good - something in the order of 20/30 chars would be enough) but we shouldn't break the UI of the applications using EggRecent code.
>But we shouldn't break the UI of the applications using EggRecent code Backwards compatibility doesnt seem like a strong point. How would the UI break by introducing such default? It would just be different but improved, as far as I can see. Furthermore this is cut/paste code not a library, if someone really wants (but I fail to see why) the old behavior they can just avoid to update or patch the new code. About consistency with the GtkLabel I dont see the point. This is a particular case (unknown width menu items) and tweaking the behavior at a lower level, so that it can be shared by all users without further work, make perfectly sense.
Uhm, there is bug 124330 which is a dup of this one. I've had a short talk with pbor on IRC about this, and he pointed me to bug #124330; we should use 25/30 as a sensible default; but we should also clamp on the shortest item's name. the API is necessary since we mask the menu and its children inside EggRecentViewGtk (that is why we should really get rid of it, and use a widget directly inheriting from GtkMenu, instead); we need to make the maximum width customizable.
>but we should also clamp on the shortest item's name I think gtk does this. The number of chars is a maximum, if strings are shorter it doesnt waste unnecessary space. If I got what you mean at least. What about if I go ahead and commit your patch with a 30 chars max. Epiphany use 32 too, so I guess it make sense. We can tweak it later at worst.
actually I didn't mean clamp on the shortest item name... what I meant on irc (and I have no idea if it is possible at all) is that if there is a menu item long 50 chars *outside* the recent list (in other words if the menu is already larger than 30 chars), then the recent list should ellipsiza at 50 not at 30.
Paolo, I'd expect that to just work in gtk. That function just set size requisition, so, if there is space, the label should expand. Emmanuele, ok to commit with a default of 30?
paolo: sorry, I misunderstood. marco: fine by me.
The same should be used for EggRecentViewUIManager
*** Bug 124330 has been marked as a duplicate of this bug. ***
Created attachment 48008 [details] [review] simple patch Add ellipsisation to the UI manager view.
Created attachment 48017 [details] [review] updated patch Add ellipsisation to ui manager view, and remove unneeded crap (icon size, leading/trailing separator).
Christian, your patch should be broken down. I see some code cleanups, the removal of definitions of functions - without removing their declarations from the header file, ouch! - and some code changes that I personally do not understand; for instance, why did you change this: - gchar *name; - name = g_strdup_printf (EGG_RECENT_NAME_PREFIX"%u-%u", - view->merge_id, - index); With this: + gchar name[80]; + g_snprintf (name, sizeof (name), EGG_RECENT_NAME_PREFIX "-%p-%u", + view, index); It's pointless using a stack variable of 80 chars here instead of an allocated one, and using the memory location of the viewer instance instead of the merge id does not enhance the uniqueness of the prefix. You should break down your patch into three: the ellipsization of the labels inside the UIManager; the code cleanups, which should go into bug 168578; and the API removal, which should go into another bug report - since the RecentViewUIManager API maps the RecentViewGtk one, thus if you remove some calls from one you should do the same for the other.
> Christian, your patch should be broken down. I was just too lazy to create indedependent patches. I'll do that now. > I see some code cleanups, the removal of definitions of functions - without > removing their declarations from the header file, ouch! Sorry, forgot to remove the icon_size decls from the header. > - and some code changes > that I personally do not understand; for instance, why did you change this: > > - gchar *name; > > - name = g_strdup_printf (EGG_RECENT_NAME_PREFIX"%u-%u", > - view->merge_id, > - index); > > With this: > > + gchar name[80]; > > + g_snprintf (name, sizeof (name), EGG_RECENT_NAME_PREFIX "-%p-%u", > + view, index); > > It's pointless using a stack variable of 80 chars here instead of an allocated > one, and using the memory location of the viewer instance instead of the merge > id does not enhance the uniqueness of the prefix. It's not pointless to use a stack variable: it saves mallocs. And the reason for the change from %u-%u (mergeID-index) to %p-%u (ptr-index) is that the merge ID will change every time the menu is rebuilt. GtkUIManager stores the action names in quarks, which will never be deallocated until program exit. So, to save memory, it's better to use names which will change less often. I should have explained that originally. > You should break down your patch into three: the ellipsization of the labels > inside the UIManager; the code cleanups, which should go into bug 168578; and > the API removal, which should go into another bug report - since the > RecentViewUIManager API maps the RecentViewGtk one, thus if you remove some > calls from one you should do the same for the other. The removed parts may make sense for the GtkMenu based recent view, but they definitely don't make sense for the UI manager based one (icon size cannot be changes, and pre/post-separator can be added in UI manager itself).
Created attachment 48023 [details] [review] only ellipsisation changes Just the ellipsisation changes. I'm going to drop the other parts of the above patch when this one is committed (that makes diff'ing easier).
christian, I've committed your latest patch. thanks.
Reopening since the similar fixes should be applied to the other views too. For example we need similar fixes for egg-recent-view-bonobo and egg-recent-view-gtk to fix the gedit bug #137539. Probably the panel has the same problem too. For example try to open a very long URI in gedit. For example: http://landfill.mozilla.org/bugzilla-tip/buglist.cgi?query_format=&short_desc_type=allwordssubstr&short_desc=&long_desc_type=allwordssubstr&long_desc=&bug_file_loc_type=allwordssubstr&bug_file_loc=&status_whiteboard_type=allwordssubstr&status_whiteboard=&keywords_type=allwords&keywords=&bug_status=REOPENED&emailassigned_to1=1&emailtype1=substring&email1=&emailassigned_to2=1&emailreporter2=1&emailcc2=1&emailtype2=substring&email2=&bugidtype=include&bug_id=&votes=&chfieldfrom=&chfieldto=Now&chfieldvalue=&field0-0-0=noop&type0-0-0=noop&value0-0-0=&ctype=ics and look how bad the the File menu and the recent file list look. Then look at the "Recent Documents" menu in the panel.
IMHO, a possible solution could be modifying the egg_recent_item_get_uri_for_display so that it returns an ellipsized string. In this way all the views will be automatically fixed (if, as I hope, they use this function).
Also egg_recent_item_get_short_name should me modified. Actually I think egg_recent_item_get_short_name should be also rewritten to return a short name similar to the one nautilus and gedit are using. Ok, I think I will provide a patch.
> IMHO, a possible solution could be modifying the > egg_recent_item_get_uri_for_display so that it returns an ellipsized string. > In this way all the views will be automatically fixed (if, as I hope, they use > this function). No that would be totally wrong. In order to make different strings ellipsised to the same length, you should use GtkLabel's ellipsisation; if you manually clip them they will have different lengths.
Also the egg-recent-view-gtk appears to already be fixed? At least evince had a fix in its cut-and-paste lib, http://cvs.gnome.org/viewcvs/evince/cut-n-paste/recent-files/egg-recent-view-gtk.c?hideattic=0&r1=1.3&r2=1.4 and it shouldn't be too hard to adapt that to the bonobo view.
I agree with you... using GtkLabel's ellipsisation would be the best solution but you cannot use it for the bonobo view. The evince patch has not been applied to libegg (at least I have not seen it). How are you going to solve the same problem for the tooltips using GtkLabel's ellipsisation? And for the status bar? I have attached a patch to bug #137539 (see attachment #49945 [details]) that seems to solve this problem using the approach I have proposed in my previous comments. I know it is not the best solution (when you have two or more truncated items in the same menu, I think it is very infrequent case), but it works for all the cases.
Hmm okay, then string-snipping should be used only for the bonobo view. GtkStatusbar ellipsises its content so I think there's no problem. Tooltips on the other hand don't do that, so string-snipping there too :( I was just concerned that you were going to snip strings where it matters most, in the menu items :) I could be mistaken, but your/eel's str_middle_truncate doesn't look like it's UTF-8 safe?
You are right, eel's str_middle_truncate is not UTF-8 safe. Need to fix it. I will modify the patch so to use the string ellipsisation only for the bonobo view and the tooltips.
the ellipsization code has been applied for both egg-recent-view-gtk and egg-recent-view-uimanager; paolo, if you can provide a patch for the bonobo implementation, it would be really great: I don't know bonobo enough, so I didn't touch it. we should really get the pango ellipsization code to be used inside tooltips too: I think it would be useful there.
I will provide code for the bonobo implementation (note that we will not be able to use the pango ellipsisation in that case). I will ellipsize the end of the file name since it seems you are using PANGO_ELLIPSIZE_END in the patches for egg-recent-view-uimanager and egg-recent-view-gtk. Why didn't you use PANGO_ELLIPSIZE_MIDDLE? It is not clear to me how you want to solve this problem with tooltips.
paolo, you should see comment #18 of bug #137539: we could provide a: gchar *egg_recent_item_get_uri_ellipsized (EggRecentItem *item, PangoEllipsizeMode mode); that could be used also internally for tooltips. or we could ask for ellipsization support of the GtkTooltips inside Gtk.
I have just added a new patch to bug #137539 (attachment #50117 [details]). It implements ellipsization for the bonobo view in the same way it is implemented for the other views. It does not solve the problem of tooltips (or better the solution is gedit specific). Please, review the patch and lemme know if I can commit it to libegg. Since it works great for me, I will commit it in gedit in any case.
Please, note that my patch also removes old/unused code from egg-recent-item.c and re-implements egg_recent_item_get_short_name. The new egg_recent_item_get_short_name uses the same code used by nautilus and gedit for displaying short names and works better than the current one for non-utf8 file names. At the same time it increases consistency among gnome apps.
the patch looks good, please commit it (I won't be back until saturday). I'll reuse your code in the RecentInfo object, so I'll keep this bug open as a reminder for when I get back.
Created attachment 50176 [details] [review] Committed patch This is the patch I have just committed. I have tested it and also with my broken .recently-used file (see bug #150306) it seems to work without problems.