Merge lp:~didrocks/unity/fix-lens-shortcuts into lp:unity

[Didier Roche-Tolomelli]

bring back lenses shortcut activation. Seems there is still an issue on
registering keybinding on first super invocation, but good enough for
main cases (LP: #834078).

No python code was written during this hacking period.

[Neil J. Patel (njpatel)]

Awesome stuff! I found a couple of niggles, not sure if they are to do with your branch or not, so let me know:

- Pressing Super no longer shows the numbers on the icons :/
- If I press Super+f quickly, then the dash shows and hides, instead of just showing. I have to press super, wait around 100ms and then press f to make it show.

Any ideas?

review: Needs Fixing

[Mikkel Kamstrup Erlandsen (kamstrup)]

The missing numbers and letters when holding down super is already in
Oneiric, so definitely not from this branch.

[Tim Penhey (thumper)]

On 08/09/11 04:10, Didier Roche wrote:

+Lens::Ptr FilesystemLenses::Impl::GetLensForShortcut(std::string const&
lens_shortcut) const
+{
+ Lens::Ptr p;
+
+ for (Lens::Ptr lens: lenses_)
+ {
+ if (lens->shortcut == lens_shortcut)
+ {
+ p = lens;
+ break;
+ }
+ }
+
+ return p;
+}

In a previous branch I found some weirdness with break nor exiting the
new range based for loop. What I found fixed it was to return. So
something like this:

Lens::Ptr FilesystemLenses::Impl::GetLensForShortcut(std::string const&
lens_shortcut) const
{
   for (auto lens : lenses_)
   {
     if (lens->shortcut == lens_shortcut)
       return lens
   }
   return Lens::Ptr();
}

You could use "auto" or "Lens::Ptr const&". Using just Lens::Ptr causes
an extra object to be constructed and destroyed for each time through
the loop.

Tim

[Didier Roche-Tolomelli (didrocks)]

I've made the change mentionned by Tim in UnityCore, I've also fixed another one in UnityCore which had a similar pattern.

The "quick typing shortcut" is now fixed as well. But that makes me thing that we should have a keybinding controller which has a reference to both models to trigger the actions in both the Launcher and Dash, being able to know about both part and so avoiding the "show/hide" in a short period of time like we had there.
I initially put everything in the Launcher as it's where we controlled all the Super keys, but it's not the case anymore with this branch, so we should think about it. I'll get to it, but maybe after this release.

Please tell me if it fixes this :)
There is still the issue in first launch (will need to check compiz registration process with Sam)
The numbers not showing and acting is an oneiric regression, indeed, not related to this branch. I'll try to have a look there as well later (some people will apparently break some libunity ABI, and that + patch piloting + zomg new Qt! needs some attention from now ;))

[Neil J. Patel (njpatel)]

Sweet, approved!

review: Approve

=== modified file 'UnityCore/FilesystemLenses.cpp'
--- UnityCore/FilesystemLenses.cpp	2011-08-07 09:21:37 +0000
+++ UnityCore/FilesystemLenses.cpp	2011-09-08 06:47:28 +0000
@@ -114,6 +114,7 @@
   LensList GetLenses() const;
   Lens::Ptr GetLens(std::string const& lens_id) const;
   Lens::Ptr GetLensAtIndex(std::size_t index) const;
+  Lens::Ptr GetLensForShortcut(std::string const& lens_shortcut) const;
   std::size_t count() const;
 
   void Init();
@@ -334,18 +335,15 @@
 
 Lens::Ptr FilesystemLenses::Impl::GetLens(std::string const& lens_id) const
 {
-  Lens::Ptr p;
-
-  for (Lens::Ptr lens: lenses_)
+  for (auto lens: lenses_)
   {
     if (lens->id == lens_id)
     {
-      p = lens;
-      break;
+      return lens;
     }
   }
 
-  return p;
+  return Lens::Ptr();
 }
 
 Lens::Ptr FilesystemLenses::Impl::GetLensAtIndex(std::size_t index) const
@@ -361,6 +359,19 @@
   return Lens::Ptr();
 }
 
+Lens::Ptr FilesystemLenses::Impl::GetLensForShortcut(std::string const& lens_shortcut) const
+{
+  for (auto lens: lenses_)
+  {
+    if (lens->shortcut == lens_shortcut)
+    {
+      return lens;
+    }
+  }
+
+  return Lens::Ptr();
+}
+
 std::size_t FilesystemLenses::Impl::count() const
 {
   return lenses_.size();
@@ -404,5 +415,10 @@
   return pimpl->GetLensAtIndex(index);
 }
 
