Merge lp:~larryprice/libertine-scope/update-app-preview into lp:libertine-scope

[Larry Price]

Redesign preview pane with reasonably-sized image, title, and description to closer match the store preview.

Redesign preview pane with reasonably-sized image, title, and description to closer match the store preview.

[lp:~larryprice/libertine-scope/update-app-preview]

40. By Larry Price on 2016-04-14

use the unity ini file parser instead of rolling our own to parse the desktop file

41. By Larry Price on 2016-04-14

comparing list ptr to nullptr instead of NULL in preview test

42. By Larry Price on 2016-04-14

sticking constants into anonymous namespace

[Christopher Townsend (townsend)]

This looks great to me. I'll wait on Stephen to chime in before we top approve.

review: Approve

[Stephen M. Webb (bregma)]

Nice, but please back out the unrelated change to tests/scopefixture.h (see inline comment). There is a larger and unrelated change required here.

review: Needs Fixing

[lp:~larryprice/libertine-scope/update-app-preview]

43. By Larry Price on 2016-04-15

Reverting change to scopefixture.h as it is related to a larger change

[Christopher Townsend (townsend)]

Top approving since Stephen's comment has been taken care of.

[Stephen M. Webb (bregma)]

review: Approve

=== added file 'libertine-scope/app_description.cpp'
--- libertine-scope/app_description.cpp	1970-01-01 00:00:00 +0000
+++ libertine-scope/app_description.cpp	2016-04-15 12:42:14 +0000
@@ -0,0 +1,57 @@
+/*
+ * Copyright 2016 Canonical Ltd.
+ *
+ * This program is free software: you can redistribute it and/or modify it under
+ * the terms of the GNU General Public License, version 3, as published by the
+ * Free Software Foundation.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+ * GNU General Public License for more details.
+ *
+ * You should have received a copy of the GNU General Public License
+ * along with this program.  If not, see <http://www.gnu.org/licenses/>.
+ */
+
+#include "libertine-scope/app_description.h"
+#include <unity/scopes/Variant.h>
+#include <unity/util/IniParser.h>
+#include <QFileInfo>
+#include <QStringList>
+
+
+namespace
+{
+static const std::string DESKTOP_FILE_GROUP("Desktop Entry");
+static const std::string DESKTOP_FILE_KEY_COMMENT("Comment");
+}
+
+
+AppDescription::
+AppDescription(const std::string& desktop_file, const std::string& locale)
+  : desktop_file_(desktop_file), language_(locale)
+{
+}
+
+
+AppDescription::
+~AppDescription()
+{
+}
+
+
+unity::scopes::Variant AppDescription::
+description() const
+{
+  if (QFileInfo(QString::fromStdString(desktop_file_)).exists())
+  {
+    unity::util::IniParser parser(desktop_file_.c_str());
+    if (parser.has_key(DESKTOP_FILE_GROUP, DESKTOP_FILE_KEY_COMMENT))
+    {
+      return unity::scopes::Variant(parser.get_locale_string(DESKTOP_FILE_GROUP, DESKTOP_FILE_KEY_COMMENT, language_));
+    }
+  }
+
+  return unity::scopes::Variant("");
+}

=== added file 'libertine-scope/app_description.h'
--- libertine-scope/app_description.h	1970-01-01 00:00:00 +0000
+++ libertine-scope/app_description.h	2016-04-15 12:42:14 +0000
@@ -0,0 +1,41 @@
+/*
+ * Copyright 2016 Canonical Ltd.
+ *
+ * This program is free software: you can redistribute it and/or modify it under
+ * the terms of the GNU General Public License, version 3, as published by the
+ * Free Software Foundation.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+ * GNU General Public License for more details.
+ *
+ * You should have received a copy of the GNU General Public License
+ * along with this program.  If not, see <http://www.gnu.org/licenses/>.
+ */
+
+#ifndef APP_DESCRIPTION_H
+#define APP_DESCRIPTION_H
+
+#include <string>
+
+namespace unity {
+  namespace scopes {
+    class Variant;
+  }
+}
+
+class AppDescription
+{
+public:
+  explicit
+  AppDescription(const std::string& desktop_file, const std::string& locale);
+  ~AppDescription();
+  unity::scopes::Variant description() const;
+
+private:
+  const std::string& desktop_file_;
+  const std::string& language_;
+};
+
+#endif

