Merge lp:~osomon/unity/dash-custom-home-screen into lp:unity

[Olivier Tilloy]

Allow customizing the dash’s home screen.

The contents of the custom home screen are described in a JSON file called HomeShortcutsCustomized.json.

The file is looked for in the following locations in decreasing order of priority:
    - $XDG_CONFIG_HOME/unity/ (defaults to $HOME/.config/unity/)
    - $DIR/unity/ for $DIR in $XDG_CONFIG_DIRS

The syntax of the file is as follows:
==========================================================================
{
  "shortcut1": {
    "source": $source,
    "name": $name,
    "name[fr]": $name_in_french,
    […]
    "icon": $icon
  },
  "shortcut2": {
    […]
  },
  […]
}
==========================================================================

The source attribute may either be a desktop file (full path or just its basename if it is located in a standard directory), or a lens file (basename only). This attribute is mandatory.
The 'name' attribute is optional. If present, it will override the default display name as advertised by the desktop file or by the lens. The name can be localized in several languages using the square brackets suffix notation, in which case the locale matching the system’s will be used, defaulting to the untranslated 'name' attribute if necessary.
The 'icon' attribute is optional. If present, it will override the default icon as advertised by the desktop file or by the lens. It should be a full path name.
If the source is a lens, the optional 'filter' attribute allows specifying a filter in the form "$name:$value", e.g. "type:videos" for the files lens.
If the source is a lens, the optional 'section' attribute allows specifying a section number (an integer starting at index 0).

So it seems that the patch I wrote some time ago to make the home screen of the dash customizable just like in unity-2d is being considered for inclusion in Oneiric…

I refreshed it to apply against the current trunk, functional testing and comments are welcome. Please bear with me as I’m not really familiar with unity’s code base, coding style and conventions.

[Mikkel Kamstrup Erlandsen (kamstrup)]

I am not sure how this relates to bug #885738...

[Olivier Tilloy (osomon)]

Yeah, I know this is in apparent contradiction with the description of bug #885738 and with what was discussed at UDS.
But OEM needs this functionality for a project running Oneiric, and they want the patch reviewed and approved by Unity core developers, hence this merge request. Whether it should actually be merged into trunk is beyond me, but a review and functional tests would be nice. Thanks!

[Gary Ekker (gekker)]

We need an SRU for oneiric that includes this patch.

As per bug #885738, this won't be a problem for precise and beyond.

[Gord Allott (gordallott)]

On 23/11/11 15:31, Olivier Tilloy wrote:
> Olivier Tilloy has proposed merging lp:~osomon/unity/dash-custom-home-screen into lp:unity.
>
> Requested reviews:
> Unity Team (unity-team): code functional
> Related bugs:
> Bug #785840 in unity: "Home dash icons need to be customizable"
> https://bugs.launchpad.net/unity/+bug/785840
>
> For more details, see:
> https://code.launchpad.net/~osomon/unity/dash-custom-home-screen/+merge/83175
>
> So it seems that the patch I wrote some time ago to make the home screen of the dash customizable just like in unity-2d is being considered for inclusion in Oneiric…
>
> I refreshed it to apply against the current trunk, functional testing and comments are welcome. Please bear with me as I’m not really familiar with unity’s code base, coding style and conventions.

So I have a few questions and a few notes;

I have some concerns over adding extra translation in a non standard
format, there are good well established formats for .desktop files and
in source code, but nothing for a custom json file. We should really
either be extending the desktop files to include an even more generic
name that is translated such as:
X-Unity-Generic-Action=Browse the web

or creating new desktop files and installing them somewhere like
/usr/share/unity/dash/home - using those to store the required
information. I'd like a compelling reason to be using json over desktop
files really.

I'm not a fan of having the old code and this new code sit side-by-side,
we should stick to one method and lose the other. Which means that we
should be including a default json file in unity that gets installed to
somewhere sane like /usr/share/unity/dash/.

There is also an issue in that non of this is tested, thus it can't be
approved until tests exist.

small thing, but style wise, we shouldn't be using #define's, (the
define that exists is from quite old code before we started using this
style)

