Opened 5 years ago

Last modified 5 years ago

#9401 new Patch

Get file icons on linux

Reported by: Francois Ferrand Owned by:
Priority: normal Component: FileZilla Client
Keywords: Cc:
Component version: Operating system type: Linux
Operating system version:

Description

wxFileType::GetIcon() is not implemented in wxGtk.
This patch replaces calls to wxTheMimeTypesManager and wxFileType::GetIcon() with calls to gnome.

Other platforms are not affected.

In addition, wxStat is used to properly detect 'local' wxLstat.

Attachments (3)

0001-Get-file-icons-on-linux.patch (8.5 KB) - added by Francois Ferrand 5 years ago.
0001-Get-file-icons-on-linux.2.patch (9.0 KB) - added by Francois Ferrand 5 years ago.
patch v2
0001-Get-file-icons-on-linux.3.patch (9.2 KB) - added by Francois Ferrand 5 years ago.

Download all attachments as: .zip

Change History (6)

Changed 5 years ago by Francois Ferrand

comment:1 Changed 5 years ago by Tim Kosse

Status: newmoreinfo

Thank you for your patch.

Could you please address the following issues in a revised patch?

1) Including GTK/GIO-specific headers without checking for their existence first in the configure script.

2) No runtime fallback on the old behavior if there is no Gnome

3) You're bypassing the cache which will have a measureable runtime impact. Please tell me: How long does it take to list a directory containing a million files both with and without the patch applied?

4) wxFileName is very slow. Consider using FileZilla's CLocalFileSystem instead.

5) You are using various magic constants, e.g. the various GNOME_ICON_LOOKUP_FLAGS. Why not use the Gnome headers for this which surely must contain this information?

Changed 5 years ago by Francois Ferrand

patch v2

comment:2 Changed 5 years ago by Francois Ferrand

Status: moreinfonew

I attached a new patch with the changes, as documented bellow.
Not all issues are fixed, see my answer below and please provide some feedback.

1) Including GTK/GIO-specific headers without checking for their existence first in
the configure script.

gio.h is part of GLib, and gtk.h is part of Gtk: both are already needed to compile with wxGTK (i.e. GTK on linux). Do we need extra checks? If so, would you have any pointers on on how to do this (I have very limited knowledge of autotools...)?

2) No runtime fallback on the old behavior if there is no Gnome

Done.

3) You're bypassing the cache which will have a measureable runtime impact. Please
tell me: How long does it take to list a directory containing a million files both
with and without the patch applied?

I think I am still going through the cache: lookup is unaffected (before the call to GetFileIcon()), and insertion is still done at the end of the CSystemImageList::GetIconIndex() function. Or am I missing something here?

4) wxFileName is very slow. Consider using FileZilla's CLocalFileSystem instead.

Done.

5) You are using various magic constants, e.g. the various GNOME_ICON_LOOKUP_FLAGS.
Why not use the Gnome headers for this which surely must contain this information?

The Gnome headers will certainly contain the information. However, this would mean the Gnome libs are needed for compilation, which is not the case at the moment. Would it be better to change this?

Changed 5 years ago by Francois Ferrand

comment:3 Changed 5 years ago by Francois Ferrand

patch v3, just fix a crash in previous patch, which now happens with wxWidgets 3.0.

Note: See TracTickets for help on using tickets.