Merge lp:~didrocks/unity-2d/subdatadir into lp:unity-2d

Proposed by Didier Roche on 2011-08-04
Status: Merged
Approved by: Aurélien Gâteau on 2011-08-09
Approved revision: 639
Merged at revision: 639
Proposed branch: lp:~didrocks/unity-2d/subdatadir
Merge into: lp:unity-2d
Diff against target: 61 lines (+22/-3)
2 files modified
libunity-2d-private/src/launcherapplicationslist.cpp (+20/-2)
libunity-2d-private/src/launcherapplicationslist.h (+2/-1)
To merge this branch: bzr merge lp:~didrocks/unity-2d/subdatadir
Reviewer Review Type Date Requested Status
Aurélien Gâteau (community) 2011-08-04 Approve on 2011-08-09
Review via email: mp+70463@code.launchpad.net

This proposal supersedes a proposal from 2011-08-04.

Description of the change

[launcher] Enable storing subdir (like kde4/) in the gsettings schema.
For instance /usr/share/applications/kde4/konversation.desktop should be stored
as kde4-konversation.desktop.

This is valid for all .desktop file in XDG_DATA_DIR/applications.
Note that full path and relative path are still supported

To post a comment you must log in.
lp:~didrocks/unity-2d/subdatadir updated on 2011-08-04
637. By Didier Roche on 2011-08-04

fix typo

Aurélien Gâteau (agateau) wrote :

Works fine, thanks!

I have a few remarks regarding implementation, though:

# Coding style
This file is old and does not comply with unity-2d coding style, but we try to ensure new code does comply. Changes to do are:
- Use camelCase for variables
- Use curly braces for single line ifs
- Attach opening curly brace with the previous line (ie: "if (foo) {", not "if (foo)\n{")
- Whenever possible, use const refs for looping variables in Q_FOREACH

See the CODING file at the root of the repository

# Implementation
- What you are storing in m_xdg_data_dirs is no longer the list of data dirs, it is the list of application dirs. I would suggest renaming the variable to m_xdgApplicationDirs and implement the parsing this way:

Q_FOREACH(const QString& dirName, xdgString.split(':')) {
    m_xdgApplicationDirs << QDir::cleanPath(dirName + "/applications/");
}

- favoriteFromDesktopFilePath() is no longer a static method but it does not modify the object, so it should be marked "const".

review: Needs Fixing
Didier Roche (didrocks) wrote :

Thanks Aurélien.

The new version (once the network will allow me to push it) should address all the concerns there. Please note that I slightly changed your example code (as cleanPath remove the final "/").

lp:~didrocks/unity-2d/subdatadir updated on 2011-08-09
638. By Didier Roche on 2011-08-08

make it more elegant, follow unity-2d guidelines and send a reference for favoriteFromDesktopFilePath (LP: #741129)

639. By Didier Roche on 2011-08-09

fixing coding guidelines

Aurélien Gâteau (agateau) wrote :

Looks good, approved.

review: Approve

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
=== modified file 'libunity-2d-private/src/launcherapplicationslist.cpp'
--- libunity-2d-private/src/launcherapplicationslist.cpp 2011-07-29 13:49:34 +0000
+++ libunity-2d-private/src/launcherapplicationslist.cpp 2011-08-09 09:23:30 +0000
@@ -101,6 +101,16 @@
101 application->installX11EventFilter(this);101 application->installX11EventFilter(this);
102 }102 }
103103
104 /* Get the system applications data dirs and be flexible if / is not at the
105 end of each path. */
106 QString xdgDataDir = QFile::decodeName(getenv("XDG_DATA_DIRS"));
107 if (xdgDataDir.isEmpty()) {
108 xdgDataDir = "/usr/local/share/:/usr/share/";
109 }
110 Q_FOREACH(const QString& dirName, xdgDataDir.split(':')) {
111 m_xdgApplicationDirs << QDir::cleanPath(dirName + "/applications") + "/";
112 }
113
104 load();114 load();
105}115}
106116
@@ -178,9 +188,17 @@
178}188}
179189
180QString190QString
181LauncherApplicationsList::favoriteFromDesktopFilePath(QString desktop_file)191LauncherApplicationsList::favoriteFromDesktopFilePath(const QString& _desktopFile) const
182{192{
183 return QDir(desktop_file).dirName();193 QString desktopFile(_desktopFile);
194 Q_FOREACH(const QString& applicationDir, m_xdgApplicationDirs) {
195 if (_desktopFile.startsWith(applicationDir)) {
196 desktopFile.remove(applicationDir);
197 desktopFile.replace("/", "-");
198 break;
199 }
200 }
201 return desktopFile;
184}202}
185203
186void204void
187205
=== modified file 'libunity-2d-private/src/launcherapplicationslist.h'
--- libunity-2d-private/src/launcherapplicationslist.h 2011-07-29 13:49:34 +0000
+++ libunity-2d-private/src/launcherapplicationslist.h 2011-08-09 09:23:30 +0000
@@ -71,7 +71,7 @@
71 void insertApplication(LauncherApplication* application);71 void insertApplication(LauncherApplication* application);
72 void removeApplication(LauncherApplication* application);72 void removeApplication(LauncherApplication* application);
7373
74 static QString favoriteFromDesktopFilePath(QString desktop_file);74 QString favoriteFromDesktopFilePath(const QString& desktop_file) const;
7575
76 void writeFavoritesToGConf();76 void writeFavoritesToGConf();
7777
@@ -90,6 +90,7 @@
90 */90 */
91 QHash<QString, LauncherApplication*> m_applicationForExecutable;91 QHash<QString, LauncherApplication*> m_applicationForExecutable;
92 QConf* m_dconf_launcher;92 QConf* m_dconf_launcher;
93 QStringList m_xdgApplicationDirs;
9394
94 /* Startup notification support */95 /* Startup notification support */
95 SnDisplay *m_snDisplay;96 SnDisplay *m_snDisplay;

Subscribers

People subscribed via source and target branches