Merge lp:~osomon/unity-2d/launcher-items-keyboard-shortcuts into lp:unity-2d/3.0

[Olivier Tilloy]

[launcher] Keyboard shortcuts for place entries (Meta+[letter]) and the trash (Meta+T).

[lp:~osomon/unity-2d/launcher-items-keyboard-shortcuts]

559. By Olivier Tilloy on 2011-05-03

No need to manually disconnect from the hotkey event, this is done automatically by Qt upon destruction.

[Ugo Riboni (uriboni)]

Functionally there's only one corner case where it doesn't work: if the shortcut key is some accented character, it's not displayed as a shortcut. For example if you edit /usr/share/unity/places/applications.place and make the shortcut key "é", it will not work (the key is not even grabbed).

review: Needs Fixing

[Ugo Riboni (uriboni)]

134 + if (value.size() == 1) {
135 + QChar c = value.at(0);
136 + if (c.isLetter()) {
137 + Qt::Key key = (Qt::Key) (Qt::Key_A + (c.toLower().toAscii() - 'a'));
138 + entry->setShortcutKey(key);
139 + }
140 + }

I am a fan of the principle "be strict in what you emit and relaxed in what you accept", and in this case if the string length is larger than 1 character, why don't you just pick the first character and emit a warning.
Or if you prefer just emit a warning and set no shortcut. But don't just silently do nothing.

review: Needs Fixing

[lp:~osomon/unity-2d/launcher-items-keyboard-shortcuts]

560. By Olivier Tilloy on 2011-05-03

Print out a warning if the shortcut key is invalid (more than one single character).

561. By Olivier Tilloy on 2011-05-03

More reliable (and simple) conversion from QString to Qt::Key.

[Ugo Riboni (uriboni)]

It's still not working. I set the shortcut to "é" (one single accented character), and I get this warning message: unity-2d-launcher: [WARNING] void Place::setFileName(const QString&): Invalid shorcut key (should be one single character): "é"

review: Needs Fixing

[Olivier Tilloy (osomon)]

This is very likely because your text editor, when you are typing "é", is in fact inserting two characters (0xc3 + 0xa9) instead of one (0xc9). The visual representation is the same, but the underlying unicode representation isn’t, and as a consequence the string extracted from the place file is too long. At least the code now prints out a warning, it doesn’t ignore it silently.

Still, if you use an hexadecimal editor to set the shortcut to the correct unicode character (0xc9), grabbing the key fails lower down the stack with the following warning:

    Could not convert "É" to an x11 keysym

This happens during the conversion from a Qt::Key to an X11 keysym in the constructor for Hotkey. I’m trying to figure out how to fix the conversion.

[lp:~osomon/unity-2d/launcher-items-keyboard-shortcuts]

562. By Olivier Tilloy on 2011-05-03

Fix the translation from Qt::Key to an X11 KeySym for unicode characters such as "É".

563. By Olivier Tilloy on 2011-05-03

Comment to explain why setting the shortcut key of a place entry to an accentuated character might fail.

=== modified file 'launcher/LauncherList.qml'
--- launcher/LauncherList.qml	2011-04-29 16:11:04 +0000
+++ launcher/LauncherList.qml	2011-05-03 15:41:14 +0000
@@ -98,9 +98,16 @@
         emblem: (noOverlays && item.emblem) ? "image://icons/" + item.emblem : ""
         emblemVisible: (noOverlays) ? false : item.emblemVisible
 
-        shortcutVisible: item.toString().indexOf("LauncherApplication") == 0 &&
-                         index <= 9 && launcherView.superKeyHeld
-        shortcutText: (index + 1) % 10
+        shortcutVisible: launcherView.superKeyHeld &&
+                         ((item.toString().indexOf("LauncherApplication") == 0 && index <= 9) ||
+                          item.shortcutKey != 0)
+        shortcutText: {
+            if (item.toString().indexOf("LauncherApplication") == 0) {
+                return (index + 1) % 10
+            } else {
+                return String.fromCharCode(item.shortcutKey).toLowerCase()
+            }
+        }
 
         isBeingDragged: (reorder.draggedTileId != "") && (reorder.draggedTileId == desktopFile)
         dragPosition: reorder.listCoordinates.y - list.contentY