=== modified file 'libertine-scope/preview.cpp'
--- libertine-scope/preview.cpp	2016-01-19 21:10:09 +0000
+++ libertine-scope/preview.cpp	2016-04-15 12:42:14 +0000
@@ -13,16 +13,21 @@
  * You should have received a copy of the GNU General Public License
  * along with this program.  If not, see <http://www.gnu.org/licenses/>.
  */
+
 #include "libertine-scope/preview.h"
+#include "libertine-scope/app_description.h"
 
 #include <unity/scopes/PreviewReply.h>
 #include <unity/scopes/Variant.h>
 #include <unity/scopes/VariantBuilder.h>
+#include <unity/scopes/ActionMetadata.h>
+#include <unity/scopes/Result.h>
 
+namespace usc = unity::scopes;
 
 Preview::
-Preview(unity::scopes::Result const&         result,
-        unity::scopes::ActionMetadata const& metadata)
+Preview(usc::Result const&         result,
+        usc::ActionMetadata const& metadata)
 : PreviewQueryBase(result, metadata)
 {
 }
@@ -41,22 +46,24 @@
 
 
 void Preview::
-run(unity::scopes::PreviewReplyProxy const& reply)
+run(usc::PreviewReplyProxy const& reply)
 {
-  reply->push("description", unity::scopes::Variant("A Description"));
-
-  unity::scopes::PreviewWidget image_widget("myimage", "image");
-  image_widget.add_attribute_mapping("source", "art");
-
-  unity::scopes::PreviewWidget buttons("buttons", "actions");
-  unity::scopes::VariantBuilder vb;
+  usc::PreviewWidget header("hdr", "header");
+  header.add_attribute_mapping("title", "title");
+  header.add_attribute_mapping("mascot", "art");
+
+  usc::PreviewWidget buttons("buttons", "actions");
+  usc::VariantBuilder vb;
   vb.add_tuple({
-    {"id",    unity::scopes::Variant("open")},
-    {"label", unity::scopes::Variant("Open")},
+    {"id",    usc::Variant("open")},
+    {"label", usc::Variant("Open")},
   });
   buttons.add_attribute_value("actions", vb.end());
 
-  unity::scopes::PreviewWidgetList widgets{ image_widget, buttons };
+  auto desktop_file_name = result()["desktop_file"].get_string();
+  usc::PreviewWidget desc("desc", "text");
+  desc.add_attribute_value("text", AppDescription(desktop_file_name, action_metadata().locale()).description());
+
+  usc::PreviewWidgetList widgets{ header, desc, buttons };
   reply->push(widgets);
 }
-

=== modified file 'libertine-scope/query.cpp'
--- libertine-scope/query.cpp	2016-01-19 21:10:09 +0000
+++ libertine-scope/query.cpp	2016-04-15 12:42:14 +0000
@@ -96,12 +96,15 @@
     for (auto const& app: container->app_launchers())
     {
       if (app.no_display())
+      {
         continue;
+      }
 
       usc::CategorisedResult result(category);
       result.set_title(app.name());
       result.set_art(app.icon());
       result.set_uri(app_uri(*container, app));
+      result["desktop_file"] = app.desktop_file();
       if (!reply->push(result))
       {
         break;
@@ -109,4 +112,3 @@
     }
   }
 }
-

=== modified file 'po/libertine-scope.pot'
--- po/libertine-scope.pot	2016-02-27 03:54:52 +0000
+++ po/libertine-scope.pot	2016-04-15 12:42:14 +0000
@@ -8,7 +8,7 @@
 msgstr ""
 "Project-Id-Version: PACKAGE VERSION\n"
 "Report-Msgid-Bugs-To: \n"
-"POT-Creation-Date: 2016-02-26 22:50-0500\n"
+"POT-Creation-Date: 2016-04-14 13:30-0400\n"
 "PO-Revision-Date: YEAR-MO-DA HO:MI+ZONE\n"
 "Last-Translator: FULL NAME <EMAIL@ADDRESS>\n"
 "Language-Team: LANGUAGE <[e-mail]>\n"

