GNOME Bugzilla – Bug 657627
new font chooser API inconsistent
Last modified: 2011-09-13 11:05:12 UTC
Consider these preexisting APIs: GtkFileChooser is an interface. - GtkFileChooserWidget is a GtkWidget implementing this interface - GtkFileChooserDialog is a GtkDialog implementing this interface, containing a GtkFileChooserWidget - GtkFileChooserButton is a GtkButton implementing this interface, that pops up a GtkFileChooserDialog when clicked GtkRecentChooser is an interface - GtkRecentChooserWidget is a GtkWidget implementing this interface - GtkRecentChooserDialog is a GtkDialog implementing this interface, containing a GtkRecentChooserWidget GtkAppChooser is an interface. - GtkAppChooserWidget is a GtkWidget implementing this interface - GtkAppChooserDialog is a GtkDialog implementing this interface, containing a GtkAppChooserWidget - GtkAppChooserButton is a GtkButton implementing this interface, that pops up a GtkAppChooserDialog when clicked Now look at the new font chooser APIs: GtkFontChooser is NOT an interface, but the widget itself. GtkFontChooserDialog is a GtkDialog, with a gtk_font_chooser_dialog_get_font_chooser() API. GtkFontButton is a GtkButton, which pops up a GtkFontChooserDialog when clicked, and it doesn't even have a get_font_chooser() type API. This is an obvious inconsistency. Also, it means one is unable to e.g. set a font filter func on a font button (to e.g. only show monospaced fonts). That could be fixed by adding a get_font_chooser() API to the font button, but I'd like to propose to - make GtkFontChooser an interface, like the API examples above - make GtkFontChooserWidget implemtent this interface - make GtkFontChooserDialog and GtkFontButton implement this interface instead of the get_font_chooser() type API they have now. If you agree with this plan, I could prepare patches / branch for this.
(In reply to comment #0) > If you agree with this plan, I could prepare patches / branch for this. tbh, I had considered this to be too much work for something as simple as the font chooser. But if you are willing to do the work, sure
Done and pushed to the 'font-chooser-api' branch. - I'd like to add a "font-desc" property of type PANGO_TYPE_FONT_DESCRIPTION too (similar to how GtkCellRendererText has both "font" and "font-desc" property), since that will make it easier to bind a PangoFontDescription property to a font chooser - should the family, face, and size accessors gain matching read-only object properties? - the accessors are gtk_font_chooser_get_{family,face,size}, should these be amended to get_font_{family,face,size} so as to not conflict with anything, in bindings? Do you know if the new API is in use in any gnome module yet, so that I could make patches for them?
(In reply to comment #2) > Done and pushed to the 'font-chooser-api' branch. > > - I'd like to add a "font-desc" property of type PANGO_TYPE_FONT_DESCRIPTION > too (similar to how GtkCellRendererText has both "font" and "font-desc" > property), since that will make it easier to bind a PangoFontDescription > property to a font chooser agree. > - should the family, face, and size accessors gain matching read-only object > properties? this I'm not entirely sure; essentially, you could connect to the :font-desc notification and get the family, face and size out of the PangoFontDescription object. you'd have to notify those properties along with the :font-desc one, so it might be redundant. > - the accessors are gtk_font_chooser_get_{family,face,size}, should these be > amended to get_font_{family,face,size} so as to not conflict with anything, in > bindings? I prefer get_font_*. get_size() is the most problematic, for instance. > Do you know if the new API is in use in any gnome module yet, so that I could > make patches for them? from a cursory git grep in my jhbuild, nothing seems to be using gtk_font_chooser_* yet - and the freezes are now in effect, so no patches should be required.
I've looked over the branch, and it looks good to me. I want to land this soon, so we can do a final 3.1.x release with this new api before declaring it stable. I've pushed some cosmetic doc fixes which you might want to squash before merging. Adding a font-desc property sounds fine to. Lets hold off on properties for font components. But yes to changing the getters to ..._get_font_...
*** Bug 563001 has been marked as a duplicate of this bug. ***
Alright, I'll work on the remaining issues today.
I have pushed an updated branch. Changes since last include: - add 'font-desc' property - rename get_{family,face,size} to get_font_{family,face,size} - keep the set font even when it's not found There's some bug with the font size in the font button, which I'll look at tomorrow, but API-wise this should be it. Question: If a font filter is set, and the font set from the API doesn't match the filter, should get_font_desc etc return NULL ? IMHO yes.
Actually, now that we have gtk_font_chooser_get_font_desc() / 'font-desc' property, do we need the get_font_{family,face,size} accessors anymore? Do they give us anything that we can't get from the font desc?
There is no good way to go from a font description to a face or family, I think (see bug 496545). Not sure that is really necessary, but that is the reason why I pass the family and face to the filter function, instead of a font description.
Landet on master. If there's anything remaining, let's open new bugs for that; calling this one FIXED.