Convert AppShortcut in a ui::Shortcut subclass

src/app/app_menus.cpp

@@ -285,7 +285,7 @@ void destroy_menu_item(ui::Widget* item)
 os::Shortcut get_os_shortcut_from_key(const Key* key)
 {
   if (key && !key->shortcuts().empty()) {
-    const ui::Shortcut& shortcut = key->shortcuts().front().shortcut;
+    const ui::Shortcut& shortcut = key->shortcuts().front();
 
 #if LAF_MACOS
     // Shortcuts with spacebar as modifier do not work well in macOS

src/app/commands/cmd_keyboard_shortcuts.cpp

@@ -207,7 +207,7 @@ private:
   void onChangeShortcut(int index)
   {
     LockButtons lock(this);
-    Shortcut origShortcut = m_key->shortcuts()[index].shortcut;
+    Shortcut origShortcut = m_key->shortcuts()[index];
     SelectShortcut window(origShortcut, m_key->keycontext(), m_keys);
     window.openWindowInForeground();
 
@@ -225,7 +225,7 @@ private:
     LockButtons lock(this);
     // We need to create a copy of the shortcut because
     // Key::disableShortcut() will modify the shortcuts() collection itself.
-    Shortcut shortcut = m_key->shortcuts()[index].shortcut;
+    Shortcut shortcut = m_key->shortcuts()[index];
 
     if (ui::Alert::show(Strings::alerts_delete_shortcut(shortcut.toString())) != 1)
       return;

src/app/ui/key.cpp

@@ -208,7 +208,7 @@ void erase_shortcut(app::AppShortcuts& kvs,
 {
   for (auto it = kvs.begin(); it != kvs.end();) {
     auto& kv = *it;
-    if (kv.source == source && kv.shortcut == shortcut)
+    if (kv.source() == source && kv.shortcut() == shortcut)
       it = kvs.erase(it);
     else
       ++it;
@@ -219,7 +219,7 @@ void erase_shortcuts(app::AppShortcuts& kvs, const app::KeySource source)
 {
   for (auto it = kvs.begin(); it != kvs.end();) {
     auto& kv = *it;
-    if (kv.source == source)
+    if (kv.source() == source)
       it = kvs.erase(it);
     else
       ++it;
@@ -301,15 +301,15 @@ bool AppShortcut::fitsBetterThan(const KeyContext currentContext,
                                  const AppShortcut& otherShortcut) const
 {
   // Better context in the same source
-  if (otherShortcut.source == this->source && otherShortcutContext != currentContext &&
+  if (otherShortcut.source() == this->source() && otherShortcutContext != currentContext &&
       thisShortcutContext == currentContext)
     return true;
 
   // Better key source/level: User-defined > Extension-defined > App-defined
-  if (int(this->source) > int(otherShortcut.source) && (thisShortcutContext == currentContext ||
+  if (int(source()) > int(otherShortcut.source()) && (thisShortcutContext == currentContext ||
-                                                        // User-defined "Any" context overwrites all
+                                                      // User-defined "Any" context overwrites all
-                                                        // app-defined context
+                                                      // app-defined context
-                                                        thisShortcutContext == KeyContext::Any))
+                                                      thisShortcutContext == KeyContext::Any))
     return true;
 
   // Normal > SelectionTool > Transformation
@@ -420,30 +420,30 @@ const AppShortcuts& Key::shortcuts() const
 
     // Add default keys
     for (const auto& kv : m_adds) {
-      if (kv.source == KeySource::Original)
+      if (kv.source() == KeySource::Original)
         m_shortcuts->add(kv);
     }
 
     // Delete/add extension-defined keys
     for (const auto& kv : m_dels) {
-      if (kv.source == KeySource::ExtensionDefined)
+      if (kv.source() == KeySource::ExtensionDefined)
         m_shortcuts->remove(kv);
       else {
-        ASSERT(kv.source != KeySource::Original);
+        ASSERT(kv.source() != KeySource::Original);
       }
     }
     for (const auto& kv : m_adds) {
-      if (kv.source == KeySource::ExtensionDefined)
+      if (kv.source() == KeySource::ExtensionDefined)
         m_shortcuts->add(kv);
     }
 
     // Delete/add user-defined keys
     for (const auto& kv : m_dels) {
-      if (kv.source == KeySource::UserDefined)
+      if (kv.source() == KeySource::UserDefined)
         m_shortcuts->remove(kv);
     }
     for (const auto& kv : m_adds) {
-      if (kv.source == KeySource::UserDefined)
+      if (kv.source() == KeySource::UserDefined)
         m_shortcuts->add(kv);
     }
   }
@@ -493,9 +493,7 @@ const AppShortcut* Key::isPressed(const Message* msg, const KeyContext keyContex
   if (const auto* keyMsg = dynamic_cast<const KeyMessage*>(msg)) {
     if (fitsContext(keyContext)) {
       for (const AppShortcut& shortcut : shortcuts()) {
-        if (shortcut.shortcut.isPressed(keyMsg->modifiers(),
+        if (shortcut.isPressed(keyMsg->modifiers(), keyMsg->scancode(), keyMsg->unicodeChar()) &&
-                                        keyMsg->scancode(),
-                                        keyMsg->unicodeChar()) &&
             (!best || shortcut.fitsBetterThan(keyContext, keycontext(), keycontext(), *best))) {
           best = &shortcut;
         }
@@ -509,7 +507,7 @@ const AppShortcut* Key::isPressed(const Message* msg, const KeyContext keyContex
         // etc.
         m_keycontext == KeyContext::MouseWheel) {
       for (const AppShortcut& shortcut : shortcuts()) {
-        if (shortcut.shortcut.modifiers() == mouseMsg->modifiers())
+        if (shortcut.modifiers() == mouseMsg->modifiers())
           return &shortcut;
       }
     }
@@ -554,7 +552,7 @@ bool Key::hasShortcut(const ui::Shortcut& shortcut) const
 bool Key::hasUserDefinedShortcuts() const
 {
   return std::any_of(m_adds.begin(), m_adds.end(), [](const auto& kv) {
-    return (kv.source == KeySource::UserDefined);
+    return (kv.source() == KeySource::UserDefined);
   });
 }
 
@@ -587,7 +585,7 @@ void Key::copyOriginalToUser()
   // Then copy all original & extension-defined keys as user-defined
   auto copy = m_adds;
   for (const auto& kv : copy)
-    m_adds.push_back(AppShortcut(KeySource::UserDefined, kv.shortcut));
+    m_adds.push_back(AppShortcut(KeySource::UserDefined, kv));
   m_shortcuts.reset();
 }
 

src/app/ui/key.h

@@ -105,27 +105,19 @@ inline KeyAction operator&(KeyAction a, KeyAction b)
 
 // This is a ui::Shortcut wrapper (just one key shortcut) defined by
 // the app, an extension, or the user (KeySource).
-struct AppShortcut {
+class AppShortcut : public ui::Shortcut {
-  KeySource source;
+public:
-  ui::Shortcut shortcut;
-
   AppShortcut(const KeySource source, const ui::Shortcut& shortcut)
-    : source(source)
+    : Shortcut(shortcut)
-    , shortcut(shortcut)
+    , m_source(source)
   {
   }
 
-  bool isEmpty() const { return shortcut.isEmpty(); }
+  KeySource source() const { return m_source; }
-  std::string toString() const { return shortcut.toString(); }
+  const ui::Shortcut& shortcut() const { return *this; }
-  bool isPressed() const { return shortcut.isPressed(); }
-  bool isLooselyPressed() const { return shortcut.isLooselyPressed(); }
 
-  bool operator==(const AppShortcut& other) const { return shortcut.operator==(other.shortcut); }
+  // bool operator==(const AppShortcut& other) const { return shortcut.operator==(other.shortcut); }
-  bool operator!=(const AppShortcut& other) const { return shortcut.operator!=(other.shortcut); }
+  // bool operator!=(const AppShortcut& other) const { return shortcut.operator!=(other.shortcut); }
-
-  ui::KeyModifiers modifiers() const { return shortcut.modifiers(); }
-  ui::KeyScancode scancode() const { return shortcut.scancode(); }
-  int unicodeChar() const { return shortcut.unicodeChar(); }
 
   // Returns true if this AppShortcut is better for the current
   // context, compared to another shortcut.
@@ -133,6 +125,9 @@ struct AppShortcut {
                       KeyContext thisShortcutContext,
                       KeyContext otherShortcutContext,
                       const AppShortcut& otherShortcut) const;
+
+private:
+  KeySource m_source;
 };
 
 using AppShortcuts = ui::ShortcutsT<AppShortcut>;

src/app/ui/keyboard_shortcuts.cpp

@@ -369,12 +369,12 @@ void KeyboardShortcuts::exportKeys(XMLElement* parent, KeyType type)
       continue;
 
     for (const auto& kv : key->delsKeys())
-      if (kv.source == KeySource::UserDefined)
+      if (kv.source() == KeySource::UserDefined)
-        exportShortcut(parent, key.get(), kv.shortcut, true);
+        exportShortcut(parent, key.get(), kv, true);
 
     for (const auto& kv : key->addsKeys())
-      if (kv.source == KeySource::UserDefined)
+      if (kv.source() == KeySource::UserDefined)
-        exportShortcut(parent, key.get(), kv.shortcut, false);
+        exportShortcut(parent, key.get(), kv, false);
   }
 }