Merge pull request #5000 from chennes/checkForConflictingAccelerators

[Core] Add checks for conflicting accelerators

src/Gui/Command.cpp

@@ -958,16 +958,24 @@ void Command::adjustCameraPosition()
     }
 }
 
+void Command::printConflictingAccelerators() const
+{
+    auto cmd = Application::Instance->commandManager().checkAcceleratorForConflicts(sAccel, this);
+    if (cmd)
+        Base::Console().Warning("Accelerator conflict between %s (%s) and %s (%s)\n", sName, sAccel, cmd->sName, cmd->sAccel);
+}
+
 Action * Command::createAction(void)
 {
     Action *pcAction;
-
     pcAction = new Action(this,getMainWindow());
+#ifdef FC_DEBUG
+    printConflictingAccelerators();
+#endif
     pcAction->setShortcut(QString::fromLatin1(sAccel));
     applyCommandData(this->className(), pcAction);
     if (sPixmap)
         pcAction->setIcon(Gui::BitmapFactory().iconFromTheme(sPixmap));
-
     return pcAction;
 }
 
@@ -1131,6 +1139,9 @@ Action * MacroCommand::createAction(void)
     pcAction->setWhatsThis(QString::fromUtf8(sWhatsThis));
     if (sPixmap)
         pcAction->setIcon(Gui::BitmapFactory().pixmap(sPixmap));
+#ifdef FC_DEBUG
+    printConflictingAccelerators();
+#endif
     pcAction->setShortcut(QString::fromLatin1(sAccel));
 
     QString accel = pcAction->shortcut().toString(QKeySequence::NativeText);
@@ -1333,6 +1344,9 @@ Action * PythonCommand::createAction(void)
     Action *pcAction;
 
     pcAction = new Action(this, qtAction, getMainWindow());
+#ifdef FC_DEBUG
+    printConflictingAccelerators();
+#endif
     pcAction->setShortcut(QString::fromLatin1(getAccel()));
     applyCommandData(this->getName(), pcAction);
     if (strcmp(getResource("Pixmap"),"") != 0)
@@ -1845,3 +1859,57 @@ void CommandManager::updateCommands(const char* sContext, int mode)
         }
     }
 }
+
+const Command* Gui::CommandManager::checkAcceleratorForConflicts(const char* accel, const Command* ignore) const
+{
+    if (!accel || accel[0] == '\0')
+        return nullptr;
+
+    QString newCombo = QString::fromLatin1(accel);
+    if (newCombo.isEmpty())
+        return nullptr;
+    auto newSequence = QKeySequence::fromString(newCombo);
+    if (newSequence.count() == 0)
+        return nullptr;
+
+    // Does this command shortcut conflict with other commands already defined?
+    auto commands = Application::Instance->commandManager().getAllCommands();
+    for (const auto& cmd : commands) {
+        if (cmd == ignore)
+            continue;
+        auto existingAccel = cmd->getAccel();
+        if (!existingAccel || existingAccel[0] == '\0')
+            continue;
+
+        // Three possible conflict scenarios:
+        // 1) Exactly the same combo as another command
+        // 2) The new command is a one-char combo that overrides an existing two-char combo
+        // 3) The old command is a one-char combo that overrides the new command
+
+        QString existingCombo = QString::fromLatin1(existingAccel);
+        if (existingCombo.isEmpty())
+            continue;
+        auto existingSequence = QKeySequence::fromString(existingCombo);
+        if (existingSequence.count() == 0)
+            continue;
+
+        // Exact match
+        if (existingSequence == newSequence)
+            return cmd;
+
+        // If it's not exact, then see if one of the sequences is a partial match for
+        // the beginning of the other sequence
+        auto numCharsToCheck = std::min(existingSequence.count(), newSequence.count());
+        bool firstNMatch = true;
+        for (int i = 0; i < numCharsToCheck; ++i) {
+            if (newSequence[i] != existingSequence[i]) {
+                firstNMatch = false;
+                break;
+            }
+        }
+        if (firstNMatch)
+            return cmd;
+    }
+
+    return nullptr;
+}

src/Gui/Command.h

@@ -330,6 +330,7 @@ protected:
     /// Applies the menu text, tool and status tip to the passed action object
     void applyCommandData(const char* context, Action* );
     const char* keySequenceToAccel(int) const;
+    void printConflictingAccelerators() const;
     //@}
 
 public:
@@ -872,6 +873,14 @@ public:
     void addCommandMode(const char* sContext, const char* sName);
     void updateCommands(const char* sContext, int mode);
 
+    /** 
+     * Returns a pointer to a conflicting command, or nullptr if there is no conflict.
+     * In the case of multiple conflicts, only the first is returned. 
+     * \param accel The accelerator to check
+     * \param ignore (optional) A command to ignore matches with
+     */
+    const Command* checkAcceleratorForConflicts(const char* accel, const Command *ignore = nullptr) const;
+
 private:
     /// Destroys all commands in the manager and empties the list.
     void clearCommands();