=== modified file 'tests/CMakeLists.txt'
--- tests/CMakeLists.txt	2016-01-19 00:16:56 +0000
+++ tests/CMakeLists.txt	2016-04-15 12:42:14 +0000
@@ -6,7 +6,11 @@
 add_executable(libertine_scope_tests
   fake_container.cpp
   fake_libertine.cpp
+
+  # tests
   test_scope.cpp
+  test_app_description.cpp
+  test_preview.cpp
 )
 
 target_link_libraries(libertine_scope_tests
@@ -16,6 +20,8 @@
   gmock_main
 )
 
-add_test(test_scope
-  libertine_scope_tests
-)
+add_test(test_scope libertine_scope_tests)
+add_test(test_app_description libertine_scope_tests)
+add_test(test_preview libertine_scope_tests)
+
+execute_process(COMMAND ${CMAKE_COMMAND} -E create_symlink ${CMAKE_SOURCE_DIR}/tests/data/ ${CMAKE_CURRENT_BINARY_DIR}/data)

=== added directory 'tests/data'
=== added file 'tests/data/Full.desktop'
--- tests/data/Full.desktop	1970-01-01 00:00:00 +0000
+++ tests/data/Full.desktop	2016-04-15 12:42:14 +0000
@@ -0,0 +1,13 @@
+[Desktop Entry]
+Encoding=UTF-8
+Name=Matchbox
+Comment=This session logs you into Matchbox
+Comment[da]=Surf på internettet
+Comment[de]=Im Internet surfen
+Comment[el]=Μπορείτε να περιηγηθείτε στο διαδίκτυο (Web)
+Comment[es]=Navegue por la web
+Comment[et]=Lehitse veebi
+Exec=matchbox-session
+Terminal=False
+Icon=
+Type=Application

=== added file 'tests/data/NoComment.desktop'
--- tests/data/NoComment.desktop	1970-01-01 00:00:00 +0000
+++ tests/data/NoComment.desktop	2016-04-15 12:42:14 +0000
@@ -0,0 +1,7 @@
+[Desktop Entry]
+Encoding=UTF-8
+Name=Matchbox
+Exec=matchbox-session
+Terminal=False
+Icon=
+Type=Application

=== added file 'tests/test_app_description.cpp'
--- tests/test_app_description.cpp	1970-01-01 00:00:00 +0000
+++ tests/test_app_description.cpp	2016-04-15 12:42:14 +0000
@@ -0,0 +1,61 @@
+/*
+ * Copyright 2016 Canonical Ltd.
+ *
+ * This program is free software: you can redistribute it and/or modify it under
+ * the terms of the GNU General Public License, version 3, as published by the
+ * Free Software Foundation.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+ * GNU General Public License for more details.
+ *
+ * You should have received a copy of the GNU General Public License
+ * along with this program.  If not, see <http://www.gnu.org/licenses/>.
+ */
+
+#include "libertine-scope/app_description.h"
+#include <unity/scopes/Variant.h>
+#include <gtest/gtest.h>
+#include <QDir>
+
+
+TEST(TestAppDescription, returnsEmptyStringForEmptyNamedFile)
+{
+	AppDescription desc("", "");
+	EXPECT_EQ("", desc.description().get_string());
+}
+
+TEST(TestAppDescription, returnsEmptyStringForNonExistentDesktopFile)
+{
+	AppDescription desc("/non/existent/file", "");
+	EXPECT_EQ("", desc.description().get_string());
+}
+
+TEST(TestAppDescription, returnsEmptyStringForDesktopFileWithoutComment)
+{
+	auto data_path = QString("%1/data/NoComment.desktop").arg(QDir::currentPath()).toStdString();
+	AppDescription desc(data_path, "");
+	EXPECT_EQ("", desc.description().get_string());
+}
+
+TEST(TestAppDescription, returnsTopLevelCommentWhenNoLocaleSpecified)
+{
+	auto data_path = QString("%1/data/Full.desktop").arg(QDir::currentPath()).toStdString();
+	AppDescription desc(data_path, "");
+	EXPECT_EQ("This session logs you into Matchbox", desc.description().get_string());
+}
+
+TEST(TestAppDescription, returnsTopLevelCommentWhenLocaleNotAvailable)
+{
+	auto data_path = QString("%1/data/Full.desktop").arg(QDir::currentPath()).toStdString();
+	AppDescription desc(data_path, "fr_FR");
+	EXPECT_EQ("This session logs you into Matchbox", desc.description().get_string());
+}
+
+TEST(TestAppDescription, returnsLocaleBasedComment)
+{
+	auto data_path = QString("%1/data/Full.desktop").arg(QDir::currentPath()).toStdString();
+	AppDescription desc(data_path, "de_DE");
+	EXPECT_EQ("Im Internet surfen", desc.description().get_string());
+}

