Merge lp:~larryprice/ubuntu-app-launch/no-display-options into lp:ubuntu-app-launch/16.04

Proposed by Larry Price on 2016-04-25
Status: Merged
Approved by: Ted Gould on 2016-04-26
Approved revision: 272
Merged at revision: 222
Proposed branch: lp:~larryprice/ubuntu-app-launch/no-display-options
Merge into: lp:ubuntu-app-launch/16.04
Prerequisite: lp:~ted/ubuntu-app-launch/app-object
Diff against target: 239 lines (+136/-22)
2 files modified
libubuntu-app-launch/application-info-desktop.cpp (+59/-1)
tests/application-info-desktop.cpp (+77/-21)
To merge this branch: bzr merge lp:~larryprice/ubuntu-app-launch/no-display-options
Reviewer Review Type Date Requested Status
Ted Gould (community) 2016-04-25 Approve on 2016-04-26
Review via email: mp+292857@code.launchpad.net

Commit message

Add additional cases for not presenting desktop launchers based on keyfile entries.

Description of the change

Add additional cases for not presenting desktop launchers based on keyfile entries.

This includes looking for NoDisplay and Hidden, verifying the Type is Application, and checking to see if Unity is included in the "OnlyShowIn" or "NotShowIn" lists. This logic is similar to what we've been using in libertine-container-manager to determine which app launchers to show. The intent is to update the UAL API for use in libertine-scope to reduce duplication of logic in fetching application launchers.

To post a comment you must log in.
Ted Gould (ted) wrote :

Overall, cool, thanks for the fix! Some little things that need to be fixed inline.

review: Needs Fixing
272. By Larry Price on 2016-04-26

Fix memory management in stringlist method and use XDG_CURRENT_DESKTOP to determine environment (review recycle)

Ted Gould (ted) wrote :

Great, thanks!

review: Approve
273. By Larry Price on 2016-04-26

