From 717d19d3c3e7e0ad38c0344da066eeda21255fdc Mon Sep 17 00:00:00 2001
From: Richard Heck <rgheck@lyx.org>
Date: Sat, 15 Mar 2014 15:36:55 -0400
Subject: [PATCH] Fix for bug #8939: Don't just close if the user enters a
 shortcut that is already assigned.

From new contributor Roy Xia.
---
 lib/generate_contributions.py  |  8 +++++++
 src/frontends/qt4/GuiPrefs.cpp | 38 +++++++++++++++++++++++++---------
 2 files changed, 36 insertions(+), 10 deletions(-)

diff --git a/lib/generate_contributions.py b/lib/generate_contributions.py
index 0c7e59a80d..f398355986 100755
--- a/lib/generate_contributions.py
+++ b/lib/generate_contributions.py
@@ -1907,6 +1907,14 @@ contributors = [
                  "3 March 2014",
                  u"Chinese (traditional) translations"),
 
+     contributor(u"Roy Xia",
+                 "royxia062 () gmail ! com",
+                 "GPL",
+                 "GPL Statement",
+                 "m=139434481324689",
+                 "9 March 2014",
+                 u"Bugfixing"),
+
      contributor(u"Yihui Xie",
                  "xie () yihui ! name",
                  "GPL",
diff --git a/src/frontends/qt4/GuiPrefs.cpp b/src/frontends/qt4/GuiPrefs.cpp
index e910b0cd97..d93124931e 100644
--- a/src/frontends/qt4/GuiPrefs.cpp
+++ b/src/frontends/qt4/GuiPrefs.cpp
@@ -3182,8 +3182,6 @@ void PrefShortcuts::shortcutOkPressed()
 		return;
 	}
 
-	shortcut_->accept();
-
 	// check to see if there's been any change
 	FuncRequest oldBinding = system_bind_.getBinding(k);
 	if (oldBinding.action() == LFUN_UNKNOWN_ACTION)
@@ -3192,20 +3190,40 @@ void PrefShortcuts::shortcutOkPressed()
 		// nothing has changed
 		return;
 	
-	// make sure this key isn't already bound---and, if so, not unbound
+	// make sure this key isn't already bound---and, if so, prompt user
 	FuncCode const unbind = user_unbind_.getBinding(k).action();
 	docstring const action_string = makeCmdString(oldBinding);
 	if (oldBinding.action() > LFUN_NOACTION && unbind == LFUN_UNKNOWN_ACTION
 		  && save_lfun_ != toqstr(action_string)) {
-		// FIXME Perhaps we should offer to over-write the old shortcut?
-		// If so, we'll need to remove it from our list, etc.
-		Alert::error(_("Failed to create shortcut"),
-			bformat(_("Shortcut `%1$s' is already bound to:\n%2$s\n"
-			  "You need to remove that binding before creating a new one."), 
-			k.print(KeySequence::ForGui), action_string));
-		return;
+		docstring const new_action_string = makeCmdString(func);
+		docstring const text = bformat(_("Shortcut `%1$s' is already bound to "
+						 "%2$s.\n"
+						 "Are you sure you want to unbind the "
+						 "current shortcut and bind it to %3$s?"),
+					       k.print(KeySequence::ForGui), action_string,
+					       new_action_string);
+		int ret = Alert::prompt(_("Redefine shortcut?"),
+					text, 0, 1, _("&Redefine"), _("&Cancel"));
+		if (ret != 0)
+			return;
+		QString const sequence_text = toqstr(k.print(KeySequence::ForGui));
+		QList<QTreeWidgetItem*> items = shortcutsTW->findItems(sequence_text,
+			Qt::MatchFlags(Qt::MatchExactly | Qt::MatchRecursive), 1);
+		if (items.size() > 0) {
+			// should always happen
+			bool expanded = items[0]->parent()->isExpanded();
+			shortcutsTW->setCurrentItem(items[0]);
+			removeShortcut();
+			shortcutsTW->setCurrentItem(0);
+			// make sure user doesn't see tree expansion if
+			// old binding wasn't in an expanded tree
+			if (!expanded)
+				items[0]->parent()->setExpanded(false);
+		}
 	}
 
+	shortcut_->accept();
+
 	if (!save_lfun_.isEmpty())
 		// real modification of the lfun's shortcut,
 		// so remove the previous one