=== modified file 'launcher/UnityApplications/launcheritem.cpp'
--- launcher/UnityApplications/launcheritem.cpp	2011-03-31 08:12:28 +0000
+++ launcher/UnityApplications/launcheritem.cpp	2011-05-03 15:41:14 +0000
@@ -20,7 +20,13 @@
 #include "launcheritem.h"
 #include "launchermenu.h"
 
-LauncherItem::LauncherItem(QObject* parent): QObject(parent)
+// libunity-2d
+#include <hotkey.h>
+#include <hotkeymonitor.h>
+
+LauncherItem::LauncherItem(QObject* parent)
+    : QObject(parent)
+    , m_shortcutKey((Qt::Key) 0)
 {
     m_menu = new LauncherContextualMenu;
     m_menu->setLauncherItem(this);
@@ -31,6 +37,29 @@
     delete m_menu;
 }
 
+Qt::Key
+LauncherItem::shortcutKey() const
+{
+    return m_shortcutKey;
+}
+
+void
+LauncherItem::setShortcutKey(Qt::Key key)
+{
+    if (m_shortcutKey != key) {
+        if (m_shortcutKey != 0) {
+            Hotkey* hotkey = HotkeyMonitor::instance().getHotkeyFor(m_shortcutKey, Qt::MetaModifier);
+            disconnect(hotkey, SIGNAL(pressed()), this, SLOT(activate()));
+        }
+        m_shortcutKey = key;
+        if (m_shortcutKey != 0) {
+            Hotkey* hotkey = HotkeyMonitor::instance().getHotkeyFor(m_shortcutKey, Qt::MetaModifier);
+            connect(hotkey, SIGNAL(pressed()), SLOT(activate()));
+        }
+        Q_EMIT shortcutKeyChanged(m_shortcutKey);
+    }
+}
+
 QObject*
 LauncherItem::menu() const
 {

=== modified file 'launcher/UnityApplications/launcheritem.h'
--- launcher/UnityApplications/launcheritem.h	2011-03-31 08:12:28 +0000
+++ launcher/UnityApplications/launcheritem.h	2011-05-03 15:41:14 +0000
@@ -22,6 +22,7 @@
 
 #include <QObject>
 #include <QString>
+#include <Qt>
 
 #include "dragdropevent.h"
 
@@ -38,6 +39,7 @@
     Q_PROPERTY(QString name READ name NOTIFY nameChanged)
     Q_PROPERTY(QString icon READ icon NOTIFY iconChanged)
     Q_PROPERTY(bool launching READ launching NOTIFY launchingChanged)
+    Q_PROPERTY(Qt::Key shortcutKey READ shortcutKey WRITE setShortcutKey NOTIFY shortcutKeyChanged)
     /* Export the menu as a plain QObject so that it can be used from QML */
     Q_PROPERTY(QObject* menu READ menu NOTIFY menuChanged)
 
@@ -53,8 +55,12 @@
     virtual QString name() const = 0;
     virtual QString icon() const = 0;
     virtual bool launching() const = 0;
+    Qt::Key shortcutKey() const;
     QObject* menu() const;
 
+    /* setters */
+    void setShortcutKey(Qt::Key);
+
     /* methods */
     Q_INVOKABLE virtual void activate() = 0;
     Q_INVOKABLE virtual void createMenuActions() = 0;
@@ -71,6 +77,7 @@
     void nameChanged(QString);
     void iconChanged(QString);
     void launchingChanged(bool);
+    void shortcutKeyChanged(Qt::Key);
     void menuChanged(QObject*);
 
 public Q_SLOTS:
@@ -78,6 +85,9 @@
        subclasses to implement custom behaviours. */
     virtual void onDragEnter(DeclarativeDragDropEvent*);
     virtual void onDrop(DeclarativeDragDropEvent*);
+
+private:
+    Qt::Key m_shortcutKey;
 };
 
 #endif // LAUNCHERITEM_H

