Merge lp:~alecu/unity-scope-click/broken-dot-desktop into lp:unity-scope-click/devel

[Alejandro J. Cura]

Check for valid .desktop files

[dobey (dobey)]

Do we not need to trap the LogicException here? I had thought that we would need to do so, as well as ignoring otherwise broken .desktop files which are just text but not .desktop files.

review: Needs Information

[PS Jenkins bot (ps-jenkins)]

PASSED: Continuous integration, rev:224
http://jenkins.qa.ubuntu.com/job/unity-team-unity-scope-click-devel-ci/15/
Executed test runs:
    SUCCESS: http://jenkins.qa.ubuntu.com/job/unity-team-unity-scope-click-devel-trusty-amd64-ci/15
    SUCCESS: http://jenkins.qa.ubuntu.com/job/unity-team-unity-scope-click-devel-trusty-armhf-ci/15
        deb: http://jenkins.qa.ubuntu.com/job/unity-team-unity-scope-click-devel-trusty-armhf-ci/15/artifact/work/output/*zip*/output.zip
    SUCCESS: http://jenkins.qa.ubuntu.com/job/unity-team-unity-scope-click-devel-trusty-i386-ci/15

Click here to trigger a rebuild:
http://s-jenkins.ubuntu-ci:8080/job/unity-team-unity-scope-click-devel-ci/15/rebuild

review: Approve (continuous-integration)

[Alejandro J. Cura (alecu)]

> Do we not need to trap the LogicException here? I had thought that we would
> need to do so, as well as ignoring otherwise broken .desktop files which are
> just text but not .desktop files.

We don't need to trap that exception, because the .has_group() method is declared noexcept, and if the DESKTOP_FILE_GROUP is not present then that's enough to catch every case. I'm testing for that with the broken.desktop file, which has 100 bytes of zero, and with semi-broken.desktop, which has just a text line, and with the has_group() fix both cases are handled well.

[dobey (dobey)]

review: Approve

=== modified file 'scope/click/interface.cpp'
--- scope/click/interface.cpp	2014-04-22 22:24:46 +0000
+++ scope/click/interface.cpp	2014-04-28 22:12:28 +0000
@@ -136,6 +136,10 @@
     auto enumerator = [&result, &installTimes, this, search_query, include_desktop_results]
             (const unity::util::IniParser& keyFile, const std::string& filename)
     {
+        if (keyFile.has_group(DESKTOP_FILE_GROUP) == false) {
+            qWarning() << "Broken desktop file:" << QString::fromStdString(filename);
+            return;
+        }
         if (is_visible_app(keyFile) == false) {
             return; // from the enumerator lambda
         }

=== added file 'scope/tests/applications/user/broken.desktop'
Binary files scope/tests/applications/user/broken.desktop	1970-01-01 00:00:00 +0000 and scope/tests/applications/user/broken.desktop	2014-04-28 22:12:28 +0000 differ
=== added file 'scope/tests/applications/user/semi-broken.desktop'
--- scope/tests/applications/user/semi-broken.desktop	1970-01-01 00:00:00 +0000
+++ scope/tests/applications/user/semi-broken.desktop	2014-04-28 22:12:28 +0000
@@ -0,0 +1,1 @@
+semi broken file