Re: gtkstock fixes and other comments



On 10/26/00 Havoc Pennington wrote:
> Paolo Molaro <lupus lettere unipd it> writes:
> > I think there is a bug in the gtkstock implementation:
> > the stock label is translated at each lookup of the stock
> > item (unnecessary, IMHO, and could well get called on the
> > already translated string)
> 
> It couldn't, the GtkStockItem is defined to contain the untranslated
> string. Though I don't think there's any problem with the optimization

Uhm, you're right here, the GtkStockItem in the hash table will
have the untranslated string... and startup overhead should
be considered as well as Owen points out.

> > I'd go for the second option. And while we are changing the 
> > GtkStockItem structure, why not add a GtkIconSet* as well?
> 
> GtkStockItem is supposed to be usable in a static table, it's only by
> coincidence an internal data structure. So if something is only of
> interest internally (e.g. the static flag) it should be internal, not
> something the user has to type in.

The default value for the flag 'need_free'  could be 0 and gtk_stock_add
will enforce it to TRUE.

> > What is the reason for having different GtkIconFactory objects,
> > anyway? I see no reason for an application to have different
> > stock items in different windows, for example (and if you
> > need that you'd need also different stock labels).
> > So my plan would be to remove the GtkItemFactory stuff and
> > move the functionality for stock icons in gtkstock.c.
> > Comments?
> > 
> 
> The icon factory is separate so that you can theme your icons.  Like
> all themeable widget properties, icons can be changed per-widget.

That's fine, but why have a class for it when it could be a simple hash
table in the style? And if you want to change the icon from a style,
why not the label and the keyval as well? Right now stock keyboard
accelerators can't be changed, it seems, and it would be nice to
be able to set them in the style.
I think these are all arguments to have a single structure for
the iconset, the label and the key accelerator (while a key
accelerator doesn't make sense for a warning icon in a dialog,
a label is required and in fact is included in gtkstock.c).
This will reduce startup time as well as memory requirements
because you don't need two hash tables anymore for iconsets
and labels that have the same stock id.

> A screen reader could just look up an image's stock ID in the stock
> system to get the stock info, there's no need to store it in the same
> place for that to be possible.

Yep, but if the stock id is the same, why not store all the info
associated with that stock id in the same hash table?

Thanks,
	lupus

-- 
-----------------------------------------------------------------
lupus debian org                                     debian/rules




[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]