=== modified file 'launcher/UnityApplications/place.cpp'
--- launcher/UnityApplications/place.cpp	2011-04-28 13:09:25 +0000
+++ launcher/UnityApplications/place.cpp	2011-05-03 15:41:14 +0000
@@ -106,6 +106,21 @@
             entry->setIcon(m_file->value("Icon").toString());
             entry->setSearchHint(u2dTr(m_file->value("SearchHint").toString().toUtf8().constData(),
                                  gettextDomain.toUtf8().constData()));
+            if (m_file->contains("Shortcut")) {
+                QString value = m_file->value("Shortcut").toString();
+                if (value.size() == 1) {
+                    Qt::Key key = (Qt::Key) value.at(0).toUpper().unicode();
+                    entry->setShortcutKey(key);
+                } else {
+                    /* Note: some text editors insert the decomposed form of
+                       e.g. accented characters (e.g. 0xc3 + 0xa9 for "É"
+                       instead of the canonical form 0xc9). Unfortunately Qt
+                       doesn’t seem to be able to perform composition, so in
+                       such cases setting the shortcut key fails. See
+                       http://www.unicode.org/reports/tr15/ for details. */
+                    UQ_WARNING << "Invalid shorcut key (should be one single character):" << value;
+                }
+            }
             if (!m_file->contains("ShowEntry")) {
                 entry->setShowEntry(true);
             } else {

=== modified file 'launcher/UnityApplications/placeentry.cpp'
--- launcher/UnityApplications/placeentry.cpp	2011-04-07 09:48:24 +0000
+++ launcher/UnityApplications/placeentry.cpp	2011-05-03 15:41:14 +0000
@@ -21,6 +21,7 @@
 #include "place.h"
 #include "launchermenu.h"
 
+// libunity-2d
 #include <debug_p.h>
 
 #include <QDBusMetaType>

=== modified file 'launcher/UnityApplications/trash.cpp'
--- launcher/UnityApplications/trash.cpp	2011-04-28 13:09:25 +0000
+++ launcher/UnityApplications/trash.cpp	2011-05-03 15:41:14 +0000
@@ -33,6 +33,7 @@
 Trash::Trash()
 {
     m_trash = g_file_new_for_uri(TRASH_URI);
+    setShortcutKey(Qt::Key_T);
 }
 
 Trash::Trash(const Trash& other)

=== modified file 'libunity-2d-private/src/hotkey.cpp'
--- libunity-2d-private/src/hotkey.cpp	2011-04-28 13:09:25 +0000
+++ libunity-2d-private/src/hotkey.cpp	2011-05-03 15:41:14 +0000
@@ -72,13 +72,16 @@
     QString keyString = QKeySequence(key).toString();
     KeySym keysym = XStringToKeysym(keyString.toLatin1().data());
     if (keysym == NoSymbol) {
-        UQ_WARNING << "Could not convert" << keyString << "to an x11 keysym";
-    } else {
-        m_x11key = XKeysymToKeycode(QX11Info::display(), keysym);
-        if (m_x11key == 0) {
-            UQ_WARNING << "Could not get keycode for keysym" << keysym
-                       << "(" << keyString << ")";
-        }
+        /* XStringToKeysym doesn’t work well with exotic characters (such as
+          'É'). Note that this fallback code path looks much simpler but doesn’t
+          work for special keys such as the function keys (e.g. F1), which is
+          why the translation with XStringToKeysym is attempted first. */
+        keysym = (ushort) key;
+    }
+    m_x11key = XKeysymToKeycode(QX11Info::display(), keysym);
+    if (m_x11key == 0) {
+        UQ_WARNING << "Could not get keycode for keysym" << keysym
+                   << "(" << keyString << ")";
     }
 }