Merge lp:~3v1n0/sni-qt/icons-user-cache-on-snap into lp:sni-qt

[Marco Trevisan (Treviño)]

IconCache: get the proper theme path based on the fact we're using a themed icon or not

If an app uses QIcon::fromTheme then we only have the icon name of it,
and thus we need to return a valid icon theme path. Now, it's not possible
to return the whole list of dirs, so we just fallback to the home icons
older, so that users might in case add icons there.

In $SNAP world, when using desktop-launcher ~/.local/share/icons also
contains all the themes in from /usr/share/icons and potentially more
(depending on the wrapper used for launching) so, we can be quite flexible
with it.

Also when running in a snap, save icons in $XDG_CACHE_HOME or in the user
cache, but still in an indicator readable location

You can use https://github.com/3v1n0/indicators-examples-snaps/tree/master/qt4-systray to test this.

[Lukáš Tinkl (lukas-kde)]

You don't have to use QDir::separator(), it's even discouraged (http://doc.qt.io/qt-5/qdir.html#separator)

review: Needs Fixing

[Lukáš Tinkl (lukas-kde)]

Inline comment about the way you find the .cache directories

review: Needs Fixing

[lp:~3v1n0/sni-qt/icons-user-cache-on-snap]

107. By Marco Trevisan (Treviño) on 2016-11-16

Don't use deprecated QDir::separator()

108. By Marco Trevisan (Treviño) on 2016-11-16

fsutils: add some comments about how we get the .cache

[Marco Trevisan (Treviño) (3v1n0)]

Lukáš Tinkl (lukas-kde):
> Trevinho, I still have a slight doubt about ~/.cache; what if it doesn't exist? what creates it?

The subsequent call to dir.mkpath(".").

[lp:~3v1n0/sni-qt/icons-user-cache-on-snap]

109. By Marco Trevisan (Treviño) on 2016-11-16

IconCache: try to get $XDG_DATA_HOME as base icon folders

[Lukáš Tinkl (lukas-kde)]

LGTM, working fine

review: Approve

=== modified file 'src/fsutils.cpp'
--- src/fsutils.cpp	2011-10-14 21:41:08 +0000
+++ src/fsutils.cpp	2016-11-16 23:16:14 +0000
@@ -32,16 +32,28 @@
 
 QString generateTempDir(const QString& prefix)
 {
-    QDir dir = QDir::temp();
+    QDir dir;
+
+    if (!getenv("SNAP")) {
+        dir = QDir::temp();
+    } else {
+        // Try to get the .cache from $XDG_CACHE_HOME, if it's not set,
+        // it has to be in ~/.cache as per XDG standard
+        QString path = QString::fromUtf8(getenv("XDG_CACHE_HOME"));
+        if (path.isEmpty()) {
+            path = QDir::cleanPath(QDir::homePath() + "/.cache");
+        }
+
+        dir.setPath(path);
+    }
+
     if (!dir.mkpath(".")) {
         qCritical("Failed to generate temporary file for prefix %s: could not create %s",
             qPrintable(prefix), qPrintable(dir.path()));
         return QString();
     }
 
-    QString tmpl = QString("%1/%2-XXXXXX")
-        .arg(dir.path())
-        .arg(prefix);
+    QString tmpl = QDir::cleanPath(dir.path() + '/' + prefix + "-XXXXXX");
     QByteArray ba = QFile::encodeName(tmpl);
     const char* name = mkdtemp(ba.data());
     if (!name) {
 
=== modified file 'src/iconcache.cpp'
--- src/iconcache.cpp	2011-10-10 16:37:00 +0000
+++ src/iconcache.cpp	2016-11-16 23:16:14 +0000
@@ -26,7 +26,6 @@
 #include <QDateTime>
 #include <QDebug>
 #include <QDir>
-#include <QIcon>
 #include <QList>
 
 const int IconCache::MaxIconCount = 20;
@@ -86,8 +85,18 @@
     }
 }
 
-QString IconCache::themePath() const
+QString IconCache::themePath(const QIcon& icon) const
 {
+    if (!icon.isNull() && !icon.name().isEmpty() && QIcon::hasThemeIcon(icon.name())) {
+        QString dataHome = QString::fromUtf8(getenv("XDG_DATA_HOME"));
+
+        if (dataHome.isEmpty()) {
+            dataHome = QDir::homePath() + "/.local/share";
+        }
+
+        return QDir::cleanPath(dataHome + "/icons");
+    }
+
     return m_themePath;
 }
 
 
=== modified file 'src/iconcache.h'
--- src/iconcache.h	2011-10-10 16:37:00 +0000
+++ src/iconcache.h	2016-11-16 23:16:14 +0000
@@ -19,6 +19,7 @@
 
 // Qt
 #include <QObject>
+#include <QIcon>
 #include <QStringList>
 
 class QIcon;
@@ -35,8 +36,7 @@
 
     static const int MaxIconCount;
 
-    QString themePath() const;
-
+    QString themePath(const QIcon& icon = QIcon()) const;
     QString nameForIcon(const QIcon& icon) const;
 
     // Internal, testing only
 
=== modified file 'src/statusnotifieritem.cpp'
--- src/statusnotifieritem.cpp	2016-11-16 23:16:14 +0000
+++ src/statusnotifieritem.cpp	2016-11-16 23:16:14 +0000
@@ -204,7 +204,7 @@
 
 QString StatusNotifierItem::iconThemePath() const
 {
-    return m_iconCache->themePath();
+    return m_iconCache->themePath(trayIcon->icon());
 }
 
 QString StatusNotifierItem::iconName() const
 
=== modified file 'tests/auto/iconcachetest.cpp'
--- tests/auto/iconcachetest.cpp	2011-10-10 16:37:00 +0000
+++ tests/auto/iconcachetest.cpp	2016-11-16 23:16:14 +0000
@@ -73,6 +73,32 @@
         QVERIFY(info.isDir());
     }
 
+    void testThemePathForIcon()
+    {
+        QList<int> sizes = QList<int>() << 16 << 22 << 32;
+        QIcon icon = createTestIcon(sizes, Qt::red);
+
+        IconCache cache(m_sandBoxDirName);
+        QString themePath = cache.themePath(icon);
+        QCOMPARE(themePath, m_sandBoxDirName + "/icons");
+
+        QFileInfo info(themePath);
+        QVERIFY(info.isDir());
+    }
+
+    void testThemePathForThemedIcons()
+    {
+        IconCache cache(m_sandBoxDirName);
+        QIcon icon = QIcon::fromTheme("indicator-messages");
+        QString themePath = cache.themePath(icon);
+
+        if (icon.name().isEmpty()) {
+            QSKIP("Icon has not been found in theme, so there's nothing to test here", SkipSingle);
+        }
+
+        QCOMPARE(themePath, QDir::homePath() + "/.local/share/icons");
+    }
+
     void testPixmapIcon()
     {
         IconCache cache(m_sandBoxDirName);
@@ -82,7 +108,7 @@
 
         QString name = cache.nameForIcon(icon);
         Q_FOREACH(int size, sizes) {
-            QString dirName = cache.themePath() + QString("/hicolor/%1x%1/apps").arg(size);
+            QString dirName = cache.themePath(icon) + QString("/hicolor/%1x%1/apps").arg(size);
             QVERIFY(QFile::exists(dirName));
             QImage image;
             QVERIFY(image.load(dirName + "/" + name + ".png"));