namespace
{
  std::string custom_shortcuts_file =
"/unity/HomeShortcuts/Customized.json";
}

is a preferred style.

Apart from that the code looks good

 review needs-info

--
Gordon Allott
Canonical Ltd.
27 Floor, Millbank Tower
London SW1P 4QP
www.canonical.com

review: Needs Information

[lp:~osomon/unity/dash-custom-home-screen]

1741. By Olivier Tilloy on 2011-11-30

Cosmetics: replace a #define by a global variable in the unnamed namespace.

[Olivier Tilloy (osomon)]

Thanks for the review Gordon!

> I have some concerns over adding extra translation in a non standard
> format, there are good well established formats for .desktop files and
> in source code, but nothing for a custom json file. We should really
> either be extending the desktop files to include an even more generic
> name that is translated such as:
> X-Unity-Generic-Action=Browse the web
>
> or creating new desktop files and installing them somewhere like
> /usr/share/unity/dash/home - using those to store the required
> information. I'd like a compelling reason to be using json over desktop
> files really.

Right, installing desktop files in a custom location to override attributes such as (translated) name, icon, etc. sounds like a good option.
The reason I allowed overriding those attributes directly in the JSON file was to minimize the work of whoever wants to customize the dash, especially in the context of OEM customizations: in most cases that’s only one additional file to ship.

> I'm not a fan of having the old code and this new code sit side-by-side,
> we should stick to one method and lose the other. Which means that we
> should be including a default json file in unity that gets installed to
> somewhere sane like /usr/share/unity/dash/.

That sounds like a very good idea, thanks for the suggestion.

> There is also an issue in that non of this is tested, thus it can't be
> approved until tests exist.

Right. At this point it’s unclear to me whether we actually want this in the trunk, since the (presumably) only target is Oneiric. The point of the exercise of submitting a merge request against the trunk was to have a formal review by someone competent. I’ll clarify this with involved parties.
Obviously tests would be very nice to have, but it may be beyond the scope of the patch at this point.

> small thing, but style wise, we shouldn't be using #define's, (the
> define that exists is from quite old code before we started using this
> style)

Thanks for pointing this out, I fixed it.

[Gord Allott (gordallott)]

> Right. At this point it’s unclear to me whether we actually want this in the trunk, since the (presumably) only target is Oneiric. The point of the exercise of submitting a merge request against the trunk was to have a formal review by someone competent. I’ll clarify this with involved parties.
> Obviously tests would be very nice to have, but it may be beyond the scope of the patch at this point.

So if we aren't targeting trunk you need to re-submit this against lp:unity/4.0 not lp:unity - small thing, but the auto lander will put this in trunk if we leave it against lp:unity

[Olivier Tilloy (osomon)]

Rejecting as this functionality should target oneiric (4.0), not trunk.

Superseded by https://code.launchpad.net/~osomon/unity/4.0-dash-custom-home-screen/+merge/85852.

review: Disapprove

Unmerged revisions

1741. By Olivier Tilloy on 2011-11-30

Cosmetics: replace a #define by a global variable in the unnamed namespace.

1740. By Olivier Tilloy on 2011-11-23

Allow customizing the dash’s home screen.

The contents of the custom home screen are described in a JSON file called HomeShortcutsCustomized.json.

The file is looked for in the following locations in decreasing order of priority:
    - $XDG_CONFIG_HOME/unity/ (defaults to $HOME/.config/unity/)
    - $DIR/unity/ for $DIR in $XDG_CONFIG_DIRS

The syntax of the file is as follows:
==========================================================================
{
  "shortcut1": {
    "source": $source,
    "name": $name,
    "name[fr]": $name_in_french,
    […]
    "icon": $icon
  },
  "shortcut2": {
    […]
  },
  […]
}
==========================================================================

