Merge lp:~azzar1/unity/fix-971332 into lp:unity

[Andrea Azzarone]

Use gtk_accelerator_get_label to get a translatable key binding.

== The problem ==
Overlay key bindings are not fully translatable.

== The fix ==
Use gtk_accelerator_get_label to get a translatable key binding. The goal is to increase the use of GtkAccelerator in the shortcut overlay.

== Test ==
The shortcut overlay has already tests. No need to test gtk_accelerator_get_label.

[Marco Trevisan (Treviño) (3v1n0)]

41 + glib::String key(gtk_accelerator_get_label(accelerator_key, accelerator_mods));
42 +
43 + std::string temp(key.Str());
44 + if (temp.empty())
45 + return "";

This could just be:

std::string key(glib::String(gtk_accelerator_get_label(accelerator_key, accelerator_mods)).Str())

or... use std::string const& temp = key.Str()

[Tim Penhey (thumper)]

On 04/04/12 09:58, Marco Trevisan (Treviño) wrote:
> 41 + glib::String key(gtk_accelerator_get_label(accelerator_key, accelerator_mods));
> 42 +
> 43 + std::string temp(key.Str());
> 44 + if (temp.empty())
> 45 + return "";
>
> This could just be:
>
> std::string key(glib::String(gtk_accelerator_get_label(accelerator_key, accelerator_mods)).Str())
>
> or... use std::string const& temp = key.Str()

No, don't do that last one.

key.Str() creates an object.
   std::string const& temp = key.Str();
Creates a reference to a temporary. At the best, the compiler will
complain, at the worst, your program will crash when you try to access temp.

[Tim Penhey (thumper)]

> std::string ret(temp.begin(), temp.end() - 1);

What are you trying to do here?

> if (scut[scut.size() - 1] != '+')
> ret += scut[scut.size() - 1];

And what is happening here?

review: Needs Information

[Andrea Azzarone (azzar1)]

> > std::string ret(temp.begin(), temp.end() - 1);
>
> What are you trying to do here?
>
> > if (scut[scut.size() - 1] != '+')
> > ret += scut[scut.size() - 1];
>
> And what is happening here?

gtk_accelerator_get_label adds an extra '+'. Also it doesn't add spaces around the '+'.

[Andrea Azzarone (azzar1)]

Now it should be clearer.

[Gord Allott (gordallott)]

review: Approve

=== modified file 'plugins/unityshell/src/ShortcutHint.cpp'
--- plugins/unityshell/src/ShortcutHint.cpp	2012-03-14 06:24:18 +0000
+++ plugins/unityshell/src/ShortcutHint.cpp	2012-04-04 15:51:42 +0000
@@ -103,7 +103,7 @@
       {
           if (opt.name() == arg2())
           {
-            std::string temp = impl::FixShortcutFormat(opt.value().action().keyToString());
+            std::string temp(impl::GetTranslatableLabel(opt.value().action().keyToString()));
             temp = impl::ProperCase(temp);
             
             if (value() != temp)

=== modified file 'plugins/unityshell/src/ShortcutHintPrivate.cpp'
--- plugins/unityshell/src/ShortcutHintPrivate.cpp	2012-03-14 06:24:18 +0000
+++ plugins/unityshell/src/ShortcutHintPrivate.cpp	2012-04-04 15:51:42 +0000
@@ -17,8 +17,10 @@
  */
 
 #include <glib/gi18n-lib.h>
+#include <gtk/gtk.h>
 
 #include "ShortcutHintPrivate.h"
+#include "UnityCore/GLibWrapper.h"
 
 #include <boost/algorithm/string/replace.hpp>
 
@@ -51,6 +53,31 @@
   return ret;
 }
 
+std::string GetTranslatableLabel(std::string const& scut)
+{
+  guint accelerator_key;
+  GdkModifierType accelerator_mods;
+
+  gtk_accelerator_parse(scut.c_str(),
+                        &accelerator_key,
+                        &accelerator_mods);
+
+  std::string temp(glib::String(gtk_accelerator_get_label(accelerator_key, accelerator_mods)).Str());
+
+  // gtk_accelerator_get_label adds an extra '+' at the end of the label.
+  if (temp.length() > 0)
+  {
+    std::string::iterator it = temp.end() - 1;
+    if (*it == '+')
+      temp.erase(it);
+  }
+
+  // Adds an extra space around the '+'.
+  boost::replace_all(temp, "+", " + ");
+    
+  return temp;
+}
+
 std::string FixMouseShortcut(std::string const& scut)
 {
   std::string ret(scut);

=== modified file 'plugins/unityshell/src/ShortcutHintPrivate.h'
--- plugins/unityshell/src/ShortcutHintPrivate.h	2012-03-14 06:24:18 +0000
+++ plugins/unityshell/src/ShortcutHintPrivate.h	2012-04-04 15:51:42 +0000
@@ -30,6 +30,7 @@
 
 std::string GetMetaKey(std::string const& scut);
 std::string FixShortcutFormat(std::string const& scut);
+std::string GetTranslatableLabel(std::string const& scut);
 std::string FixMouseShortcut(std::string const& scut);
 std::string ProperCase(std::string const& str);
 

=== modified file 'plugins/unityshell/src/ShortcutView.cpp'
--- plugins/unityshell/src/ShortcutView.cpp	2012-03-29 05:14:03 +0000
+++ plugins/unityshell/src/ShortcutView.cpp	2012-04-04 15:51:42 +0000
@@ -38,7 +38,29 @@
   int SHORTKEY_COLUMN_WIDTH = 150;
   int DESCRIPTION_COLUMN_WIDTH = 265;
   int LINE_SPACING = 5;
-} // namespace anonymouse
+
+  // We need this class because SetVisible doesn't work for layouts.
+  class SectionView : public nux::View
+  {
+    public:
+      SectionView(NUX_FILE_LINE_DECL)
+        : nux::View(NUX_FILE_LINE_PARAM)
+      {
+      }
+
+    protected:
+      void Draw(nux::GraphicsEngine& graphics_engine, bool force_draw)
+      {
+      }
+
+      void DrawContent(nux::GraphicsEngine& graphics_engine, bool force_draw)
+      {
+        if (GetLayout())
+          GetLayout()->ProcessDraw(graphics_engine, force_draw);
+      }
+  };
+
+} // unnamed namespace
 
 NUX_IMPLEMENT_OBJECT_TYPE(View);
 