=== added file 'tests/test_preview.cpp'
--- tests/test_preview.cpp	1970-01-01 00:00:00 +0000
+++ tests/test_preview.cpp	2016-04-15 12:42:14 +0000
@@ -0,0 +1,64 @@
+/*
+ * Copyright 2016 Canonical Ltd.
+ *
+ * This program is free software: you can redistribute it and/or modify it under
+ * the terms of the GNU General Public License, version 3, as published by the
+ * Free Software Foundation.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+ * GNU General Public License for more details.
+ *
+ * You should have received a copy of the GNU General Public License
+ * along with this program.  If not, see <http://www.gnu.org/licenses/>.
+ */
+
+#include "libertine-scope/preview.h"
+#include <unity/scopes/Variant.h>
+#include <gtest/gtest.h>
+#include <unity/scopes/ActionMetadata.h>
+#include <unity/scopes/testing/Result.h>
+#include <unity/scopes/testing/MockPreviewReply.h>
+#include <unity/scopes/PreviewWidget.h>
+#include <memory>
+#include <QDir>
+
+
+TEST(TestPreview, pushesWidgetsWithAppInformation)
+{
+  unity::scopes::testing::Result result;
+  result["desktop_file"] = unity::scopes::Variant(QString("%1/data/Full.desktop").arg(QDir::currentPath()).toStdString());
+  unity::scopes::ActionMetadata metadata("en_US", "phone");
+
+  std::unique_ptr<unity::scopes::PreviewWidgetList> list(new unity::scopes::PreviewWidgetList());
+	testing::NiceMock<unity::scopes::testing::MockPreviewReply> reply;
+  EXPECT_CALL(reply, push(testing::_)).WillOnce(testing::SaveArg<0>(list.get()));
+  unity::scopes::PreviewReplyProxy proxy(&reply, [](unity::scopes::PreviewReply*) {});
+
+	Preview preview(result, metadata);
+	preview.run(proxy);
+
+  ASSERT_NE(nullptr, list);
+  ASSERT_EQ(3, list->size());
+
+  auto header = list->front();
+  EXPECT_EQ("hdr", header.id());
+  EXPECT_EQ("header", header.widget_type());
+  EXPECT_EQ("title", header.attribute_mappings()["title"]);
+  EXPECT_EQ("art", header.attribute_mappings()["mascot"]);
+
+  list->pop_front();
+  auto desc = list->front();
+  EXPECT_EQ("desc", desc.id());
+  EXPECT_EQ("This session logs you into Matchbox", desc.attribute_values()["text"].get_string());
+
+  list->pop_front();
+  auto buttons = list->front();
+  EXPECT_EQ("buttons", buttons.id());
+  EXPECT_EQ("actions", buttons.widget_type());
+  auto buttons_actions = buttons.attribute_values()["actions"].get_array();
+  ASSERT_EQ(1, buttons_actions.size());
+  EXPECT_EQ("open", buttons_actions[0].get_dict()["id"].get_string());
+  EXPECT_EQ("Open", buttons_actions[0].get_dict()["label"].get_string());
+}

=== modified file 'tests/test_scope.cpp'
--- tests/test_scope.cpp	2016-01-19 00:16:56 +0000
+++ tests/test_scope.cpp	2016-04-15 12:42:14 +0000
@@ -54,4 +54,3 @@
   SearchReplyProxy search_reply_proxy(&reply, [](unity::scopes::SearchReply*) {});
   search_query->run(search_reply_proxy);
 }
-