The source attribute may either be a desktop file (full path or just its basename if it is located in a standard directory), or a lens file (basename only). This attribute is mandatory.
The 'name' attribute is optional. If present, it will override the default display name as advertised by the desktop file or by the lens. The name can be localized in several languages using the square brackets suffix notation, in which case the locale matching the system’s will be used, defaulting to the untranslated 'name' attribute if necessary.
The 'icon' attribute is optional. If present, it will override the default icon as advertised by the desktop file or by the lens. It should be a full path name.
If the source is a lens, the optional 'filter' attribute allows specifying a filter in the form "$name:$value", e.g. "type:videos" for the files lens.
If the source is a lens, the optional 'section' attribute allows specifying a section number (an integer starting at index 0).

=== modified file 'plugins/unityshell/src/JSONParser.cpp'
--- plugins/unityshell/src/JSONParser.cpp	2011-09-15 04:28:49 +0000
+++ plugins/unityshell/src/JSONParser.cpp	2011-11-30 10:25:11 +0000
@@ -145,6 +145,19 @@
     values[i] = json_array_get_double_element(array, i);
 }
 
+void Parser::ReadString(std::string const& node_name,
+                        std::string const& member_name,
+                        std::string& value) const
+{
+  JsonObject* object = GetNodeObject(node_name);
+
+  if (!object)
+    return;
+
+  if (json_object_has_member(object, member_name.c_str()))
+    value = json_object_get_string_member(object, member_name.c_str());
+}
+
 void Parser::ReadColor(std::string const& node_name,
                        std::string const& member_name,
                        std::string const& opacity_name,
 
=== modified file 'plugins/unityshell/src/JSONParser.h'
--- plugins/unityshell/src/JSONParser.h	2011-09-15 04:28:49 +0000
+++ plugins/unityshell/src/JSONParser.h	2011-11-30 10:25:11 +0000
@@ -58,6 +58,10 @@
                    std::string const& member_name,
                    std::vector<double>& values) const;
 