+Lens::Ptr FilesystemLenses::GetLensForShortcut(std::string const& lens_shortcut) const
+{
+  return pimpl->GetLensForShortcut(lens_shortcut);
+}
+
 }
 }
 
=== modified file 'UnityCore/FilesystemLenses.h'
--- UnityCore/FilesystemLenses.h	2011-07-27 17:35:31 +0000
+++ UnityCore/FilesystemLenses.h	2011-09-08 06:47:28 +0000
@@ -48,6 +48,7 @@
   LensList GetLenses() const;
   Lens::Ptr GetLens(std::string const& lens_id) const;
   Lens::Ptr GetLensAtIndex(std::size_t index) const;
+  Lens::Ptr GetLensForShortcut(std::string const& lens_shortcut) const;
 
   sigc::signal<void> lenses_loaded;
 
 
=== modified file 'plugins/unityshell/src/DashController.cpp'
--- plugins/unityshell/src/DashController.cpp	2011-09-08 04:49:34 +0000
+++ plugins/unityshell/src/DashController.cpp	2011-09-08 06:47:28 +0000
@@ -308,6 +308,24 @@
   ShowDash();
 }
 
+gboolean DashController::CheckShortcutActivation(const char* key_string)
+{
+  EnsureDash();
+  std::string lens_id = view_->GetIdForShortcutActivation(std::string(key_string));
+  if (lens_id != "")
+  {
+    OnActivateRequest(g_variant_new("(sus)", lens_id.c_str(), 0, ""));
+    return true;
+  }
+  return false;
+}
+
+std::vector<char> DashController::GetAllShortcuts()
+{
+  EnsureDash();
+  return view_->GetAllShortcuts();
+}
+
 // Introspectable
 const gchar* DashController::GetName()
 {
 
=== modified file 'plugins/unityshell/src/DashController.h'
--- plugins/unityshell/src/DashController.h	2011-09-08 04:49:34 +0000
+++ plugins/unityshell/src/DashController.h	2011-09-08 06:47:28 +0000
@@ -48,6 +48,9 @@
 
   nux::BaseWindow* window() const;
 
+  gboolean CheckShortcutActivation(const char* key_string);
+  std::vector<char> GetAllShortcuts();
+
   nux::Property<int> launcher_width;
   nux::Property<int> panel_height;
 
 
=== modified file 'plugins/unityshell/src/DashView.cpp'
--- plugins/unityshell/src/DashView.cpp	2011-09-08 03:04:04 +0000
+++ plugins/unityshell/src/DashView.cpp	2011-09-08 06:47:28 +0000
@@ -768,6 +768,27 @@
   return false;
 }
 
+std::string const DashView::GetIdForShortcutActivation(std::string const& shortcut) const
+{
+  Lens::Ptr lens = lenses_.GetLensForShortcut(shortcut);
+  if (lens)
+    return lens->id;
+  return "";
+}
+
+std::vector<char> DashView::GetAllShortcuts()
+{
+  std::vector<char> result;
+
+  for (Lens::Ptr lens: lenses_.GetLenses())
+  {
+    std::string shortcut = lens->shortcut;
+    if(shortcut.size() > 0)
+      result.push_back(shortcut.at(0));
+  }
+  return result;
+}
+
 bool DashView::InspectKeyEvent(unsigned int eventType,
                                unsigned int key_sym,
                                const char* character)
 
=== modified file 'plugins/unityshell/src/DashView.h'
--- plugins/unityshell/src/DashView.h	2011-09-07 09:50:44 +0000
+++ plugins/unityshell/src/DashView.h	2011-09-08 06:47:28 +0000
@@ -56,6 +56,9 @@
   void DisableBlur();
   void OnActivateRequest(GVariant* args);
 
+  std::string const GetIdForShortcutActivation(std::string const& shortcut) const;
+  std::vector<char> GetAllShortcuts();
+
   nux::View* default_focus() const;
 
 private:
 
=== modified file 'plugins/unityshell/src/Launcher.cpp'
--- plugins/unityshell/src/Launcher.cpp	2011-09-08 04:49:34 +0000
+++ plugins/unityshell/src/Launcher.cpp	2011-09-08 06:47:28 +0000
@@ -235,7 +235,6 @@
   _ignore_repeat_shortcut_handle = 0;
 
   _latest_shortcut        = 0;
