diff --git a/README.localization b/README.localization index 7e4d3d7cdb..2f1b3512df 100644 --- a/README.localization +++ b/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. diff --git a/lib/ui/stdcontext.inc b/lib/ui/stdcontext.inc index dc49f44c77..32d76e603e 100644 --- a/lib/ui/stdcontext.inc +++ b/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 diff --git a/src/frontends/qt/Menus.cpp b/src/frontends/qt/Menus.cpp index 4a09a1b4a7..193e2478fa 100644 --- a/src/frontends/qt/Menus.cpp +++ b/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 - void checkShortcuts() const; + /// Check whether the shortcut of \p mi are unique and valid, and report if not + 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 - // menus are short enough - for (const_iterator it1 = begin(); it1 != end(); ++it1) { - QString shortcut = it1->shortcut(); - if (shortcut.isEmpty()) - continue; - if (!it1->label().contains(shortcut)) + // This might be a string of accelerators, a single accelerator + // or empty + QString accelerators = mi.shortcut(); + QString const label = mi.label(); + if (accelerators.size() == 0) + return QString(); + 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 << "'."); - for (const_iterator it2 = begin(); it2 != it1 ; ++it2) { - if (!it2->shortcut().compare(shortcut, Qt::CaseInsensitive)) { - LYXERR0("Menu warning: menu entries " - << '"' << it1->fulllabel() - << "\" and \"" << it2->fulllabel() - << "\" share the same shortcut."); - } + << sc << "'."); + continue; + } + if (checkShortcut(sc)) + return sc; + } + // 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