+  void ReadString(std::string const& node_name,
+                  std::string const& member_name,
+                  std::string& value) const;
+
   void ReadColor(std::string const& node_name,
                  std::string const& member_name,
                  std::string const& opacity_name,
 
=== modified file 'plugins/unityshell/src/PlacesHomeView.cpp'
--- plugins/unityshell/src/PlacesHomeView.cpp	2011-10-25 16:32:49 +0000
+++ plugins/unityshell/src/PlacesHomeView.cpp	2011-11-30 10:25:11 +0000
@@ -39,16 +39,25 @@
 
 #include "PlacesHomeView.h"
 #include "PlacesSimpleTile.h"
+#include "JSONParser.h"
 
 #include "DashStyle.h"
 #include <UnityCore/GLibWrapper.h>
 #include <UnityCore/Variant.h>
 
+#include <sstream>
 #include <string>
 #include <vector>
 
+#include <boost/algorithm/string.hpp>
+
 #define DELTA_DOUBLE_REQUEST 500000000
 
+namespace
+{
+  const std::string custom_shortcuts_file = "/unity/HomeShortcutsCustomized.json";
+}
+
 namespace unity
 {
 
@@ -135,7 +144,9 @@
 
   expanded.connect(sigc::mem_fun(this, &PlacesHomeView::Refresh));
 
-  Refresh();
+  // Wait for the asynchronous loading of the lenses to complete
+  // before refreshing the view for the first time.
+  _lenses.lenses_loaded.connect(sigc::bind(sigc::mem_fun(this, &PlacesHomeView::Refresh), (PlacesGroup*) NULL));
 }
 
 PlacesHomeView::~PlacesHomeView()
@@ -161,64 +172,227 @@
 
   _layout->Clear();
 
-  // Media Apps
-  markup = g_strdup_printf(temp, _("Media Apps"));
-  shortcut = new Shortcut(PKGDATADIR"/find_media_apps.png",
-                          markup,
-                          icon_size);
-  shortcut->_id = TYPE_PLACE;
-  shortcut->_place_id = g_strdup("applications.lens?filter_type=media");
-  shortcut->_place_section = 9;
-  _layout->AddView(shortcut, 1, nux::eLeft, nux::eFull);
-  shortcut->sigClick.connect(sigc::mem_fun(this, &PlacesHomeView::OnShortcutClicked));
-  g_free(markup);
-
-  // Internet Apps
-  markup = g_strdup_printf(temp, _("Internet Apps"));
-  shortcut = new Shortcut(PKGDATADIR"/find_internet_apps.png",
-                          markup,
-                          icon_size);
-  shortcut->_id = TYPE_PLACE;
-  shortcut->_place_id = g_strdup("applications.lens?filter_type=internet");
-  shortcut->_place_section = 8;
-  _layout->AddView(shortcut, 1, nux::eLeft, nux::eFull);
-  shortcut->sigClick.connect(sigc::mem_fun(this, &PlacesHomeView::OnShortcutClicked));
-  g_free(markup);
-
-  // More Apps
-  markup = g_strdup_printf(temp, _("More Apps"));
-  shortcut = new Shortcut(PKGDATADIR"/find_more_apps.png",
-                          markup,
-                          icon_size);
-  shortcut->_id = TYPE_PLACE;
-  shortcut->_place_id = g_strdup("applications.lens");
-  shortcut->_place_section = 0;
-  _layout->AddView(shortcut, 1, nux::eLeft, nux::eFull);
-  shortcut->sigClick.connect(sigc::mem_fun(this, &PlacesHomeView::OnShortcutClicked));
-  g_free(markup);
-
-  // Find Files
-  markup = g_strdup_printf(temp, _("Find Files"));
-  shortcut = new Shortcut(PKGDATADIR"/find_files.png",
-                          markup,
-                          icon_size);
-  shortcut->_id = TYPE_PLACE;
-  shortcut->_place_id = g_strdup("files.lens");
-  shortcut->_place_section = 0;
-  _layout->AddView(shortcut, 1, nux::eLeft, nux::eFull);
-  shortcut->sigClick.connect(sigc::mem_fun(this, &PlacesHomeView::OnShortcutClicked));
-  g_free(markup);
-
-  // Browser
-  CreateShortcutFromMime("x-scheme-handler/http", _("Browse the Web"), _browser_alternatives);
-
-  // Photos
-  // FIXME: Need to figure out the default
-  CreateShortcutFromExec("shotwell", _("View Photos"), _photo_alternatives);
-
-  CreateShortcutFromMime("x-scheme-handler/mailto", _("Check Email"), _email_alternatives);
-
-  CreateShortcutFromMime("audio/x-vorbis+ogg", _("Listen to Music"), _music_alternatives);
+  // The following code path allows one to replace the default dash home screen
+  // with a custom home screen, the contents of which are described in a JSON
+  // file. The file is looked for in the following locations in decreasing order
+  // of priority:
+  //   - $XDG_CONFIG_HOME/unity/ (defaults to $HOME/.config/unity/)
+  //   - $DIR/unity/ for $DIR in $XDG_CONFIG_DIRS
+  // The syntax of the file is as follows:
+  // ==========================================================================
+  // {
+  //   "shortcut1": {
+  //     "source": $source,
+  //     "name": $name,
+  //     "name[fr]": $name_in_french,
+  //     […]
+  //     "icon": $icon
+  //   },
+  //   "shortcut2": {
+  //     […]
+  //   },
+  //   […]
+  // }
+  // ==========================================================================
+  // The source attribute may either be a desktop file (full path or just its
+  // basename if it is located in a standard directory), or a lens file
+  // (basename only). This attribute is mandatory.
+  // The 'name' attribute is optional. If present, it will override the default
+  // display name as advertised by the desktop file or by the lens. The name can
+  // be localized in several languages using the square brackets suffix
+  // notation, in which case the locale matching the system’s will be used,
+  // defaulting to the untranslated 'name' attribute if necessary.
+  // The 'icon' attribute is optional. If present, it will override the default
+  // icon as advertised by the desktop file or by the lens. It should be a full
+  // path name.
+  // If the source is a lens, the optional 'filter' attribute allows specifying
+  // a filter in the form "$name:$value", e.g. "type:videos" for the files lens.
+  // If the source is a lens, the optional 'section' attribute allows specifying
+  // a section number (an integer starting at index 0).
+
+  std::string customShortcuts;
+  std::ostringstream file;
+  // Look for a custom shortcuts file in the user’s home directory first.
+  file << g_get_user_config_dir() << custom_shortcuts_file;
+  if (g_file_test(file.str().c_str(), G_FILE_TEST_EXISTS))
+  {
+    customShortcuts = file.str();
+  }
+  else
+  {
+    // Fall back on the standard XDG directories.
+    gchar** config = (gchar**) g_get_system_config_dirs();
+    for (gint i = 0; config[i]; ++i)
+    {
+      std::ostringstream file;
+      file << config[i] << custom_shortcuts_file;
+      if (g_file_test(file.str().c_str(), G_FILE_TEST_EXISTS))
+      {
+        customShortcuts = file.str();
+        break;
+      }
+    }
+  }
+  json::Parser parser;
+  if ((customShortcuts == "") || !parser.Open(customShortcuts))
+  {
+    // Media Apps
+    markup = g_strdup_printf(temp, _("Media Apps"));
+    shortcut = new Shortcut(PKGDATADIR"/find_media_apps.png",
+                            markup,
+                            icon_size);
+    shortcut->_id = TYPE_PLACE;
+    shortcut->_place_id = g_strdup("applications.lens?filter_type=media");
+    shortcut->_place_section = 9;
+    _layout->AddView(shortcut, 1, nux::eLeft, nux::eFull);
+    shortcut->sigClick.connect(sigc::mem_fun(this, &PlacesHomeView::OnShortcutClicked));
+    g_free(markup);
+
+    // Internet Apps
+    markup = g_strdup_printf(temp, _("Internet Apps"));
+    shortcut = new Shortcut(PKGDATADIR"/find_internet_apps.png",
+                            markup,
+                            icon_size);
+    shortcut->_id = TYPE_PLACE;
+    shortcut->_place_id = g_strdup("applications.lens?filter_type=internet");
+    shortcut->_place_section = 8;
+    _layout->AddView(shortcut, 1, nux::eLeft, nux::eFull);
+    shortcut->sigClick.connect(sigc::mem_fun(this, &PlacesHomeView::OnShortcutClicked));
+    g_free(markup);
+
+    // More Apps
+    markup = g_strdup_printf(temp, _("More Apps"));
+    shortcut = new Shortcut(PKGDATADIR"/find_more_apps.png",
+                            markup,
+                            icon_size);
+    shortcut->_id = TYPE_PLACE;
+    shortcut->_place_id = g_strdup("applications.lens");
+    shortcut->_place_section = 0;
+    _layout->AddView(shortcut, 1, nux::eLeft, nux::eFull);
+    shortcut->sigClick.connect(sigc::mem_fun(this, &PlacesHomeView::OnShortcutClicked));
+    g_free(markup);
+
+    // Find Files
+    markup = g_strdup_printf(temp, _("Find Files"));
+    shortcut = new Shortcut(PKGDATADIR"/find_files.png",
+                            markup,
+                            icon_size);
+    shortcut->_id = TYPE_PLACE;
+    shortcut->_place_id = g_strdup("files.lens");
+    shortcut->_place_section = 0;
+    _layout->AddView(shortcut, 1, nux::eLeft, nux::eFull);
+    shortcut->sigClick.connect(sigc::mem_fun(this, &PlacesHomeView::OnShortcutClicked));
+    g_free(markup);
+
+    // Browser
+    CreateShortcutFromMime("x-scheme-handler/http", _("Browse the Web"), _browser_alternatives);
+
+    // Photos
+    // FIXME: Need to figure out the default
+    CreateShortcutFromExec("shotwell", _("View Photos"), _photo_alternatives);
+
+    CreateShortcutFromMime("x-scheme-handler/mailto", _("Check Email"), _email_alternatives);
+
+    CreateShortcutFromMime("audio/x-vorbis+ogg", _("Listen to Music"), _music_alternatives);
+  }
+  else
+  {
+    // The custom shortcuts file can contain up to 8 shortcuts.
+    for (int i = 0; i < 8; ++i)
+    {
+      std::ostringstream oss;
+      oss << "shortcut" << (i + 1);
+      std::string slot = oss.str();
+      std::string source;
+      parser.ReadString(slot, "source", source);
+      if (source != "")
+      {
+        std::string icon;
+        parser.ReadString(slot, "icon", icon);
+
+        std::string name;
+        gchar** languages = (gchar**) g_get_language_names();
+        for (gint i = 0; languages[i]; ++i)
+        {
+          std::ostringstream key;
+          key << "name[" << languages[i] << "]";
+          parser.ReadString(slot, key.str(), name);
+          if (name != "")
+            break;
+        }
+        if (name == "")
+        {
+          parser.ReadString(slot, "name", name);
+        }
+
+        if (boost::iends_with(source, ".desktop"))
+        {
+          GDesktopAppInfo* info = NULL;
+          if (boost::istarts_with(source, "/"))
+            info = g_desktop_app_info_new_from_filename(source.c_str());
+          else
+            info = g_desktop_app_info_new(source.c_str());
+          if (G_IS_DESKTOP_APP_INFO(info))
+          {
+            if (name == "")
+              name = g_app_info_get_display_name(G_APP_INFO(info));
+            if (icon == "")
+            {
+              gchar* cicon = g_icon_to_string(g_app_info_get_icon(G_APP_INFO(info)));
+              icon = cicon;
+              g_free(cicon);
+            }
+            gchar* exec = g_strdup(g_app_info_get_executable(G_APP_INFO(info)));
+            markup = g_strdup_printf(temp, name.c_str());
+            shortcut = new Shortcut(icon.c_str(), markup, icon_size);
+            shortcut->_id = TYPE_EXEC;
+            shortcut->_exec = exec;
+            _layout->AddView(shortcut, 1, nux::eLeft, nux::eFull);
+            shortcut->sigClick.connect(sigc::mem_fun(this, &PlacesHomeView::OnShortcutClicked));
+            g_free(markup);
+            g_object_unref(info);
+          }
+        }
+        else if (boost::iends_with(source, ".lens"))
+        {
+          unity::dash::Lens::Ptr lens = _lenses.GetLens(source);
+          if (lens)
+          {
+            if (name == "")
+              name = lens->name;
+            if (icon == "")
+              icon = lens->icon_hint;
+          }
+          std::string filter;
+          parser.ReadString(slot, "filter", filter);
+          int section = -1;
+          parser.ReadInt(slot, "section", section);
+          markup = g_strdup_printf(temp, name.c_str());
+          shortcut = new Shortcut(icon.c_str(), markup, icon_size);
+          shortcut->_id = TYPE_PLACE;
+          if (filter != "")
+          {
+            std::vector<std::string> filter_key_value;
+            boost::split(filter_key_value, filter, boost::is_any_of(":"));
+            if (filter_key_value.size() == 2)
+              shortcut->_place_id = g_strdup_printf("%s?filter_%s=%s",
+                                                    source.c_str(),
+                                                    filter_key_value[0].c_str(),
+                                                    filter_key_value[1].c_str());
+            else
+              shortcut->_place_id = g_strdup(source.c_str());
+          }
+          else
+            shortcut->_place_id = g_strdup(source.c_str());
+          if (section != -1)
+            shortcut->_place_section = section;
+          _layout->AddView(shortcut, 1, nux::eLeft, nux::eFull);
+          shortcut->sigClick.connect(sigc::mem_fun(this, &PlacesHomeView::OnShortcutClicked));
+          g_free(markup);
+        }
+      }
+    }
+  }
 
   SetExpanded(true);
   SetCounts(8, 8);
 
=== modified file 'plugins/unityshell/src/PlacesHomeView.h'
--- plugins/unityshell/src/PlacesHomeView.h	2011-09-02 03:40:15 +0000
+++ plugins/unityshell/src/PlacesHomeView.h	2011-11-30 10:25:11 +0000
@@ -29,6 +29,8 @@
 
 #include <Nux/GridHLayout.h>
 
+#include <UnityCore/FilesystemLenses.h>
+
 #include "PlacesTile.h"
 #include "PlacesGroup.h"
 
@@ -67,6 +69,8 @@
   std::vector<std::string> _music_alternatives;
 
   guint _ubus_handle;
+
+  unity::dash::FilesystemLenses _lenses;
 };
 
 }