Allow for multiple accelerator alternatives

This needs some testing before it could go to 2.4.x eventually

README.localization

@@ -64,6 +64,16 @@ These chars should be somehow used in your translations, however you'll have to
 invent your own working shortcuts for dialog and menu entries and resolve
 possible conflicts of the same shortcut chars in one menu...
 
+You will be informed about conflicts in the terminal if you try to access the
+menu.
+
+Note that, in the case of '|', if more than one character follows, this means
+that LyX will try each of them in turn and use the first one that is not yet
+used by another entry in the menu. That way, you can define alternative shortcuts
+in the case one works in one context only, and another one only in another. You
+can use this possibility also in translations, but please use it only if no
+single shortcut that fits could be found.
+
 Note also that there are already used global shortcuts (such as p k x c m s a)
 and you should avoid using these characters for first-level menu shortcuts.
 

lib/ui/stdcontext.inc

@@ -650,7 +650,7 @@ Menuset
 	Menu "context-edit-index"
 		OptItem "Insert Subentry|n" "indexmacro-insert subentry"
 		OptItem "Insert Sortkey|k" "indexmacro-insert sortkey"
-		OptItem "Insert See Reference|c" "indexmacro-insert see"
+		OptItem "Insert See Reference|cf" "indexmacro-insert see"
 		OptItem "Insert See also Reference|a" "indexmacro-insert seealso"
 	End
 

src/frontends/qt/Menus.cpp

@@ -233,10 +233,19 @@ public:
 	}
 
 	/// The keyboard shortcut (usually underlined in the entry)
-	QString shortcut() const
+	/// If \p first is true, return only the first character
+	/// if a multi-character string has been defined.
+	QString shortcut(bool first = false) const
 	{
 		int const index = label_.lastIndexOf('|');
-		return index == -1 ? QString() : label_.mid(index + 1);
+		if (index == -1)
+			return QString();
+		QString accelerators = label_.mid(index + 1);
+		if (accelerators.size() == 1)
+			return accelerators;
+		if (first)
+			return accelerators.left(1);
+		return accelerators;
 	}
 	/// The complete label, with label and shortcut separated by a '|'
 	QString fulllabel() const { return label_; }
@@ -349,8 +358,12 @@ public:
 	/// Checks the associated FuncRequest status before adding the
 	/// menu item.
 	void addWithStatusCheck(MenuItem const &);
-	// Check whether the menu shortcuts are unique
+	/// Check whether the shortcut of \p mi are unique and valid, and report if not
-	void checkShortcuts() const;
+	void checkShortcutUnique(MenuItem const & mi) const;
+	/// Return true if a \p sc is a unique shortcut
+	bool checkShortcut(QString const sc) const;
+	/// Try to find a unique shortcut from a string of alternatives
+	QString getBestShortcut(MenuItem const & mi) const;
 	///
 	void expandLastfiles();
 	void expandDocuments();
@@ -760,31 +773,87 @@ void MenuDefinition::cat(MenuDefinition const & other)
 }
 
 
