Merge lp:~osomon/unity-2d/custom-quicklists-static into lp:unity-2d/3.0

Proposed by Olivier Tilloy
Status: Merged
Approved by: Florian Boucault
Approved revision: 552
Merged at revision: 550
Proposed branch: lp:~osomon/unity-2d/custom-quicklists-static
Merge into: lp:unity-2d/3.0
Diff against target: 167 lines (+55/-9)
4 files modified
launcher/UnityApplications/CMakeLists.txt (+3/-0)
launcher/UnityApplications/launcherapplication.cpp (+34/-1)
launcher/UnityApplications/launcherapplication.h (+7/-0)
launcher/UnityApplications/launchermenu.cpp (+11/-8)
To merge this branch: bzr merge lp:~osomon/unity-2d/custom-quicklists-static
Reviewer Review Type Date Requested Status
Florian Boucault (community) code Approve
Review via email: mp+59329@code.launchpad.net

Commit message

[launcher] Support static shortcuts in the quicklists.

Static shortcuts are defined in the application’s desktop file.
See https://wiki.ubuntu.com/Unity/LauncherAPI#Static%20Quicklist%20entries for documentation.

To post a comment you must log in.
Revision history for this message
Florian Boucault (fboucault) wrote :

Behaves properly after extensive testing.

review: Approve (functional)
Revision history for this message
Florian Boucault (fboucault) wrote :

In launcher/UnityApplications/launchermenu.cpp:

