[Geany-devel] File type icons?

[Colomban Wendling]

Le 26/10/2010 18:06, Nick Treleaven a écrit :
> On Fri, 22 Oct 2010 02:34:23 +0200
> Colomban Wendling <lists.ban at herbesfolles.org> wrote:
> 
>>> The combined patch(es) will come separately in a few minutes for less
>>> confusion.
>> Here we are:
>>
>>
>> 0001-Add-MIME-type-and-icon-to-GeanyFiletype.patch:
>>  * Combination of 0001-Add-MIME-type-to-GeanyFiletype.patch and
>>    0002-Add-filetype-icon.patch, so it adds both MIME type and
>>    icon to filetypes.
> 
> Thanks, applied. Minor changes below:
> 
>> @@ -480,6 +532,9 @@ static void filetype_add(GeanyFiletype *ft)
>>  	g_hash_table_insert(filetypes_hash, ft->name, ft);
>>  
>>  	filetypes_by_title = g_slist_insert_sorted(filetypes_by_title, ft, cmp_filetype);
>> +
>> +	if (!ft->icon && ft->mime_type)
>> +		ft->icon = ui_get_mime_icon(ft->mime_type, GTK_ICON_SIZE_MENU);
> 
> ft->icon should always be null there so I removed that part.
True, the check is useless.

> 
>> +GdkPixbuf *ui_get_mime_icon(const gchar *mime_type, GtkIconSize size)
>> +{
> ...
>> +	gint real_size;
>> +
>> +	g_return_val_if_fail(gtk_icon_size_lookup(size, &real_size, NULL), NULL);
> 
> You shouldn't put side effects into an assert statement because asserts
> can be disabled. Fixed.
Right, I didn't thought about that. But I' not sure it's a good idea to
do the call twice, I'd think it's probably better to store the return
value of gtk_icon_size_lookup() and check it, don't you think?
We would have a less beautiful error message if the check fails, but in
most cases we avoid to lookup the size twice.


Anyway, thanks for reviewing :)

Regards,
Colomban