sni-qt

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

  1. icons-user-cache-on-snap
  2. Merge into trunk.13.04
Proposed by Marco Trevisan (Treviño)
Status: Merged
Approved by: Lukáš Tinkl
Approved revision: 109
Merged at revision: 103
Proposed branch: lp:~3v1n0/sni-qt/icons-user-cache-on-snap
Merge into: lp:sni-qt
Prerequisite: lp:~3v1n0/sni-qt/x11-usertime-on-activate
Diff against target: 149 lines (+57/-10)
5 files modified
src/fsutils.cpp (+16/-4)
src/iconcache.cpp (+11/-2)
src/iconcache.h (+2/-2)
src/statusnotifieritem.cpp (+1/-1)
tests/auto/iconcachetest.cpp (+27/-1)
To merge this branch: bzr merge lp:~3v1n0/sni-qt/icons-user-cache-on-snap
Reviewer Review Type Date Requested Status
Lukáš Tinkl (community) Approve
Review via email: mp+311033@code.launchpad.net

Commit message

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

Description of the change

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

To post a comment you must log in.
Revision history for this message
Lukáš Tinkl (lukas-kde) wrote :

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

review: Needs Fixing
Revision history for this message
Lukáš Tinkl (lukas-kde) wrote :

Inline comment about the way you find the .cache directories

review: Needs Fixing
lp:~3v1n0/sni-qt/icons-user-cache-on-snap updated
107. By Marco Trevisan (Treviño)

Don't use deprecated QDir::separator()

108. By Marco Trevisan (Treviño)

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

Revision history for this message
Marco Trevisan (Treviño) (3v1n0) wrote :

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 updated
109. By Marco Trevisan (Treviño)

IconCache: try to get $XDG_DATA_HOME as base icon folders

Revision history for this message
Lukáš Tinkl (lukas-kde) wrote :

LGTM, working fine

review: Approve

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
1=== modified file 'src/fsutils.cpp'
2--- src/fsutils.cpp 2011-10-14 21:41:08 +0000
3+++ src/fsutils.cpp 2016-11-16 23:16:14 +0000
4@@ -32,16 +32,28 @@
5
6 QString generateTempDir(const QString& prefix)
7 {
8- QDir dir = QDir::temp();
9+ QDir dir;
10+
11+ if (!getenv("SNAP")) {
12+ dir = QDir::temp();
13+ } else {
14+ // Try to get the .cache from $XDG_CACHE_HOME, if it's not set,
15+ // it has to be in ~/.cache as per XDG standard
16+ QString path = QString::fromUtf8(getenv("XDG_CACHE_HOME"));
17+ if (path.isEmpty()) {
18+ path = QDir::cleanPath(QDir::homePath() + "/.cache");
19+ }
20+
21+ dir.setPath(path);
22+ }
23+
24 if (!dir.mkpath(".")) {
25 qCritical("Failed to generate temporary file for prefix %s: could not create %s",
26 qPrintable(prefix), qPrintable(dir.path()));
27 return QString();
28 }
29
30- QString tmpl = QString("%1/%2-XXXXXX")
31- .arg(dir.path())
32- .arg(prefix);
33+ QString tmpl = QDir::cleanPath(dir.path() + '/' + prefix + "-XXXXXX");
34 QByteArray ba = QFile::encodeName(tmpl);
35 const char* name = mkdtemp(ba.data());
36 if (!name) {
37
38=== modified file 'src/iconcache.cpp'
39--- src/iconcache.cpp 2011-10-10 16:37:00 +0000
40+++ src/iconcache.cpp 2016-11-16 23:16:14 +0000
41@@ -26,7 +26,6 @@
42 #include <QDateTime>
43 #include <QDebug>
44 #include <QDir>
45-#include <QIcon>
46 #include <QList>
47
48 const int IconCache::MaxIconCount = 20;
49@@ -86,8 +85,18 @@
50 }
51 }
52
53-QString IconCache::themePath() const
54+QString IconCache::themePath(const QIcon& icon) const
55 {
56+ if (!icon.isNull() && !icon.name().isEmpty() && QIcon::hasThemeIcon(icon.name())) {
57+ QString dataHome = QString::fromUtf8(getenv("XDG_DATA_HOME"));
58+
59+ if (dataHome.isEmpty()) {
60+ dataHome = QDir::homePath() + "/.local/share";
61+ }
62+
63+ return QDir::cleanPath(dataHome + "/icons");
64+ }
65+
66 return m_themePath;
67 }
68
69
70=== modified file 'src/iconcache.h'
71--- src/iconcache.h 2011-10-10 16:37:00 +0000
72+++ src/iconcache.h 2016-11-16 23:16:14 +0000
73@@ -19,6 +19,7 @@
74
75 // Qt
76 #include <QObject>
77+#include <QIcon>
78 #include <QStringList>
79
80 class QIcon;
81@@ -35,8 +36,7 @@
82
83 static const int MaxIconCount;
84
85- QString themePath() const;
86-
87+ QString themePath(const QIcon& icon = QIcon()) const;
88 QString nameForIcon(const QIcon& icon) const;
89
90 // Internal, testing only
91
92=== modified file 'src/statusnotifieritem.cpp'
93--- src/statusnotifieritem.cpp 2016-11-16 23:16:14 +0000
94+++ src/statusnotifieritem.cpp 2016-11-16 23:16:14 +0000
95@@ -204,7 +204,7 @@
96
97 QString StatusNotifierItem::iconThemePath() const
98 {
99- return m_iconCache->themePath();
100+ return m_iconCache->themePath(trayIcon->icon());
101 }
102
103 QString StatusNotifierItem::iconName() const
104
105=== modified file 'tests/auto/iconcachetest.cpp'
106--- tests/auto/iconcachetest.cpp 2011-10-10 16:37:00 +0000
107+++ tests/auto/iconcachetest.cpp 2016-11-16 23:16:14 +0000
108@@ -73,6 +73,32 @@
109 QVERIFY(info.isDir());
110 }
111
112+ void testThemePathForIcon()
113+ {
114+ QList<int> sizes = QList<int>() << 16 << 22 << 32;
115+ QIcon icon = createTestIcon(sizes, Qt::red);
116+
117+ IconCache cache(m_sandBoxDirName);
118+ QString themePath = cache.themePath(icon);
119+ QCOMPARE(themePath, m_sandBoxDirName + "/icons");
120+
121+ QFileInfo info(themePath);
122+ QVERIFY(info.isDir());
123+ }
124+
125+ void testThemePathForThemedIcons()
126+ {
127+ IconCache cache(m_sandBoxDirName);
128+ QIcon icon = QIcon::fromTheme("indicator-messages");
129+ QString themePath = cache.themePath(icon);
130+
131+ if (icon.name().isEmpty()) {
132+ QSKIP("Icon has not been found in theme, so there's nothing to test here", SkipSingle);
133+ }
134+
135+ QCOMPARE(themePath, QDir::homePath() + "/.local/share/icons");
136+ }
137+
138 void testPixmapIcon()
139 {
140 IconCache cache(m_sandBoxDirName);
141@@ -82,7 +108,7 @@
142
143 QString name = cache.nameForIcon(icon);
144 Q_FOREACH(int size, sizes) {
145- QString dirName = cache.themePath() + QString("/hicolor/%1x%1/apps").arg(size);
146+ QString dirName = cache.themePath(icon) + QString("/hicolor/%1x%1/apps").arg(size);
147 QVERIFY(QFile::exists(dirName));
148 QImage image;
149 QVERIFY(image.load(dirName + "/" + name + ".png"));

Subscribers

People subscribed via source and target branches