From 2af6a0493e32e56feed3450c322909e051026d5f Mon Sep 17 00:00:00 2001 From: David Capello Date: Wed, 8 Apr 2020 17:50:17 -0300 Subject: [PATCH] Fix recent list of files menu This is a problem introduced with the plugin groups, but now we use a group to store the list of recent files. With this commit we fixed some bugs in the impl of menu groups. --- data/gui.xml | 2 +- src/app/app_menus.cpp | 104 +++++++++++++++++++++----------- src/app/app_menus.h | 12 +++- src/app/extensions.cpp | 2 +- src/app/script/plugin_class.cpp | 3 +- 5 files changed, 81 insertions(+), 42 deletions(-) diff --git a/data/gui.xml b/data/gui.xml index 37986861d..fbc1a44b9 100644 --- a/data/gui.xml +++ b/data/gui.xml @@ -616,7 +616,7 @@ - + diff --git a/src/app/app_menus.cpp b/src/app/app_menus.cpp index d0d8ce708..e4dd22afb 100644 --- a/src/app/app_menus.cpp +++ b/src/app/app_menus.cpp @@ -38,8 +38,12 @@ #include "tinyxml.h" #include -#include #include +#include +#include +#include + +#define MENUS_TRACE(...) // TRACEARGS namespace app { @@ -61,6 +65,8 @@ const int kUnicodeRight = 0xF703; // NSRightArrowFunctionKey const int kUnicodeUp = 0xF700; // NSUpArrowFunctionKey const int kUnicodeDown = 0xF701; // NSDownArrowFunctionKey +const char* kFileRecentListGroup = "file_recent_list"; + void destroy_instance(AppMenus* instance) { delete instance; @@ -320,20 +326,22 @@ AppMenus::~AppMenus() void AppMenus::reload() { + MENUS_TRACE("MENUS: AppMenus::reload()"); + XmlDocumentRef doc(GuiXml::instance()->doc()); TiXmlHandle handle(doc.get()); const char* path = GuiXml::instance()->filename(); //////////////////////////////////////// - // Remove all menu items added from scripts so we can re-add them - // later in the new menus. + // Remove all menu items added to groups from recent files and + // scripts so we can re-add them later in the new menus. for (auto& it : m_groups) { GroupInfo& group = it.second; - group.end = nullptr; - for (auto& item : group.items) { + MENUS_TRACE("MENUS: - groups", it.first, "with", group.items.size(), "item(s)"); + group.end = nullptr; // This value will be restored later + for (auto& item : group.items) item->parent()->removeChild(item); - } } //////////////////////////////////////// @@ -379,11 +387,13 @@ void AppMenus::reload() } //////////////////////////////////////// - // Re-add menu items from scripts + // Re-add menu items in groups (recent files & scripts) for (auto& it : m_groups) { GroupInfo& group = it.second; if (group.end) { + MENUS_TRACE("MENUS: - re-adding group ", it.first, "with", group.items.size(), "item(s)"); + auto menu = group.end->parent(); int insertIndex = menu->getChildIndex(group.end); for (auto& item : group.items) { @@ -393,8 +403,9 @@ void AppMenus::reload() } // Delete items that don't have a group now else { + MENUS_TRACE("MENUS: - deleting group ", it.first, "with", group.items.size(), "item(s)"); for (auto& item : group.items) - delete item; + item->deferDelete(); group.items.clear(); } } @@ -483,6 +494,8 @@ void AppMenus::initTheme() bool AppMenus::rebuildRecentList() { + MENUS_TRACE("MENUS: AppMenus::rebuildRecentList m_recentFilesPlaceholder=", m_recentFilesPlaceholder); + if (!m_recentFilesPlaceholder) return true; @@ -494,13 +507,10 @@ bool AppMenus::rebuildRecentList() if (!owner || owner->hasSubmenuOpened()) return false; - int insertIndex = menu->getChildIndex(m_recentFilesPlaceholder)+1; - // Remove active items - while (auto appItem = dynamic_cast(menu->at(insertIndex))) { - menu->removeChild(appItem); - appItem->deferDelete(); - } + for (auto item : m_recentMenuItems) + removeMenuItemFromGroup(item); + m_recentMenuItems.clear(); Command* openFile = Commands::instance()->byId(CommandId::OpenFile()); @@ -517,18 +527,24 @@ bool AppMenus::rebuildRecentList() for (const auto& fn : files) { params.set("filename", fn.c_str()); - auto menuitem = new AppMenuItem(base::get_file_name(fn).c_str(), - openFile, params); + std::unique_ptr menuitem( + new AppMenuItem(base::get_file_name(fn).c_str(), + openFile, params)); menuitem->setIsRecentFileItem(true); - menu->insertChild(insertIndex++, menuitem); + + m_recentMenuItems.push_back(menuitem.get()); + addMenuItemIntoGroup(kFileRecentListGroup, std::move(menuitem)); } } else { - auto menuitem = new AppMenuItem( - Strings::main_menu_file_no_recent_file(), nullptr); + std::unique_ptr menuitem( + new AppMenuItem( + Strings::main_menu_file_no_recent_file(), nullptr)); menuitem->setIsRecentFileItem(true); menuitem->setEnabled(false); - menu->insertChild(insertIndex++, menuitem); + + m_recentMenuItems.push_back(menuitem.get()); + addMenuItemIntoGroup(kFileRecentListGroup, std::move(menuitem)); } // Sync native menus @@ -546,14 +562,9 @@ bool AppMenus::rebuildRecentList() } void AppMenus::addMenuItemIntoGroup(const std::string& groupId, - const std::string& title, std::unique_ptr&& menuItem) { - auto it = m_groups.find(groupId); - if (it == m_groups.end()) - return; - - GroupInfo& group = it->second; + GroupInfo& group = m_groups[groupId]; Widget* menu = group.end->parent(); ASSERT(menu); int insertIndex = menu->getChildIndex(group.end); @@ -565,25 +576,46 @@ void AppMenus::addMenuItemIntoGroup(const std::string& groupId, menuItem.release(); } -void AppMenus::removeMenuItemWithCommand(Command* cmd) +template +void AppMenus::removeMenuItemFromGroup(Pred pred) { for (auto& it : m_groups) { GroupInfo& group = it.second; - group.end = nullptr; for (auto it=group.items.begin(); it != group.items.end(); ) { auto& item = *it; - auto appMenuItem = dynamic_cast(item); - if (appMenuItem && - appMenuItem->getCommand() == cmd) { - delete appMenuItem; + if (pred(item)) { + if (item == group.end) + group.end = group.end->previousSibling(); + + item->parent()->removeChild(item); + item->deferDelete(); + it = group.items.erase(it); } - else + else { ++it; + } } } } +void AppMenus::removeMenuItemFromGroup(Command* cmd) +{ + removeMenuItemFromGroup( + [cmd](Widget* item){ + auto appMenuItem = dynamic_cast(item); + return (appMenuItem && appMenuItem->getCommand() == cmd); + }); +} + +void AppMenus::removeMenuItemFromGroup(Widget* menuItem) +{ + removeMenuItemFromGroup( + [menuItem](Widget* item){ + return (item == menuItem); + }); +} + Menu* AppMenus::loadMenuById(TiXmlHandle& handle, const char* id) { ASSERT(id != NULL); @@ -642,7 +674,8 @@ Widget* AppMenus::convertXmlelemToMenuitem(TiXmlElement* elem) m_recentFilesPlaceholder = item; } } - if (group) m_groups[group].end = item; + if (group) + m_groups[group].end = item; return item; } @@ -674,7 +707,8 @@ Widget* AppMenus::convertXmlelemToMenuitem(TiXmlElement* elem) if (id) menuitem->setId(id); menuitem->processMnemonicFromText(); - if (group) m_groups[group].end = menuitem; + if (group) + m_groups[group].end = menuitem; // Has it a ID? if (id) { diff --git a/src/app/app_menus.h b/src/app/app_menus.h index 8881c8d81..8991b8e5a 100644 --- a/src/app/app_menus.h +++ b/src/app/app_menus.h @@ -65,12 +65,16 @@ namespace app { const KeyPtr& key); void syncNativeMenuItemKeyShortcuts(); + // Menu item handling in groups void addMenuItemIntoGroup(const std::string& groupId, - const std::string& title, std::unique_ptr&& menuItem); - void removeMenuItemWithCommand(Command* cmd); + void removeMenuItemFromGroup(Command* cmd); + void removeMenuItemFromGroup(Widget* menuItem); private: + template + void removeMenuItemFromGroup(Pred pred); + Menu* loadMenuById(TiXmlHandle& handle, const char *id); Menu* convertXmlelemToMenu(TiXmlElement* elem); Widget* convertXmlelemToMenuitem(TiXmlElement* elem); @@ -88,7 +92,7 @@ namespace app { #endif struct GroupInfo { - Widget* end; + Widget* end = nullptr; WidgetsList items; }; @@ -107,6 +111,8 @@ namespace app { std::unique_ptr m_inkPopupMenu; obs::scoped_connection m_recentFilesConn; std::vector m_menus; + // List of recent menu items pointing to recent files. + WidgetsList m_recentMenuItems; // Extension points for plugins (each group is a place where new // menu items can be added). std::map m_groups; diff --git a/src/app/extensions.cpp b/src/app/extensions.cpp index 09ea7510c..c3cf092c3 100644 --- a/src/app/extensions.cpp +++ b/src/app/extensions.cpp @@ -645,7 +645,7 @@ void Extension::exitScripts() if (cmd) { #ifdef ENABLE_UI // TODO use a signal - AppMenus::instance()->removeMenuItemWithCommand(cmd); + AppMenus::instance()->removeMenuItemFromGroup(cmd); #endif // ENABLE_UI cmds->remove(cmd); diff --git a/src/app/script/plugin_class.cpp b/src/app/script/plugin_class.cpp index c05a18244..8d40ea2dc 100644 --- a/src/app/script/plugin_class.cpp +++ b/src/app/script/plugin_class.cpp @@ -134,8 +134,7 @@ int Plugin_newCommand(lua_State* L) App::instance()->isGui()) { // On CLI menus do not make sense if (auto appMenus = AppMenus::instance()) { std::unique_ptr menuItem(new AppMenuItem(title, cmd)); - appMenus->addMenuItemIntoGroup( - group, title, std::move(menuItem)); + appMenus->addMenuItemIntoGroup(group, std::move(menuItem)); } } #endif // ENABLE_UI