From f53658ea4c73479bd63fb799bf3fa6df45427781 Mon Sep 17 00:00:00 2001 From: Matthias Koefferlein Date: Mon, 2 Dec 2024 21:29:05 +0100 Subject: [PATCH] Handling problem of redundant key bindings For example, @toolbar.ruler and edit_menu.modes.ruler was redundant in a sense that they point to the same action, hence, each entry in the key_binding config was overriding the other one. To solve this, there now is a primary entry, which is the edit_menu one. Only that entry is effective and listed in key_bindings. --- src/lay/lay/layMacroController.cc | 23 +++---------------- src/lay/lay/layMacroController.h | 2 -- src/lay/lay/layMainWindow.cc | 26 +++++++++++++--------- src/lay/lay/layMainWindow.h | 13 +++++++++-- src/laybasic/laybasic/layAbstractMenu.cc | 28 ++++++++++++++---------- src/laybasic/laybasic/layAbstractMenu.h | 20 ++++++++++++++--- 6 files changed, 64 insertions(+), 48 deletions(-) diff --git a/src/lay/lay/layMacroController.cc b/src/lay/lay/layMacroController.cc index 0b11f3215..1b4a5f52e 100644 --- a/src/lay/lay/layMacroController.cc +++ b/src/lay/lay/layMacroController.cc @@ -208,11 +208,6 @@ MacroController::uninitialize (lay::Dispatcher * /*root*/) bool MacroController::configure (const std::string &key, const std::string &value) { - if (key == cfg_key_bindings) { - m_key_bindings = unpack_key_binding (value); - } else if (key == cfg_menu_items_hidden) { - m_menu_items_hidden = unpack_menu_items_hidden (value); - } return false; } @@ -810,21 +805,9 @@ MacroController::do_update_menu_with_macros () add_macro_items_to_menu (m_temp_macros, used_names, groups, tech); add_macro_items_to_menu (lym::MacroCollection::root (), used_names, groups, tech); - // apply the custom keyboard shortcuts - for (std::vector >::const_iterator kb = m_key_bindings.begin (); kb != m_key_bindings.end (); ++kb) { - if (mp_mw->menu ()->is_valid (kb->first)) { - lay::Action *a = mp_mw->menu ()->action (kb->first); - a->set_shortcut (kb->second); - } - } - - // apply the custom hidden flags - for (std::vector >::const_iterator hf = m_menu_items_hidden.begin (); hf != m_menu_items_hidden.end (); ++hf) { - if (mp_mw->menu ()->is_valid (hf->first)) { - lay::Action *a = mp_mw->menu ()->action (hf->first); - a->set_hidden (hf->second); - } - } + // apply the custom keyboard shortcuts and hidden flags + mp_mw->apply_key_bindings (); + mp_mw->apply_hidden (); } void diff --git a/src/lay/lay/layMacroController.h b/src/lay/lay/layMacroController.h index ad8517f2f..76ee94868 100644 --- a/src/lay/lay/layMacroController.h +++ b/src/lay/lay/layMacroController.h @@ -249,8 +249,6 @@ private: tl::DeferredMethod dm_do_sync_with_external_sources; tl::DeferredMethod dm_sync_file_watcher; tl::DeferredMethod dm_sync_files; - std::vector > m_key_bindings; - std::vector > m_menu_items_hidden; void sync_implicit_macros (bool ask_before_autorun); void add_macro_items_to_menu (lym::MacroCollection &collection, std::set &used_names, std::set &groups, const db::Technology *tech); diff --git a/src/lay/lay/layMainWindow.cc b/src/lay/lay/layMainWindow.cc index f7444453c..33814584a 100644 --- a/src/lay/lay/layMainWindow.cc +++ b/src/lay/lay/layMainWindow.cc @@ -1192,8 +1192,8 @@ MainWindow::configure (const std::string &name, const std::string &value) } else if (name == cfg_menu_items_hidden) { - std::vector > hidden = unpack_menu_items_hidden (value); - apply_hidden (hidden); + m_hidden = unpack_menu_items_hidden (value); + apply_hidden (); return true; } else if (name == cfg_initial_technology) { @@ -1213,12 +1213,15 @@ MainWindow::configure (const std::string &name, const std::string &value) } void -MainWindow::apply_hidden (const std::vector > &hidden) +MainWindow::apply_hidden () { - for (std::vector >::const_iterator hf = hidden.begin (); hf != hidden.end (); ++hf) { - if (menu ()->is_valid (hf->first)) { - lay::Action *a = menu ()->action (hf->first); - a->set_hidden (hf->second); + for (std::vector >::const_iterator hf = m_hidden.begin (); hf != m_hidden.end (); ++hf) { + lay::AbstractMenuItem *item = menu ()->find_item_exact (hf->first); + if (item && item->primary ()) { + lay::Action *a = item->action (); + if (a) { + a->set_hidden (hf->second); + } } } } @@ -1227,9 +1230,12 @@ void MainWindow::apply_key_bindings () { for (std::vector >::const_iterator kb = m_key_bindings.begin (); kb != m_key_bindings.end (); ++kb) { - if (menu ()->is_valid (kb->first)) { - lay::Action *a = menu ()->action (kb->first); - a->set_shortcut (kb->second); + lay::AbstractMenuItem *item = menu ()->find_item_exact (kb->first); + if (item && item->primary ()) { + lay::Action *a = item->action (); + if (a) { + a->set_shortcut (kb->second); + } } } } diff --git a/src/lay/lay/layMainWindow.h b/src/lay/lay/layMainWindow.h index 90e3f2693..f2f43d591 100644 --- a/src/lay/lay/layMainWindow.h +++ b/src/lay/lay/layMainWindow.h @@ -534,6 +534,16 @@ public: */ static std::vector menu_symbols (); + /** + * @brief For internal use: apply current key bindings + */ + void apply_key_bindings (); + + /** + * @brief For internal use: apply hidden menu flags + */ + void apply_hidden (); + /** * @brief Open a new layout in mode 'mode' * @@ -770,6 +780,7 @@ private: double m_default_grid; bool m_default_grids_updated; std::vector > m_key_bindings; + std::vector > m_hidden; bool m_new_layout_current_panel; bool m_synchronized_views; bool m_synchronous; @@ -864,8 +875,6 @@ private: void plugin_removed (lay::PluginDeclaration *cls); void libraries_changed (); - void apply_key_bindings (); - void apply_hidden (const std::vector > &hidden); }; } diff --git a/src/laybasic/laybasic/layAbstractMenu.cc b/src/laybasic/laybasic/layAbstractMenu.cc index 44de01ab0..a97e45e7c 100644 --- a/src/laybasic/laybasic/layAbstractMenu.cc +++ b/src/laybasic/laybasic/layAbstractMenu.cc @@ -220,20 +220,22 @@ parse_menu_title (const std::string &s, std::string &title, std::string &shortcu // AbstractMenuItem implementation AbstractMenuItem::AbstractMenuItem (Dispatcher *dispatcher) - : mp_action (new Action ()), mp_dispatcher (dispatcher), m_has_submenu (false), m_remove_on_empty (false) + : mp_action (new Action ()), mp_dispatcher (dispatcher), m_has_submenu (false), m_remove_on_empty (false), m_primary (false) { // ... nothing yet .. } AbstractMenuItem::AbstractMenuItem (const AbstractMenuItem &item) - : mp_action (new Action ()), mp_dispatcher (item.dispatcher ()), m_has_submenu (false), m_remove_on_empty (false) + : mp_action (new Action ()), mp_dispatcher (item.dispatcher ()), m_has_submenu (false), m_remove_on_empty (false), m_primary (false) { // ... nothing yet .. } void -AbstractMenuItem::setup_item (const std::string &pn, const std::string &s, Action *a) +AbstractMenuItem::setup_item (const std::string &pn, const std::string &s, Action *a, bool primary) { + m_primary = primary; + m_basename.clear (); tl::Extractor ex (s.c_str ()); @@ -1587,6 +1589,8 @@ AbstractMenu::items (const std::string &path) const void AbstractMenu::insert_item (const std::string &p, const std::string &name, Action *action) { + bool primary = true; + tl::Extractor extr (p.c_str ()); while (! extr.at_end ()) { @@ -1601,7 +1605,8 @@ AbstractMenu::insert_item (const std::string &p, const std::string &name, Action parent->children.insert (iter, AbstractMenuItem (mp_dispatcher)); --iter; - iter->setup_item (parent->name (), name, action); + iter->setup_item (parent->name (), name, action, primary); + primary = false; // find any items with the same name and remove them for (std::list::iterator existing = parent->children.begin (); existing != parent->children.end (); ) { @@ -1635,7 +1640,7 @@ AbstractMenu::insert_separator (const std::string &p, const std::string &name) --iter; Action *action = new Action (); action->set_separator (true); - iter->setup_item (parent->name (), name, action); + iter->setup_item (parent->name (), name, action, true); } @@ -1655,7 +1660,7 @@ AbstractMenu::insert_menu (const std::string &p, const std::string &name, Action parent->children.insert (iter, AbstractMenuItem (mp_dispatcher)); --iter; - iter->setup_item (parent->name (), name, action); + iter->setup_item (parent->name (), name, action, true); iter->set_has_submenu (); // find any items with the same name and remove them @@ -1945,7 +1950,7 @@ AbstractMenu::find_item (tl::Extractor &extr) if (parent) { parent->children.insert (iter, AbstractMenuItem (mp_dispatcher)); --iter; - iter->setup_item (parent->name (), n, new Action ()); + iter->setup_item (parent->name (), n, new Action (), true); iter->set_has_submenu (); iter->set_remove_on_empty (); iter->set_action_title (ndesc.empty () ? n : ndesc); @@ -2055,16 +2060,17 @@ AbstractMenu::get_shortcuts (const std::string &root, std::map items = this->items (root); for (std::vector::const_iterator i = items.begin (); i != items.end (); ++i) { if (i->size () > 0) { - if (is_valid (*i) && action (*i)->is_visible ()) { - if (is_menu (*i)) { + const AbstractMenuItem *item = find_item_exact (*i); + if (item && item->action () && item->action ()->is_visible ()) { + if (item->has_submenu ()) { // a menu must be listed (so it can be hidden), but does not have a shortcut // but we don't include special menus if (i->at (0) != '@') { bindings.insert (std::make_pair (*i, std::string ())); } get_shortcuts (*i, bindings, with_defaults); - } else if (! is_separator (*i)) { - bindings.insert (std::make_pair (*i, with_defaults ? action (*i)->get_default_shortcut () : action (*i)->get_effective_shortcut ())); + } else if (! item->action ()->is_separator () && item->primary ()) { + bindings.insert (std::make_pair (*i, with_defaults ? item->action ()->get_default_shortcut () : item->action ()->get_effective_shortcut ())); } } } diff --git a/src/laybasic/laybasic/layAbstractMenu.h b/src/laybasic/laybasic/layAbstractMenu.h index aec72ac7a..d7908f5b7 100644 --- a/src/laybasic/laybasic/layAbstractMenu.h +++ b/src/laybasic/laybasic/layAbstractMenu.h @@ -559,7 +559,7 @@ struct LAYBASIC_PUBLIC AbstractMenuItem /** * @brief Internal method used to set up the item */ - void setup_item (const std::string &pn, const std::string &n, Action *a); + void setup_item (const std::string &pn, const std::string &n, Action *a, bool primary); Dispatcher *dispatcher () const { @@ -616,6 +616,11 @@ struct LAYBASIC_PUBLIC AbstractMenuItem return m_remove_on_empty; } + bool primary () const + { + return m_primary; + } + std::list children; private: @@ -623,6 +628,7 @@ private: Dispatcher *mp_dispatcher; bool m_has_submenu; bool m_remove_on_empty; + bool m_primary; std::string m_name; std::string m_basename; std::set m_groups; @@ -906,6 +912,16 @@ public: return m_root; } + /** + * @brief For internal use: gets the AbstractMenuItem for a given path or 0 if the path is not valid (const version) + */ + const AbstractMenuItem *find_item_exact (const std::string &path) const; + + /** + * @brief For internal use: gets the AbstractMenuItem for a given path or 0 if the path is not valid + */ + AbstractMenuItem *find_item_exact (const std::string &path); + #if defined(HAVE_QT) signals: /** @@ -918,8 +934,6 @@ private: friend class Action; std::vector::iterator> > find_item (tl::Extractor &extr); - const AbstractMenuItem *find_item_exact (const std::string &path) const; - AbstractMenuItem *find_item_exact (const std::string &path); const AbstractMenuItem *find_item_for_action (const Action *action, const AbstractMenuItem *from = 0) const; AbstractMenuItem *find_item_for_action (const Action *action, AbstractMenuItem *from = 0); #if defined(HAVE_QT)