-  _super_pressed          = false;
   _shortcuts_shown        = false;
   _floating               = false;
   _hovered                = false;
@@ -1366,7 +1365,6 @@
 
 void Launcher::StartKeyShowLauncher()
 {
-  _super_pressed = true;
   _hide_machine->SetQuirk(LauncherHideMachine::LAST_ACTION_ACTIVATE, false);
 
   SetTimeStruct(&_times[TIME_TAP_SUPER]);
@@ -1391,7 +1389,6 @@
   clock_gettime(CLOCK_MONOTONIC, &current);
 
   _hover_machine->SetQuirk(LauncherHoverMachine::SHORTCUT_KEYS_VISIBLE, false);
-  _super_pressed = false;
   _shortcuts_shown = false;
   QueueDraw();
 
@@ -2509,9 +2506,6 @@
                                     unsigned long key_state,
                                     char*         key_string)
 {
-  if (!_super_pressed)
-    return false;
-
   LauncherModel::iterator it;
 
   // Shortcut to start launcher icons. Only relies on Keycode, ignore modifier
@@ -2520,16 +2514,6 @@
     if ((XKeysymToKeycode(x_display, (*it)->GetShortcut()) == key_code) ||
         ((gchar)((*it)->GetShortcut()) == key_string[0]))
     {
-      /*
-       * start a timeout while repressing the same shortcut will be ignored.
-       * This is because the keypress repeat is handled by Xorg and we have no
-       * way to know if a press is an actual press or just an automated repetition
-       * because the button is hold down. (key release events are sent in both cases)
-      */
-      if (_ignore_repeat_shortcut_handle > 0)
-        g_source_remove(_ignore_repeat_shortcut_handle);
-      _ignore_repeat_shortcut_handle = g_timeout_add(IGNORE_REPEAT_SHORTCUT_DURATION, &Launcher::ResetRepeatShorcutTimeout, this);
-
       if (_latest_shortcut == (*it)->GetShortcut())
         return true;
 
@@ -2538,7 +2522,7 @@
       else
         (*it)->Activate(ActionArg(ActionArg::LAUNCHER, 0));
 
-      _latest_shortcut = (*it)->GetShortcut();
+      SetLatestShortcut((*it)->GetShortcut());
 
       // disable the "tap on super" check
       _times[TIME_TAP_SUPER].tv_sec = 0;
@@ -2550,6 +2534,20 @@
   return false;
 }
 
+void Launcher::SetLatestShortcut(guint64 shortcut)
+{
+  _latest_shortcut = shortcut;
+  /*
+   * start a timeout while repressing the same shortcut will be ignored.
+   * This is because the keypress repeat is handled by Xorg and we have no
+   * way to know if a press is an actual press or just an automated repetition
+   * because the button is hold down. (key release events are sent in both cases)
+   */
+  if (_ignore_repeat_shortcut_handle > 0)
+    g_source_remove(_ignore_repeat_shortcut_handle);
+  _ignore_repeat_shortcut_handle = g_timeout_add(IGNORE_REPEAT_SHORTCUT_DURATION, &Launcher::ResetRepeatShorcutTimeout, this);
+}
+
 void
 Launcher::EdgeRevealTriggered(int mouse_x, int mouse_y)
 {
 
=== modified file 'plugins/unityshell/src/Launcher.h'
--- plugins/unityshell/src/Launcher.h	2011-09-08 04:49:34 +0000
+++ plugins/unityshell/src/Launcher.h	2011-09-08 06:47:28 +0000
@@ -156,6 +156,7 @@
   void EdgeRevealTriggered(int x, int y);
 
   gboolean CheckSuperShortcutPressed(Display *x_display, unsigned int key_sym, unsigned long key_code, unsigned long key_state, char* key_string);
+  void SetLatestShortcut(guint64 shortcut);
 
   nux::BaseWindow* GetParent()
   {
@@ -397,7 +398,6 @@
   bool  _check_window_over_launcher;
 
   bool          _shortcuts_shown;
-  bool          _super_pressed;
   bool          _keynav_activated;
   guint64       _latest_shortcut;
 
 
=== modified file 'plugins/unityshell/src/unityshell.cpp'
--- plugins/unityshell/src/unityshell.cpp	2011-09-08 03:04:04 +0000
+++ plugins/unityshell/src/unityshell.cpp	2011-09-08 06:47:28 +0000
@@ -225,6 +225,7 @@
                                                    this);
 
   g_timeout_add(0, &UnityScreen::initPluginActions, this);
+  super_keypressed_ = false;
 
   GeisAdapter::Default()->Run();
   gestureEngine = new GestureEngine(screen);
@@ -237,8 +238,6 @@
   ubus_manager_.RegisterInterest(UBUS_PLACE_VIEW_SHOWN, [&](GVariant * args) { dash_is_open_ = true; });
   ubus_manager_.RegisterInterest(UBUS_PLACE_VIEW_HIDDEN, [&](GVariant * args) { dash_is_open_ = false; });
 
-  EnsureKeybindings();
-
   LOG_INFO(logger) << "UnityScreen constructed: " << timer.ElapsedSeconds() << "s";
 }
 
@@ -301,7 +300,7 @@
 
 }
 
