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
1=== modified file 'libunity-2d-private/src/launcherapplicationslist.cpp'
2--- libunity-2d-private/src/launcherapplicationslist.cpp 2011-07-29 13:49:34 +0000
3+++ libunity-2d-private/src/launcherapplicationslist.cpp 2011-08-09 09:23:30 +0000
4@@ -101,6 +101,16 @@
5 application->installX11EventFilter(this);
6 }
7
8+ /* Get the system applications data dirs and be flexible if / is not at the
9+ end of each path. */
10+ QString xdgDataDir = QFile::decodeName(getenv("XDG_DATA_DIRS"));
11+ if (xdgDataDir.isEmpty()) {
12+ xdgDataDir = "/usr/local/share/:/usr/share/";
13+ }
14+ Q_FOREACH(const QString& dirName, xdgDataDir.split(':')) {
15+ m_xdgApplicationDirs << QDir::cleanPath(dirName + "/applications") + "/";
16+ }
17+
18 load();
19 }
20
21@@ -178,9 +188,17 @@
22 }
23
24 QString
25-LauncherApplicationsList::favoriteFromDesktopFilePath(QString desktop_file)
26+LauncherApplicationsList::favoriteFromDesktopFilePath(const QString& _desktopFile) const
27 {
28- return QDir(desktop_file).dirName();
29+ QString desktopFile(_desktopFile);
30+ Q_FOREACH(const QString& applicationDir, m_xdgApplicationDirs) {
31+ if (_desktopFile.startsWith(applicationDir)) {
32+ desktopFile.remove(applicationDir);
33+ desktopFile.replace("/", "-");
34+ break;
35+ }
36+ }
37+ return desktopFile;
38 }
39
40 void
41
42=== modified file 'libunity-2d-private/src/launcherapplicationslist.h'
43--- libunity-2d-private/src/launcherapplicationslist.h 2011-07-29 13:49:34 +0000
44+++ libunity-2d-private/src/launcherapplicationslist.h 2011-08-09 09:23:30 +0000
45@@ -71,7 +71,7 @@
46 void insertApplication(LauncherApplication* application);
47 void removeApplication(LauncherApplication* application);
48
49- static QString favoriteFromDesktopFilePath(QString desktop_file);
50+ QString favoriteFromDesktopFilePath(const QString& desktop_file) const;
51
52 void writeFavoritesToGConf();
53
54@@ -90,6 +90,7 @@
55 */
56 QHash<QString, LauncherApplication*> m_applicationForExecutable;
57 QConf* m_dconf_launcher;
58+ QStringList m_xdgApplicationDirs;
59
60 /* Startup notification support */
61 SnDisplay *m_snDisplay;

Subscribers

People subscribed via source and target branches