MenuSorter: review comments and various bug fixes

* fixed wrongful retention of submenus variable and added return to MenuSorter:findById
* fixed readerfooter_spec.lua error
* fixed review comments

frontend/apps/filemanager/filemanagerhistory.lua

@@ -21,7 +21,7 @@ end
 
 function FileManagerHistory:addToMainMenu(tab_item_table)
     -- insert table to main tab of filemanager menu
-    self.ui.menu.menu_items["history"] = {
+    self.ui.menu.menu_items.history = {
         text = self.hist_menu_title,
         callback = function()
             self:onShowHist()

frontend/apps/filemanager/filemanagermenu.lua

@@ -21,21 +21,23 @@ local FileManagerMenu = InputContainer:extend{
 }
 
 function FileManagerMenu:init()
-    self.menu_items["KOMenu:menu_buttons"] = {
-        -- top menu
-    }
-    -- items in top menu
-    self.menu_items["setting"] = {
-        icon = "resources/icons/appbar.settings.png",
-    }
-    self.menu_items["tools"] = {
-        icon = "resources/icons/appbar.tools.png",
-    }
-    self.menu_items["search"] = {
-        icon = "resources/icons/appbar.magnify.browse.png",
-    }
-    self.menu_items["main"] = {
-        icon = "resources/icons/menu-icon.png",
+    self.menu_items = {
+        ["KOMenu:menu_buttons"] = {
+            -- top menu
+        },
+        -- items in top menu
+        setting = {
+            icon = "resources/icons/appbar.settings.png",
+        },
+        tools = {
+            icon = "resources/icons/appbar.tools.png",
+        },
+        search = {
+            icon = "resources/icons/appbar.magnify.browse.png",
+        },
+        main = {
+            icon = "resources/icons/menu-icon.png",
+        },
     }
 
     self.registered_widgets = {}
@@ -69,18 +71,18 @@ function FileManagerMenu:setUpdateItemTable()
     end
 
     -- setting tab
-    self.menu_items["show_hidden_files"] = {
+    self.menu_items.show_hidden_files = {
         text = _("Show hidden files"),
         checked_func = function() return self.ui.file_chooser.show_hidden end,
         callback = function() self.ui:toggleHiddenFiles() end
     }
-    self.menu_items["sort_by"] = self.ui:getSortingMenuTable()
-    self.menu_items["reverse_sorting"] = {
+    self.menu_items.sort_by = self.ui:getSortingMenuTable()
+    self.menu_items.reverse_sorting = {
         text = _("Reverse sorting"),
         checked_func = function() return self.ui.file_chooser.reverse_collate end,
         callback = function() self.ui:toggleReverseCollate() end
     }
-    self.menu_items["start_with_last_opened_file"] = {
+    self.menu_items.start_with_last_opened_file = {
         text = _("Start with last opened file"),
         checked_func = function() return
             G_reader_settings:readSetting("open_last")
@@ -95,7 +97,7 @@ function FileManagerMenu:setUpdateItemTable()
         end
     }
     if Device.isKobo() then
-        self.menu_items["screensaver"] = {
+        self.menu_items.screensaver = {
             text = _("Screensaver"),
             sub_item_table = {
                 {
@@ -157,7 +159,7 @@ function FileManagerMenu:setUpdateItemTable()
     end
 
     -- tools tab
-    self.menu_items["advanced_settings"] = {
+    self.menu_items.advanced_settings = {
         text = _("Advanced settings"),
         callback = function()
             SetDefaults:ConfirmEdit()
@@ -166,7 +168,7 @@ function FileManagerMenu:setUpdateItemTable()
             SetDefaults:ConfirmSave()
         end,
     }
-    self.menu_items["opds_catalog"] = {
+    self.menu_items.opds_catalog = {
         text = _("OPDS catalog"),
         callback = function()
             local OPDSCatalog = require("apps/opdscatalog/opdscatalog")
@@ -178,7 +180,7 @@ function FileManagerMenu:setUpdateItemTable()
             OPDSCatalog:showCatalog()
         end,
     }
-    self.menu_items["developer_options"] = {
+    self.menu_items.developer_options = {
         text = _("Developer options"),
         sub_item_table = {
             {
@@ -208,7 +210,7 @@ function FileManagerMenu:setUpdateItemTable()
             },
         }
     }
-    self.menu_items["cloud_storage"] = {
+    self.menu_items.cloud_storage = {
         text = _("Cloud storage"),
         callback = function()
             local cloud_storage = CloudStorage:new{}
@@ -222,14 +224,14 @@ function FileManagerMenu:setUpdateItemTable()
     }
 
     -- search tab
-    self.menu_items["find_book_in_calibre_catalog"] = {
+    self.menu_items.find_book_in_calibre_catalog = {
         text = _("Find a book in calibre catalog"),
         callback = function()
             Search:getCalibre()
             Search:ShowSearch()
         end
     }
-    self.menu_items["find_file"] = {
+    self.menu_items.find_file = {
         text = _("Find a file"),
         callback = function()
             FileSearcher:init(self.ui.file_chooser.path)
@@ -237,7 +239,7 @@ function FileManagerMenu:setUpdateItemTable()
     }
 
     -- main menu tab
-    self.menu_items["open_last_document"] = {
+    self.menu_items.open_last_document = {
         text = _("Open last document"),
         enabled_func = function()
             return G_reader_settings:readSetting("lastfile") ~= nil
@@ -260,7 +262,7 @@ function FileManagerMenu:setUpdateItemTable()
     for id, common_setting in pairs(require("ui/elements/common_info_menu_table")) do
         self.menu_items[id] = common_setting
     end
-    self.menu_items["exit"] = {
+    self.menu_items.exit = {
         text = _("Exit"),
         callback = function()
             if SetDefaults.settings_changed then

frontend/apps/reader/modules/readerbookmark.lua

@@ -45,7 +45,7 @@ end
 
 function ReaderBookmark:addToMainMenu(tab_item_table)
     -- insert table to main reader menu
-    self.ui.menu.menu_items["bookmarks"] = {
+    self.ui.menu.menu_items.bookmarks = {
         text = self.bm_menu_title,
         callback = function()
             self:onShowBookmark()

frontend/apps/reader/modules/readerdictionary.lua

@@ -24,7 +24,7 @@ function ReaderDictionary:init()
 end
 
 function ReaderDictionary:addToMainMenu(tab_item_table)
-    self.ui.menu.menu_items["dictionary_lookup"] = {
+    self.ui.menu.menu_items.dictionary_lookup = {
         text = _("Dictionary lookup"),
         tap_input = {
             title = _("Enter a word to look up"),

frontend/apps/reader/modules/readerfont.lua

@@ -225,7 +225,7 @@ end
 
 function ReaderFont:addToMainMenu(tab_item_table)
     -- insert table to main reader menu
-    self.ui.menu.menu_items["change_font"] = {
+    self.ui.menu.menu_items.change_font = {
         text = self.font_menu_title,
         sub_item_table = self.face_table,
     }

frontend/apps/reader/modules/readerfooter.lua

@@ -325,7 +325,7 @@ local option_titles = {
 
 function ReaderFooter:addToMainMenu(tab_item_table)
     local sub_items = {}
-    self.ui.menu.menu_items["status_bar"] = {
+    self.ui.menu.menu_items.status_bar = {
         text = _("Status bar"),
         sub_item_table = sub_items,
     }

frontend/apps/reader/modules/readergoto.lua

@@ -16,13 +16,13 @@ end
 
 function ReaderGoto:addToMainMenu(tab_item_table)
     -- insert goto command to main reader menu
-    self.ui.menu.menu_items["go_to"] = {
+    self.ui.menu.menu_items.go_to = {
         text = self.goto_menu_title,
         callback = function()
             self:onShowGotoDialog()
         end,
     }
-    self.ui.menu.menu_items["skim_to"] = {
+    self.ui.menu.menu_items.skim_to = {
         text = self.skim_menu_title,
         callback = function()
             self:onShowSkimtoDialog()

frontend/apps/reader/modules/readerhighlight.lua

@@ -66,7 +66,7 @@ end
 
 function ReaderHighlight:addToMainMenu(tab_item_table)
     -- insert table to main reader menu
-    self.ui.menu.menu_items["highlight_options"] = {
+    self.ui.menu.menu_items.highlight_options = {
         text = _("Highlight options"),
         sub_item_table = self:genHighlightDrawerMenu(),
     }

frontend/apps/reader/modules/readerhyphenation.lua

@@ -119,7 +119,7 @@ end
 
 function ReaderHyphenation:addToMainMenu(tab_item_table)
     -- insert table to main reader menu
-    self.ui.menu.menu_items["hyphenation"] = {
+    self.ui.menu.menu_items.hyphenation = {
         text = self.hyph_menu_title,
         sub_item_table = self.hyph_table,
     }

frontend/apps/reader/modules/readerlink.lua

@@ -69,7 +69,7 @@ end
 
 function ReaderLink:addToMainMenu(tab_item_table)
     -- insert table to main reader menu
-    self.ui.menu.menu_items["follow_links"] = {
+    self.ui.menu.menu_items.follow_links = {
         text = _("Follow links"),
         sub_item_table = {
             {

frontend/apps/reader/modules/readermenu.lua

@@ -14,46 +14,48 @@ local ReaderMenu = InputContainer:new{
 }
 
 function ReaderMenu:init()
-    self.menu_items["KOMenu:menu_buttons"] = {
-        -- top menu
-    }
-    -- items in top menu
-    self.menu_items["navi"] = {
-        icon = "resources/icons/appbar.page.corner.bookmark.png",
-    }
-    self.menu_items["typeset"] = {
-        icon = "resources/icons/appbar.page.text.png",
-    }
-    self.menu_items["setting"] = {
-        icon = "resources/icons/appbar.settings.png",
-    }
-    self.menu_items["tools"] = {
-        icon = "resources/icons/appbar.tools.png",
-    }
-    self.menu_items["search"] = {
-        icon = "resources/icons/appbar.magnify.browse.png",
-    }
-    self.menu_items["filemanager"] = {
-        icon = "resources/icons/appbar.cabinet.files.png",
-        remember = false,
-        callback = function()
-            self:onTapCloseMenu()
-            self.ui:onClose()
-            local FileManager = require("apps/filemanager/filemanager")
-            local lastdir = nil
-            local last_file = G_reader_settings:readSetting("lastfile")
-            if last_file then
-                lastdir = last_file:match("(.*)/")
-            end
-            if FileManager.instance then
-                FileManager.instance:reinit(lastdir)
-            else
-                FileManager:showFiles(lastdir)
-            end
-        end,
-    }
-    self.menu_items["main"] = {
-        icon = "resources/icons/menu-icon.png",
+    self.menu_items = {
+        ["KOMenu:menu_buttons"] = {
+            -- top menu
+        },
+        -- items in top menu
+        navi = {
+            icon = "resources/icons/appbar.page.corner.bookmark.png",
+        },
+        typeset = {
+            icon = "resources/icons/appbar.page.text.png",
+        },
+        setting = {
+            icon = "resources/icons/appbar.settings.png",
+        },
+        tools = {
+            icon = "resources/icons/appbar.tools.png",
+        },
+        search = {
+            icon = "resources/icons/appbar.magnify.browse.png",
+        },
+        filemanager = {
+            icon = "resources/icons/appbar.cabinet.files.png",
+            remember = false,
+            callback = function()
+                self:onTapCloseMenu()
+                self.ui:onClose()
+                local FileManager = require("apps/filemanager/filemanager")
+                local lastdir = nil
+                local last_file = G_reader_settings:readSetting("lastfile")
+                if last_file then
+                    lastdir = last_file:match("(.*)/")
+                end
+                if FileManager.instance then
+                    FileManager.instance:reinit(lastdir)
+                else
+                    FileManager:showFiles(lastdir)
+                end
+            end,
+        },
+        main = {
+            icon = "resources/icons/menu-icon.png",
+        }
     }
 
     self.registered_widgets = {}
@@ -103,7 +105,7 @@ function ReaderMenu:setUpdateItemTable()
     -- insert DjVu render mode submenu just before the last entry (show advanced)
     -- this is a bit of a hack
     if self.ui.document.is_djvu then
-        self.menu_items["djvu_render_mode"] = self.view:getRenderModeMenuTable()
+        self.menu_items.djvu_render_mode = self.view:getRenderModeMenuTable()
     end
 
     if Device:isKobo() and Screensaver:isUsingBookCover() then
@@ -113,7 +115,7 @@ function ReaderMenu:setUpdateItemTable()
         local proportional = function()
             return self.ui.doc_settings:readSetting("proportional_screensaver") or false
         end
-        self.menu_items["screensaver"] {
+        self.menu_items.screensaver {
             text = _("Screensaver"),
             sub_item_table = {
                 {
@@ -151,7 +153,7 @@ function ReaderMenu:setUpdateItemTable()
         self.menu_items[id] = common_setting
     end
 
-    self.menu_items["exit"] = {
+    self.menu_items.exit = {
         text = _("Exit"),
         callback = function()
             self:onTapCloseMenu()

frontend/apps/reader/modules/readerpaging.lua

@@ -201,7 +201,7 @@ function ReaderPaging:addToMainMenu(tab_item_table)
     for _, menu_entry in ipairs(self.view:genOverlapStyleMenu()) do
         table.insert(page_overlap_menu, menu_entry)
     end
-    self.ui.menu.menu_items["page_overlap"] = {
+    self.ui.menu.menu_items.page_overlap = {
         text = _("Page overlap"),
         enabled_func = function()
             return not self.view.page_scroll and self.zoom_mode ~= "page"
@@ -209,7 +209,7 @@ function ReaderPaging:addToMainMenu(tab_item_table)
         end,
         sub_item_table = page_overlap_menu,
     }
-    self.ui.menu.menu_items["read_from_right_to_left"] = {
+    self.ui.menu.menu_items.read_from_right_to_left = {
         text = _("Read from right to left"),
         checked_func = function() return self.inverse_reading_order end,
         callback = function()

frontend/apps/reader/modules/readerrolling.lua

@@ -258,7 +258,7 @@ function ReaderRolling:addToMainMenu(tab_item_table)
     for _, menu_entry in ipairs(self.view:genOverlapStyleMenu()) do
         table.insert(page_overlap_menu, menu_entry)
     end
-    self.ui.menu.menu_items["page_overlap"] = {
+    self.ui.menu.menu_items.page_overlap = {
         text = _("Page overlap"),
         enabled_func = function() return self.view.view_mode ~= "page" end,
         sub_item_table = page_overlap_menu,

frontend/apps/reader/modules/readersearch.lua

@@ -14,7 +14,7 @@ function ReaderSearch:init()
 end
 
 function ReaderSearch:addToMainMenu(tab_item_table)
-    self.ui.menu.menu_items["fulltext_search"] = {
+    self.ui.menu.menu_items.fulltext_search = {
         text = _("Fulltext search"),
         tap_input = {
             title = _("Input text to search for"),

frontend/apps/reader/modules/readerstatus.lua

@@ -33,7 +33,7 @@ function ReaderStatus:init()
 end
 
 function ReaderStatus:addToMainMenu(tab_item_table)
-    self.ui.menu.menu_items["book_status"] = {
+    self.ui.menu.menu_items.book_status = {
         text = _("Book status"),
         callback = function()
             self:showStatus()

frontend/apps/reader/modules/readertoc.lua

@@ -436,7 +436,7 @@ end
 
 function ReaderToc:addToMainMenu(tab_item_table)
     -- insert table to main reader menu
-    self.ui.menu.menu_items["table_of_contents"] = {
+    self.ui.menu.menu_items.table_of_contents = {
         text = self.toc_menu_title,
         callback = function()
             self:onShowToc()

frontend/apps/reader/modules/readertypeset.lua

@@ -142,11 +142,11 @@ end
 
 function ReaderTypeset:addToMainMenu(tab_item_table)
     -- insert table to main reader menu
-    self.ui.menu.menu_items["set_render_style"] = {
+    self.ui.menu.menu_items.set_render_style = {
         text = self.css_menu_title,
         sub_item_table = self:genStyleSheetMenu(),
     }
-    self.ui.menu.menu_items["floating_punctuation"] = {
+    self.ui.menu.menu_items.floating_punctuation = {
         text = _("Floating punctuation"),
         checked_func = function() return self.floating_punctuation == 1 end,
         callback = function()

frontend/apps/reader/modules/readerwikipedia.lua

@@ -49,7 +49,7 @@ function ReaderWikipedia:lookupInput()
 end
 
 function ReaderWikipedia:addToMainMenu(tab_item_table)
-    self.ui.menu.menu_items["wikipedia_lookup"] =  {
+    self.ui.menu.menu_items.wikipedia_lookup =  {
         text = _("Wikipedia lookup"),
         callback = function()
             if NetworkMgr:isOnline() then

frontend/apps/reader/modules/readerzooming.lua

@@ -310,7 +310,7 @@ end
 
 function ReaderZooming:addToMainMenu(tab_item_table)
     if self.ui.document.info.has_pages then
-        self.ui.menu.menu_items["switch_zoom_mode"] = {
+        self.ui.menu.menu_items.switch_zoom_mode = {
             text = _("Switch zoom mode"),
             enabled_func = function()
                 return self.ui.document.configurable.text_wrap ~= 1

frontend/ui/elements/common_info_menu_table.lua

@@ -8,9 +8,9 @@ local common_info = {}
 if Device:isKindle() or Device:isKobo() or Device:isPocketBook()
     or Device:isAndroid() then
     local OTAManager = require("ui/otamanager")
-    common_info["ota_update"] = OTAManager:getOTAMenuTable()
+    common_info.ota_update = OTAManager:getOTAMenuTable()
 end
-common_info["version"] = {
+common_info.version = {
     text = _("Version"),
     callback = function()
         UIManager:show(InfoMessage:new{
@@ -18,7 +18,7 @@ common_info["version"] = {
         })
     end
 }
-common_info["help"] = {
+common_info.help = {
     text = _("Help"),
     callback = function()
         UIManager:show(InfoMessage:new{

frontend/ui/elements/common_settings_menu_table.lua

@@ -9,7 +9,7 @@ local common_settings = {}
 
 if Device:hasFrontlight() then
     local ReaderFrontLight = require("apps/reader/modules/readerfrontlight")
-    common_settings["frontlight"] = {
+    common_settings.frontlight = {
         text = _("Frontlight"),
         callback = function()
             ReaderFrontLight:onShowFlDialog()
@@ -17,7 +17,7 @@ if Device:hasFrontlight() then
     }
 end
 
-common_settings["night_mode"] = {
+common_settings.night_mode = {
     text = _("Night mode"),
     checked_func = function() return G_reader_settings:readSetting("night_mode") end,
     callback = function()
@@ -27,11 +27,11 @@ common_settings["night_mode"] = {
         G_reader_settings:saveSetting("night_mode", not night_mode)
     end
 }
-common_settings["network"] = {
+common_settings.network = {
     text = _("Network"),
     sub_item_table = NetworkMgr:getMenuTable()
 }
-common_settings["screen"] = {
+common_settings.screen = {
     text = _("Screen"),
     sub_item_table = {
         require("ui/elements/screen_dpi_menu_table"),
@@ -40,7 +40,7 @@ common_settings["screen"] = {
         require("ui/elements/refresh_menu_table"),
     },
 }
-common_settings["save_document"] = {
+common_settings.save_document = {
     text = _("Save document"),
     sub_item_table = {
         {
@@ -75,8 +75,8 @@ common_settings["save_document"] = {
         },
     },
 }
-common_settings["language"] = Language:getLangMenuTable()
-common_settings["show_advanced_options"] = {
+common_settings.language = Language:getLangMenuTable()
+common_settings.show_advanced_options = {
     text = _("Show advanced options"),
     checked_func = function() return G_reader_settings:readSetting("show_advanced") end,
     callback = function()

frontend/ui/menusorter.lua

@@ -3,7 +3,6 @@ local DEBUG = require("dbg")
 
 local MenuSorter = {
     menu_table = {},
-    sub_menus = {},
     separator = {
         text = "KOMenu:separator",
     },
@@ -60,7 +59,7 @@ DEBUG(item_table, order)
 end
 
 function MenuSorter:magic(item_table, order)
-    local tmp_menu_table = {}
+    local sub_menus = {}
     -- the actual sorting of menu items
     for order_id, order_item in pairs (order) do
         DEBUG("order_id",order_id)
@@ -79,15 +78,13 @@ function MenuSorter:magic(item_table, order)
 
                 -- this is a submenu, mark it for later
                 if order[order_number_id] then
-                    table.insert(self.sub_menus, order_number_id)
+                    table.insert(sub_menus, order_number_id)
                     self.menu_table[order_id][order_number] = {
                         id = order_number_id,
                         --sub = true,
                     }
                 -- regular, just insert a menu action
                 else
-                    --self.menu_table[order_id] = tmp_menu_table[order_id]
-
                     if order_number_id == "----------------------------" then
                         -- it's a separator
                         self.menu_table[order_id][order_number] = self.separator
@@ -107,8 +104,8 @@ function MenuSorter:magic(item_table, order)
     
     -- now do the submenus
     DEBUG("SUBMENUUUUUUUUUUUUUUUUUUUUUUUUUUUUUUUUUUUUUUUUUUUUUUUUUUUUUUUUUUUUUUUUUUUUUUUUUUUUUUUUUUUUUS")
-    DEBUG("self.sub_menus", self.sub_menus)
-    for i,sub_menu in ipairs(self.sub_menus) do
+    DEBUG("self.sub_menus", sub_menus)
+    for i,sub_menu in ipairs(sub_menus) do
         self.sub_menu_position = {}
         self:findById(self.menu_table["KOMenu:menu_buttons"], sub_menu)
         if self.sub_menu_position and self.sub_menu_position.id then
@@ -122,16 +119,19 @@ function MenuSorter:magic(item_table, order)
         self.menu_table["KOMenu:menu_buttons"][i] = self.menu_table["KOMenu:menu_buttons"][i].sub_item_table
     end
     
-    
 end
 
-function MenuSorter:findById(tbl, needle_id, result)
+function MenuSorter:findById(tbl, needle_id)
 
 
 --DEBUG("TBL given",tbl)
     for k,v in pairs(tbl) do
         if #self.sub_menu_position == 1 then
-            break
+            if self.sub_menu_position.id == needle_id then
+                return self.sub_menu_position
+            else
+                self.sub_menu_position = nil
+            end
         end
         --DEBUG("FINDBYID:", needle_id, "current:", k,v)
 
@@ -139,7 +139,7 @@ function MenuSorter:findById(tbl, needle_id, result)
             if v.id == needle_id then
                 DEBUG("FOUND IT FOUND IT FOUND IT FOUND IT FOUND IT FOUND IT FOUND IT FOUND IT FOUND IT FOUND IT ", v.id)
                 self.sub_menu_position = v
-                break
+                return self.sub_menu_position
             elseif type(v) == "table" and v.id then
                 DEBUG("GOING DEEPER", v.id)
                 self:findById(v, needle_id)

spec/unit/readerfooter_spec.lua

@@ -1,31 +1,31 @@
 describe("Readerfooter module", function()
-    local DocumentRegistry, ReaderUI, DocSettings, UIManager, DEBUG
+    local DocumentRegistry, ReaderUI, MenuSorter, DocSettings, UIManager, DEBUG
     local purgeDir, Screen
 
-    local function tapFooterMenu(menu_tab_items, menu_title)
-        for _, item in ipairs(menu_tab_items.setting) do
-            if item.text == "Status bar" then
-                for _, subitem in ipairs(item.sub_item_table) do
-                    if subitem.text == menu_title then
-                        subitem.callback()
-                        return
-                    end
-                end
-                error('Menu item not found: "' .. menu_title .. '"!')
-            end
-        end
-        error('Menu item not found: "Status bar"!')
-    end
-
     setup(function()
         require("commonrequire")
         DocumentRegistry = require("document/documentregistry")
         ReaderUI = require("apps/reader/readerui")
-        DocSettings = require("docsettings")
+        ReaderUI = require("apps/reader/readerui")
         UIManager = require("ui/uimanager")
+        MenuSorter = require("ui/menusorter")
         DEBUG = require("dbg")
         purgeDir = require("ffi/util").purgeDir
         Screen = require("device").screen
+
+        function tapFooterMenu(menu_tab_items, menu_title)
+            local status_bar = MenuSorter:findById(menu_tab_items, "status_bar") or nil
+            if status_bar then
+                for _, subitem in ipairs(status_bar) do
+                    if subitem.text == menu_title then
+                        subitem.callback()
+                        return
+                    end
+                end
+                error('Menu item not found: "' .. menu_title .. '"!')
+            end
+            error('Menu item not found: "Status bar"!')
+        end
     end)
 
     before_each(function()