-void UnityScreen::EnsureKeybindings()
+void UnityScreen::EnsureSuperKeybindings()
 {
   for (auto action : _shortcut_actions)
     screen->removeAction(action.get());
@@ -313,19 +312,26 @@
     guint64 shortcut = icon->GetShortcut();
     if (shortcut == 0)
       continue;
-
+    CreateSuperNewAction(static_cast<char>(shortcut));
+  }
+
+  for (auto shortcut : dashController->GetAllShortcuts())
+    CreateSuperNewAction(shortcut);
+}
+
+void UnityScreen::CreateSuperNewAction(char shortcut)
+{
     CompActionPtr action(new CompAction());
 
     CompAction::KeyBinding binding;
     std::ostringstream sout;
-    sout << "<Super>" << static_cast<char>(shortcut);
+    sout << "<Super>" << shortcut;
     binding.fromString(sout.str());
 
     action->setKey(binding);
 
     screen->addAction(action.get());
     _shortcut_actions.push_back(action);
-  }
 }
 
 void UnityScreen::nuxPrologue()
@@ -871,7 +877,14 @@
       if ((result = XLookupString(&(event->xkey), key_string, 2, &key_sym, 0)) > 0)
       {
         key_string[result] = 0;
-        skip_other_plugins = launcher->CheckSuperShortcutPressed(screen->dpy(), key_sym, event->xkey.keycode, event->xkey.state, key_string);
+        if (super_keypressed_) {
+          skip_other_plugins = launcher->CheckSuperShortcutPressed(screen->dpy(), key_sym, event->xkey.keycode, event->xkey.state, key_string);
+          if (!skip_other_plugins) {
+            skip_other_plugins = dashController->CheckShortcutActivation(key_string);
+            if (skip_other_plugins)
+              launcher->SetLatestShortcut(key_string[0]);
+          }
+        }
       }
       break;
   }
@@ -907,8 +920,9 @@
   if (state & CompAction::StateInitKey)
     action->setState(action->state() | CompAction::StateTermKey);
 
+  super_keypressed_ = true;
   launcher->StartKeyShowLauncher();
-  EnsureKeybindings ();
+  EnsureSuperKeybindings ();
   return false;
 }
 
@@ -916,6 +930,7 @@
                                            CompAction::State state,
                                            CompOption::Vector& options)
 {
+  super_keypressed_ = false;
   launcher->EndKeyShowLauncher();
   return false;
 }
 
=== modified file 'plugins/unityshell/src/unityshell.h'
--- plugins/unityshell/src/unityshell.h	2011-09-08 03:04:04 +0000
+++ plugins/unityshell/src/unityshell.h	2011-09-08 06:47:28 +0000
@@ -235,7 +235,8 @@
 
   void SendExecuteCommand();
 
-  void EnsureKeybindings ();
+  void EnsureSuperKeybindings ();
+  void CreateSuperNewAction(char shortcut);
 
   static gboolean initPluginActions(gpointer data);
   static void initLauncher(nux::NThread* thread, void* InitData);
@@ -281,6 +282,7 @@
   typedef std::shared_ptr<CompAction> CompActionPtr;
   typedef std::vector<CompActionPtr> ShortcutActions;
   ShortcutActions _shortcut_actions;
+  bool            super_keypressed_;
 
   /* keyboard-nav mode */
   CompWindow* newFocusedWindow;