@@ -109,9 +131,13 @@
   return layout;
 }
 
-nux::LinearLayout* View::CreateShortKeyEntryLayout(AbstractHint::Ptr const& hint)
+nux::View* View::CreateShortKeyEntryView(AbstractHint::Ptr const& hint)
 {
+  nux::View* view = new SectionView(NUX_TRACKER_LOCATION);
+
   nux::HLayout* layout = new nux::HLayout("EntryLayout", NUX_TRACKER_LOCATION);
+  view->SetLayout(layout);
+
   nux::HLayout* shortkey_layout = new nux::HLayout(NUX_TRACKER_LOCATION);
   nux::HLayout* description_layout = new nux::HLayout(NUX_TRACKER_LOCATION);
 
@@ -152,17 +178,18 @@
   layout->SetSpaceBetweenChildren(INTER_SPACE_SHORTKEY_DESCRIPTION);
   description_layout->SetContentDistribution(nux::MAJOR_POSITION_START);
 
-   auto on_shortkey_changed = [](std::string const& new_shortkey, nux::StaticText* view) {
-      std::string skey = "<b>";
+   auto on_shortkey_changed = [](std::string const& new_shortkey, nux::View* main_view, nux::StaticText* view) {
+      std::string skey("<b>");
       skey += new_shortkey;
       skey += "</b>";
 
       view->SetText(skey);
+      main_view->SetVisible(!new_shortkey.empty());
    };
 
-  hint->shortkey.changed.connect(sigc::bind(sigc::slot<void, std::string const&, nux::StaticText*>(on_shortkey_changed), shortkey_view));
+  hint->shortkey.changed.connect(sigc::bind(sigc::slot<void, std::string const&, nux::View*, nux::StaticText*>(on_shortkey_changed), view, shortkey_view));
 
-  return layout;
+  return view;
 }
 
 nux::LinearLayout* View::CreateIntermediateLayout()
@@ -208,12 +235,9 @@
 
     for (auto hint : model_->hints()[category])
     {
-      //std::string str_value = hint->prefix() + hint->value() + hint->postfix();
-      //boost::replace_all(str_value, "&", "&amp;");
-      //boost::replace_all(str_value, "<", "&lt;");
-      //boost::replace_all(str_value, ">", "&gt;");
-
-      intermediate_layout->AddLayout(CreateShortKeyEntryLayout(hint), 0, nux::MINOR_POSITION_START, nux::MINOR_SIZE_FULL);
+      nux::View* view = CreateShortKeyEntryView(hint);
+      view->SetVisible(!hint->shortkey().empty());
+      intermediate_layout->AddView(view, 0, nux::MINOR_POSITION_START, nux::MINOR_SIZE_FULL);
     }
 
     section_layout->AddLayout(intermediate_layout, 0, nux::MINOR_POSITION_CENTER, nux::MINOR_SIZE_FULL);

=== modified file 'plugins/unityshell/src/ShortcutView.h'
--- plugins/unityshell/src/ShortcutView.h	2012-03-29 05:14:03 +0000
+++ plugins/unityshell/src/ShortcutView.h	2012-04-04 15:51:42 +0000
@@ -59,7 +59,7 @@
 private:
   // Private methods
   nux::LinearLayout* CreateSectionLayout(const char* section_name);
-  nux::LinearLayout* CreateShortKeyEntryLayout(AbstractHint::Ptr const& hint);
+  nux::View* CreateShortKeyEntryView(AbstractHint::Ptr const& hint);
   nux::LinearLayout* CreateIntermediateLayout();
 
   void RenderColumns();