[...]
    if (folded) {
        /* Remove all actions but the title. */
        for (int i = actions().size(); i > 0; --i) {
            QAction* action = actions().at(i - 1);
[...]

The use of a reverse for loop does not seem necessary. A forward one would be simpler. And even better would be to use a Q_FOREACH.

review: Needs Fixing
Revision history for this message
Olivier Tilloy (osomon) wrote :

> The use of a reverse for loop does not seem necessary. A forward one would be
> simpler. And even better would be to use a Q_FOREACH.

It *is* necessary because in this loop we are potentially removing the current action from the list, thus invalidating greater indexes. Iterating backward ensures the current index is always valid without having to resort to complex arithmetics.

Revision history for this message
Florian Boucault (fboucault) wrote :

> > The use of a reverse for loop does not seem necessary. A forward one would
> be
> > simpler. And even better would be to use a Q_FOREACH.
>
> It *is* necessary because in this loop we are potentially removing the current
> action from the list, thus invalidating greater indexes. Iterating backward
> ensures the current index is always valid without having to resort to complex
> arithmetics.

I had not seen that.
Q_FOREACH would still work in that case because it operates on a copy of the list as soon as it is modified I believe.

Revision history for this message
Florian Boucault (fboucault) wrote :

The rest of the code looks good.

review: Approve (code)

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
1=== modified file 'launcher/UnityApplications/CMakeLists.txt'
2--- launcher/UnityApplications/CMakeLists.txt 2011-03-22 22:55:11 +0000
3+++ launcher/UnityApplications/CMakeLists.txt 2011-04-28 08:01:10 +0000
4@@ -8,6 +8,7 @@
5 pkg_check_modules(GIO REQUIRED gio-2.0)
6 pkg_check_modules(WNCK REQUIRED libwnck-1.0)
7 pkg_check_modules(STARTUPNOTIFICATION REQUIRED libstartup-notification-1.0)
8+pkg_check_modules(INDICATOR REQUIRED indicator)
9
10 # Sources
11 set(UnityApplications_SRCS
12@@ -70,6 +71,7 @@
13 ${GIO_INCLUDE_DIRS}
14 ${WNCK_INCLUDE_DIRS}
15 ${STARTUPNOTIFICATION_INCLUDE_DIRS}
16+ ${INDICATOR_INCLUDE_DIRS}
17 )
18
19 target_link_libraries(UnityApplications
20@@ -86,6 +88,7 @@
21 ${GIO_LDFLAGS}
22 ${WNCK_LDFLAGS}
23 ${STARTUPNOTIFICATION_LDFLAGS}
24+ ${INDICATOR_LDFLAGS}
25 )
26
27 # Install
28
29=== modified file 'launcher/UnityApplications/launcherapplication.cpp'
30--- launcher/UnityApplications/launcherapplication.cpp 2011-04-14 14:27:36 +0000
31+++ launcher/UnityApplications/launcherapplication.cpp 2011-04-28 08:01:10 +0000
32@@ -61,6 +61,8 @@
33 #include <libsn/sn.h>
34 }
35
36+const char* SHORTCUT_NICK_PROPERTY = "nick";
37+
38 LauncherApplication::LauncherApplication()
39 : m_application(NULL)
40 , m_desktopFileWatcher(NULL)
41@@ -290,6 +292,10 @@
42 Q_EMIT executableChanged(executable());
43 }
44
45+ /* Update the list of static shortcuts
46+ (quicklist entries defined in the desktop file). */
47+ m_staticShortcuts.reset(indicator_desktop_shortcuts_new(newDesktopFile.toUtf8().constData(), "Unity"));
48+
49 monitorDesktopFile(newDesktopFile);
50 }
51
52@@ -840,9 +846,27 @@
53 void
54 LauncherApplication::createStaticMenuActions()
55 {
56- m_menu->addSeparator();
57 QList<QAction*> actions;
58
59+ /* Custom menu actions from the desktop file. */
60+ if (!m_staticShortcuts.isNull()) {
61+ const gchar** nicks = indicator_desktop_shortcuts_get_nicks(m_staticShortcuts.data());
62+ if (nicks) {
63+ int i = 0;
64+ while (((gpointer*) nicks)[i]) {
65+ const gchar* nick = nicks[i];
66+ QAction* action = new QAction(m_menu);
67+ action->setText(QString::fromUtf8(indicator_desktop_shortcuts_nick_get_name(m_staticShortcuts.data(), nick)));
68+ action->setProperty(SHORTCUT_NICK_PROPERTY, QVariant(nick));
69+ actions.append(action);
70+ connect(action, SIGNAL(triggered()), SLOT(onStaticShortcutTriggered()));
71+ ++i;
72+ }
73+ }
74+ }
75+ m_menu->insertActions(m_menu->actions().first(), actions);
76+
77+ actions.clear();
78 bool is_running = running();
79
80 /* Only applications with a corresponding desktop file can be kept in the launcher */
81@@ -911,6 +935,15 @@
82 }
83
84 void
85+LauncherApplication::onStaticShortcutTriggered()
86+{
87+ QAction* action = static_cast<QAction*>(sender());
88+ QString nick = action->property(SHORTCUT_NICK_PROPERTY).toString();
89+ m_menu->hide();
90+ indicator_desktop_shortcuts_nick_exec(m_staticShortcuts.data(), nick.toUtf8().constData());
91+}
92+
93+void
94 LauncherApplication::onKeepTriggered()
95 {
96 QAction* keep = static_cast<QAction*>(sender());
97
98=== modified file 'launcher/UnityApplications/launcherapplication.h'
99--- launcher/UnityApplications/launcherapplication.h 2011-04-07 09:16:50 +0000
100+++ launcher/UnityApplications/launcherapplication.h 2011-04-28 08:01:10 +0000
101@@ -20,6 +20,9 @@
102 #include <gio/gdesktopappinfo.h>
103 #include <libwnck/libwnck.h>
104
105+// libindicator
106+#include <libindicator/indicator-desktop-shortcuts.h>
107+
108 #include "launcheritem.h"
109
110 // libunity-2d
111@@ -47,6 +50,7 @@
112 typedef GObjectScopedPointer<GAppInfo> GAppInfoPointer;
113 typedef GObjectScopedPointer<GDesktopAppInfo> GDesktopAppInfoPointer;
114 typedef GScopedPointer<SnStartupSequence, sn_startup_sequence_unref> SnStartupSequencePointer;
115+typedef GObjectScopedPointer<IndicatorDesktopShortcuts> IndicatorDesktopShortcutsPointer;
116 class LauncherApplication : public LauncherItem
117 {
118 Q_OBJECT
119@@ -135,6 +139,7 @@
120 void show();
121
122 /* Contextual menu callbacks */
123+ void onStaticShortcutTriggered();
124 void onKeepTriggered();
125 void onQuitTriggered();
126
127@@ -175,6 +180,8 @@
128 template<typename T>
129 bool updateOverlayState(QMap<QString, QVariant> properties,
130 QString propertyName, T* member);
131+
132+ IndicatorDesktopShortcutsPointer m_staticShortcuts;
133 };
134
135 Q_DECLARE_METATYPE(LauncherApplication*)
136
137=== modified file 'launcher/UnityApplications/launchermenu.cpp'
138--- launcher/UnityApplications/launchermenu.cpp 2011-03-21 13:03:36 +0000
139+++ launcher/UnityApplications/launchermenu.cpp 2011-04-28 08:01:10 +0000
140@@ -190,17 +190,20 @@
141 }
142
143 if (folded) {
144- /* Remove all actions but the first one (the title). */
145- while (actions().size() > 1) {
146- QAction* action = actions().last();
147- removeAction(action);
148- if (action->parent() == this) {
149- /* Delete the action only if we "own" it,
150- otherwise let its parent take care of it. */
151- delete action;
152+ /* Remove all actions but the title. */
153+ for (int i = actions().size(); i > 0; --i) {
154+ QAction* action = actions().at(i - 1);
155+ if (action != m_titleAction) {
156+ removeAction(action);
157+ if (action->parent() == this) {
158+ /* Delete the action only if we "own" it,
159+ otherwise let its parent take care of it. */
160+ delete action;
161+ }
162 }
163 }
164 } else {
165+ insertSeparator(m_titleAction);
166 addSeparator();
167 m_launcherItem->createMenuActions();
168

Subscribers

People subscribed via source and target branches