-void MenuDefinition::checkShortcuts() const
+QString MenuDefinition::getBestShortcut(MenuItem const & mi) const
 {
-	// This is a quadratic algorithm, but we do not care because
+	// This might be a string of accelerators, a single accelerator
-	// menus are short enough
+	// or empty
-	for (const_iterator it1 = begin(); it1 != end(); ++it1) {
+	QString accelerators = mi.shortcut();
-		QString shortcut = it1->shortcut();
+	QString const label = mi.label();
-		if (shortcut.isEmpty())
+	if (accelerators.size() == 0)
-			continue;
+		return QString();
-		if (!it1->label().contains(shortcut))
+	if (accelerators.size() == 1) {
+		// check and report clashes
+		checkShortcutUnique(mi);
+		return accelerators;
+	}
+	for (int i = 0; i < accelerators.length(); i++)
+	{
+		// check each character in the string
+		// and use the first that does not conflict
+		QString const sc = accelerators.at(i);
+		if (!label.contains(sc)) {
+			// invalid shortcut. Report.
 			LYXERR0("Menu warning: menu entry \""
-			       << it1->label()
+			       << label
 			       << "\" does not contain shortcut `"
-			       << shortcut << "'.");
+			       << sc << "'.");
-		for (const_iterator it2 = begin(); it2 != it1 ; ++it2) {
+			continue;
-			if (!it2->shortcut().compare(shortcut, Qt::CaseInsensitive)) {
+		}
-				LYXERR0("Menu warning: menu entries "
+		if (checkShortcut(sc))
-				       << '"' << it1->fulllabel()
+			return sc;
-				       << "\" and \"" << it2->fulllabel()
+	}
-				       << "\" share the same shortcut.");
+	// At this point, we found no non-conflicting one
-			}
+	// issue a warning and omit the accelerator
+	LYXERR0("Menu warning: All accelerators of menu entry "
+	       << '"' << mi.fulllabel()
+	       << "\" are already taken. Omitting shortcut.");
+	return QString();
+}
+
+
+void MenuDefinition::checkShortcutUnique(MenuItem const & mi) const
+{
+	// We know this might only be a single character or nothing
+	QString const shortcut = mi.shortcut();
+	QString const label = mi.label();
+	if (shortcut.isEmpty())
+		return;
+	if (!label.contains(shortcut))
+		// invalid shortcut
+		LYXERR0("Menu warning: menu entry \""
+		       << label
+		       << "\" does not contain shortcut `"
+		       << shortcut << "'.");
+	for (const_iterator it = begin(); it != end() ; ++it) {
+		if (mi.fulllabel() == it->fulllabel())
+			// do not compare with itself
+			continue;
+		if (!it->shortcut().compare(shortcut, Qt::CaseInsensitive)) {
+			// conflict
+			LYXERR0("Menu warning: menu entries "
+			       << '"' << it->fulllabel()
+			       << "\" and \"" << mi.fulllabel()
+			       << "\" share the same shortcut.");
 		}
 	}
 }
 
 
+bool MenuDefinition::checkShortcut(QString const shortcut) const
+{
+	if (shortcut.isEmpty())
+		return true;
+	for (const_iterator it = begin(); it != end(); ++it) {
+		if (it->shortcut(true).compare(shortcut, Qt::CaseInsensitive) == 0) {
+			// conflict
+			return false;
+		}
+	}
+	// no conflict
+	return true;
+}
+
+
 bool MenuDefinition::searchMenu(FuncRequest const & func, docstring_list & names) const
 {
 	const_iterator m = begin();
@@ -2116,12 +2185,12 @@ struct Menu::Impl
 
 
 /// Get a MenuDefinition item label from the menu backend
-static QString label(MenuItem const & mi)
+static QString label(MenuItem const & mi, MenuDefinition const & menu)
 {
 	QString label = mi.label();
 	label.replace("&", "&&");
 
-	QString shortcut = mi.shortcut();
+	QString shortcut = menu.getBestShortcut(mi);
 	if (!shortcut.isEmpty()) {
 		int pos = label.indexOf(shortcut);
 		if (pos != -1)
@@ -2158,7 +2227,7 @@ void Menu::Impl::populate(QMenu * qMenu, MenuDefinition const & menu)
 			qMenu->addSeparator();
 			break;
 		case MenuItem::Submenu: {
-			QMenu * subMenu = qMenu->addMenu(label(m));
+			QMenu * subMenu = qMenu->addMenu(label(m, menu));
 			populate(subMenu, m.submenu());
 			subMenu->setEnabled(!subMenu->isEmpty());
 			break;
@@ -2168,7 +2237,7 @@ void Menu::Impl::populate(QMenu * qMenu, MenuDefinition const & menu)
 			// FIXME: A previous comment assured that MenuItem::Command was the
 			// only possible case in practice, but this is wrong.  It would be
 			// good to document which cases are actually treated here.
-			qMenu->addAction(new Action(m.func(), QIcon(), label(m),
+			qMenu->addAction(new Action(m.func(), QIcon(), label(m, menu),
 			                            m.tooltip(), qMenu));
 			break;
 		}
@@ -2529,9 +2598,6 @@ void Menus::Impl::expand(MenuDefinition const & frommenu,
 	// we do not want the menu to end with a separator
 	if (!tomenu.empty() && tomenu.items_.back().kind() == MenuItem::Separator)
 		tomenu.items_.pop_back();
-
-	// Check whether the shortcuts are unique
-	tomenu.checkShortcuts();
 }
 
 
@@ -2700,7 +2766,7 @@ void Menus::fillMenuBar(QMenuBar * qmb, GuiView * view, bool initial)
 		}
 
 		Menu * menuptr = new Menu(view, m.submenuname(), true);
-		menuptr->setTitle(label(m));
+		menuptr->setTitle(label(m, menu));
 
 #if defined(Q_OS_MAC)
 		// On Mac OS with Qt/Cocoa, the menu is not displayed if there is no action