Merge with trunk 16.04

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
1=== modified file 'libubuntu-app-launch/application-info-desktop.cpp'
2--- libubuntu-app-launch/application-info-desktop.cpp 2016-03-03 20:48:24 +0000
3+++ libubuntu-app-launch/application-info-desktop.cpp 2016-04-26 13:49:48 +0000
4@@ -18,6 +18,7 @@
5 */
6
7 #include "application-info-desktop.h"
8+#include <cstdlib>
9
10 namespace ubuntu
11 {
12@@ -25,9 +26,20 @@
13 {
14 namespace app_info
15 {
16-
17+namespace
18+{
19 constexpr const char* DESKTOP_GROUP = "Desktop Entry";
20
21+struct TypeTag;
22+typedef TypeTagger<TypeTag, std::string> Type;
23+
24+struct HiddenTag;
25+typedef TypeTagger<HiddenTag, bool> Hidden;
26+
27+struct NoDisplayTag;
28+typedef TypeTagger<NoDisplayTag, bool> NoDisplay;
29+} // anonymous namespace
30+
31 template <typename T>
32 auto stringFromKeyfile(std::shared_ptr<GKeyFile> keyfile, const std::string& key, const std::string& exceptionText = {})
33 -> T
34@@ -113,12 +125,58 @@
35 return retval;
36 }
37
38+bool stringlistFromKeyfileContains(std::shared_ptr<GKeyFile> keyfile,
39+ const gchar* key,
40+ const std::string& match,
41+ bool defaultValue)
42+{
43+ GError* error = nullptr;
44+ auto results = g_key_file_get_string_list(keyfile.get(), DESKTOP_GROUP, key, nullptr, &error);
45+ if (error != nullptr)
46+ {
47+ g_error_free(error);
48+ return defaultValue;
49+ }
50+
51+ bool result = false;
52+ for (auto i = 0; results[i] != nullptr; ++i)
53+ {
54+ if (results[i] == match)
55+ {
56+ result = true;
57+ break;
58+ }
59+ }
60+ g_strfreev(results);
61+
62+ return result;
63+}
64+
65 Desktop::Desktop(std::shared_ptr<GKeyFile> keyfile, const std::string& basePath)
66 : _keyfile([keyfile]() {
67 if (!keyfile)
68 {
69 throw std::runtime_error("Can not build a desktop application info object with a null keyfile");
70 }
71+ if (stringFromKeyfile<Type>(keyfile, "Type").value() != "Application")
72+ {
73+ throw std::runtime_error("Keyfile does not represent application type");
74+ }
75+ if (boolFromKeyfile<NoDisplay>(keyfile, "NoDisplay", false).value())
76+ {
77+ throw std::runtime_error("Application is not meant to be displayed");
78+ }
79+ if (boolFromKeyfile<Hidden>(keyfile, "Hidden", false).value())
80+ {
81+ throw std::runtime_error("Application keyfile is hidden");
82+ }
83+ auto xdg_current_desktop = getenv("XDG_CURRENT_DESKTOP");
84+ if (stringlistFromKeyfileContains(keyfile, "NotShowIn", xdg_current_desktop, false)
85+ || !stringlistFromKeyfileContains(keyfile, "OnlyShowIn", xdg_current_desktop, true))
86+ {
87+ throw std::runtime_error("Application is not shown in Unity");
88+ }
89+
90 return keyfile;
91 }())
92 , _basePath(basePath)
93
94=== modified file 'tests/application-info-desktop.cpp'
95--- tests/application-info-desktop.cpp 2016-02-10 22:04:00 +0000
96+++ tests/application-info-desktop.cpp 2016-04-26 13:49:48 +0000
97@@ -20,28 +20,46 @@
98 #include "application-info-desktop.h"
99
100 #include <gtest/gtest.h>
101+#include <cstdlib>
102+
103+namespace
104+{
105+#define DESKTOP "Desktop Entry"
106
107 class ApplicationInfoDesktop : public ::testing::Test
108 {
109+protected:
110+ ApplicationInfoDesktop()
111+ : test_dekstop_env("SomeFreeDesktop")
112+ {
113+ }
114+
115 virtual void SetUp()
116 {
117+ setenv("XDG_CURRENT_DESKTOP", test_dekstop_env.c_str(), true);
118 }
119
120 virtual void TearDown()
121 {
122 }
123+
124+ std::shared_ptr<GKeyFile> defaultKeyfile()
125+ {
126+ auto keyfile = std::shared_ptr<GKeyFile>(g_key_file_new(), g_key_file_free);
127+ g_key_file_set_string(keyfile.get(), DESKTOP, "Type", "Application");
128+ g_key_file_set_string(keyfile.get(), DESKTOP, "Name", "Foo App");
129+ g_key_file_set_string(keyfile.get(), DESKTOP, "Exec", "foo");
130+ g_key_file_set_string(keyfile.get(), DESKTOP, "Icon", "foo.png");
131+ return keyfile;
132+ }
133+
134+ const std::string test_dekstop_env;
135 };
136
137-#define DESKTOP "Desktop Entry"
138
139 TEST_F(ApplicationInfoDesktop, DefaultState)
140 {
141- auto keyfile = std::shared_ptr<GKeyFile>(g_key_file_new(), g_key_file_free);
142- g_key_file_set_string(keyfile.get(), DESKTOP, "Name", "Foo App");
143- g_key_file_set_string(keyfile.get(), DESKTOP, "Exec", "foo");
144- g_key_file_set_string(keyfile.get(), DESKTOP, "Icon", "foo.png");
145-
146- auto appinfo = ubuntu::app_launch::app_info::Desktop(keyfile, "/");
147+ auto appinfo = ubuntu::app_launch::app_info::Desktop(defaultKeyfile(), "/");
148
149 EXPECT_EQ("Foo App", appinfo.name().value());
150 EXPECT_EQ("", appinfo.description().value());
151@@ -66,21 +84,61 @@
152
153 TEST_F(ApplicationInfoDesktop, KeyfileErrors)
154 {
155+ // empty
156 EXPECT_THROW(ubuntu::app_launch::app_info::Desktop({}, "/"), std::runtime_error);
157
158- auto noname = std::shared_ptr<GKeyFile>(g_key_file_new(), g_key_file_free);
159- g_key_file_set_string(noname.get(), DESKTOP, "Comment", "This is a comment");
160- g_key_file_set_string(noname.get(), DESKTOP, "Exec", "foo");
161- g_key_file_set_string(noname.get(), DESKTOP, "Icon", "foo.png");
162-
163+ // empty name
164+ auto noname = defaultKeyfile();
165+ g_key_file_remove_key(noname.get(), DESKTOP, "Name", nullptr);
166 EXPECT_THROW(ubuntu::app_launch::app_info::Desktop(noname, "/"), std::runtime_error);
167
168- auto noicon = std::shared_ptr<GKeyFile>(g_key_file_new(), g_key_file_free);
169- g_key_file_set_string(noicon.get(), DESKTOP, "Name", "Foo App");
170- g_key_file_set_string(noicon.get(), DESKTOP, "Comment", "This is a comment");
171- g_key_file_set_string(noicon.get(), DESKTOP, "Exec", "foo");
172-
173+ // empty icon
174+ auto noicon = defaultKeyfile();
175+ g_key_file_remove_key(noicon.get(), DESKTOP, "Icon", nullptr);
176 EXPECT_THROW(ubuntu::app_launch::app_info::Desktop(noicon, "/"), std::runtime_error);
177+
178+ // wrong type
179+ auto wrongtype = defaultKeyfile();
180+ g_key_file_set_string(wrongtype.get(), DESKTOP, "Type", "MimeType");
181+ EXPECT_THROW(ubuntu::app_launch::app_info::Desktop(wrongtype, "/"), std::runtime_error);
182+
183+ // not displayable
184+ auto nodisplay = defaultKeyfile();
185+ g_key_file_set_string(nodisplay.get(), DESKTOP, "NoDisplay", "true");
186+ EXPECT_THROW(ubuntu::app_launch::app_info::Desktop(nodisplay, "/"), std::runtime_error);
187+
188+ // hidden
189+ auto hidden = defaultKeyfile();
190+ g_key_file_set_string(hidden.get(), DESKTOP, "Hidden", "true");
191+ EXPECT_THROW(ubuntu::app_launch::app_info::Desktop(hidden, "/"), std::runtime_error);
192+
193+ // not shown in Unity
194+ auto notshowin = defaultKeyfile();
195+ g_key_file_set_string(notshowin.get(), DESKTOP, "NotShowIn", ("Gnome;" + test_dekstop_env + ";").c_str());
196+ EXPECT_THROW(ubuntu::app_launch::app_info::Desktop(notshowin, "/"), std::runtime_error);
197+
198+ // only show in not Unity
199+ auto onlyshowin = defaultKeyfile();
200+ g_key_file_set_string(onlyshowin.get(), DESKTOP, "OnlyShowIn", "KDE;Gnome;");
201+ EXPECT_THROW(ubuntu::app_launch::app_info::Desktop(onlyshowin, "/"), std::runtime_error);
202+}
203+
204+TEST_F(ApplicationInfoDesktop, KeyfileShowListEdgeCases)
205+{
206+ // Not appearing in not show list
207+ auto notshowin = defaultKeyfile();
208+ g_key_file_set_string(notshowin.get(), DESKTOP, "NotShowIn", "Gnome;KDE;");
209+ EXPECT_NO_THROW(ubuntu::app_launch::app_info::Desktop(notshowin, "/"));
210+
211+ // Appearing explicitly in only show list
212+ auto onlyshowin = defaultKeyfile();
213+ g_key_file_set_string(onlyshowin.get(), DESKTOP, "OnlyShowIn", (test_dekstop_env + ";Gnome;").c_str());
214+ EXPECT_NO_THROW(ubuntu::app_launch::app_info::Desktop(onlyshowin, "/"));
215+
216+ // Appearing explicitly in only show list not first
217+ auto onlyshowinmiddle = defaultKeyfile();
218+ g_key_file_set_string(onlyshowinmiddle.get(), DESKTOP, "OnlyShowIn", ("Gnome;" + test_dekstop_env + ";KDE;").c_str());
219+ EXPECT_NO_THROW(ubuntu::app_launch::app_info::Desktop(onlyshowinmiddle, "/"));
220 }
221
222 TEST_F(ApplicationInfoDesktop, Orientations)
223@@ -97,10 +155,7 @@
224 true
225 };
226
227- auto keyfile = std::shared_ptr<GKeyFile>(g_key_file_new(), g_key_file_free);
228- g_key_file_set_string(keyfile.get(), DESKTOP, "Name", "Foo App");
229- g_key_file_set_string(keyfile.get(), DESKTOP, "Exec", "foo");
230- g_key_file_set_string(keyfile.get(), DESKTOP, "Icon", "foo.png");
231+ auto keyfile = defaultKeyfile();
232
233 EXPECT_EQ(defaultOrientations, ubuntu::app_launch::app_info::Desktop(keyfile, "/").supportedOrientations());
234
235@@ -146,3 +201,4 @@
236 EXPECT_EQ(defaultOrientations,
237 ubuntu::app_launch::app_info::Desktop(keyfile, "/").supportedOrientations());
238 }
239+} //anonymous namespace

Subscribers

People subscribed via source and target branches