From 3b1e704a374eadd6510b885aaa8a3e886fbace8d Mon Sep 17 00:00:00 2001 From: Frans de Jonge Date: Fri, 24 Mar 2017 21:15:35 +0100 Subject: [PATCH] MenuSorter: resolve review comments --- frontend/apps/filemanager/filemanagermenu.lua | 2 +- frontend/apps/reader/modules/readermenu.lua | 2 +- frontend/ui/menusorter.lua | 83 ++++++------------- spec/unit/menusorter_spec.lua | 36 +++++++- 4 files changed, 60 insertions(+), 63 deletions(-) diff --git a/frontend/apps/filemanager/filemanagermenu.lua b/frontend/apps/filemanager/filemanagermenu.lua index 1f4605e1b682..b3af3dc0f1b4 100644 --- a/frontend/apps/filemanager/filemanagermenu.lua +++ b/frontend/apps/filemanager/filemanagermenu.lua @@ -280,7 +280,7 @@ function FileManagerMenu:setUpdateItemTable() local order = require("ui/elements/filemanager_menu_order") local MenuSorter = require("frontend/ui/menusorter") - self.tab_item_table = MenuSorter:sort(self.menu_items, order, "filemanager") + self.tab_item_table = MenuSorter:mergeAndSort("filemanager", self.menu_items, order) end function FileManagerMenu:onShowMenu() diff --git a/frontend/apps/reader/modules/readermenu.lua b/frontend/apps/reader/modules/readermenu.lua index 1f20f7c6cafe..9196b4e4337b 100644 --- a/frontend/apps/reader/modules/readermenu.lua +++ b/frontend/apps/reader/modules/readermenu.lua @@ -168,7 +168,7 @@ function ReaderMenu:setUpdateItemTable() local order = require("frontend/ui/elements/reader_menu_order") local MenuSorter = require("frontend/ui/menusorter") - self.tab_item_table = MenuSorter:sort(self.menu_items, order, "reader") + self.tab_item_table = MenuSorter:mergeAndSort("reader", self.menu_items, order) end function ReaderMenu:onShowReaderMenu() diff --git a/frontend/ui/menusorter.lua b/frontend/ui/menusorter.lua index 5f613e0ccc07..5858a10c2cfa 100644 --- a/frontend/ui/menusorter.lua +++ b/frontend/ui/menusorter.lua @@ -4,12 +4,12 @@ menu_items and a separate menu order. ]] local DataStorage = require("datastorage") +local lfs = require("libs/libkoreader-lfs") local util = require("util") local DEBUG = require("dbg") local _ = require("gettext") local MenuSorter = { - menu_table = {}, orphaned_prefix = _("NEW: "), separator = { id = "----------------------------", @@ -26,68 +26,39 @@ end function MenuSorter:readMSSettings(table, config_prefix) if config_prefix then - local config_prefix = config_prefix.."_" - local menu_order = DataStorage:getSettingsDir().."/"..config_prefix.."menu_order" + local menu_order = string.format( + "%s/%s_menu_order", DataStorage:getSettingsDir(), config_prefix) - if file_exists(menu_order..".lua") then + if lfs.attributes(menu_order..".lua") then return require(menu_order) or {} - else - return {} end - else - return {} end + return {} end -function MenuSorter:sort(item_table, order, config_prefix) -DEBUG(item_table, order) - --local menu_table = {} - --local separator = { - --text = "KOMenu:separator", - --} - DEBUG("menu before user order", order) - -- take care of user customizations +function MenuSorter:mergeAndSort(config_prefix, item_table, order) local user_order = self:readMSSettings(item_table_name, config_prefix) - if user_order then - for user_order_id,user_order_item in pairs(user_order) do - for order_id, order_item in pairs (order) do - if user_order_id == order_id then - order[order_id] = user_order[order_id] - end + for user_order_id,user_order_item in pairs(user_order) do + for order_id, order_item in pairs (order) do + if user_order_id == order_id then + order[order_id] = user_order[order_id] end end end - DEBUG("menu after user order", order) - - --self.menu_table = self:magic(item_table, order) - self:magic(item_table, order) - DEBUG("after sort",self.menu_table["KOMenu:menu_buttons"]) - - - - -- deal with leftovers - - return self.menu_table["KOMenu:menu_buttons"] + return self:sort(item_table, order) end -function MenuSorter:magic(item_table, order) +function MenuSorter:sort(item_table, order) + local 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) - DEBUG("order_item",order_item) - DEBUG("item_table[order_id]",item_table[order_id]) -- user might define non-existing menu item if item_table[order_id] ~= nil then local tmp_menu_table = {} - self.menu_table[order_id] = item_table[order_id] - --self.menu_table[order_id] = item_table[order_id] - self.menu_table[order_id].id = order_id - --item_table[order_id].processed = true - DEBUG("tmp_menu_table[order_id]",tmp_menu_table[order_id]) + menu_table[order_id] = item_table[order_id] + menu_table[order_id].id = order_id for order_number,order_number_id in ipairs(order_item) do - DEBUG("order_number,order_number_id", order_number,order_number_id) - -- this is a submenu, mark it for later if order[order_number_id] then table.insert(sub_menus, order_number_id) @@ -119,10 +90,10 @@ function MenuSorter:magic(item_table, order) if v then if v.id == "----------------------------" then new_index = new_index - 1 - self.menu_table[order_id][new_index].separator = true + menu_table[order_id][new_index].separator = true else -- fix the index - self.menu_table[order_id][new_index] = tmp_menu_table[i] + menu_table[order_id][new_index] = tmp_menu_table[i] end new_index = new_index + 1 @@ -135,26 +106,23 @@ function MenuSorter:magic(item_table, order) end -- now do the submenus - DEBUG("SUBMENUUUUUUUUUUUUUUUUUUUUUUUUUUUUUUUUUUUUUUUUUUUUUUUUUUUUUUUUUUUUUUUUUUUUUUUUUUUUUUUUUUUUUS") - DEBUG("self.sub_menus", sub_menus) for i,sub_menu in ipairs(sub_menus) do - local sub_menu_position = self:findById(self.menu_table["KOMenu:menu_buttons"], sub_menu) or nil + local sub_menu_position = self:findById(menu_table["KOMenu:menu_buttons"], sub_menu) or nil if sub_menu_position and sub_menu_position.id then - sub_menu_position.sub_item_table = self.menu_table[sub_menu] + sub_menu_position.sub_item_table = menu_table[sub_menu] -- remove reference from top level output - self.menu_table[sub_menu] = nil + menu_table[sub_menu] = nil -- remove reference from input so it won't show up as orphaned item_table[sub_menu] = nil end end -- @TODO avoid this extra mini-loop -- cleanup, top-level items shouldn't have sub_item_table - for i,top_menu in ipairs(self.menu_table["KOMenu:menu_buttons"]) do - self.menu_table["KOMenu:menu_buttons"][i] = self.menu_table["KOMenu:menu_buttons"][i].sub_item_table + for i,top_menu in ipairs(menu_table["KOMenu:menu_buttons"]) do + menu_table["KOMenu:menu_buttons"][i] = menu_table["KOMenu:menu_buttons"][i].sub_item_table end -- handle disabled - DEBUG("MenuSorter: order.KOMenu_disabled", order.KOMenu_disabled) if order.KOMenu__disabled then for _,item in ipairs(order.KOMenu_disabled) do if item_table[item] then @@ -167,9 +135,7 @@ function MenuSorter:magic(item_table, order) -- remove top level reference before orphan handling item_table["KOMenu:menu_buttons"] = nil --attach orphans based on menu_hint - DEBUG("ORPHAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAANSS", util.tableSize(item_table)) for k,v in pairs(item_table) do - DEBUG(k) -- normally there should be menu text but check to be sure if v.text and v.new ~= true then v.id = k @@ -177,8 +143,9 @@ function MenuSorter:magic(item_table, order) -- prevent text being prepended to item on menu reload, i.e., on switching between reader and filemanager v.new = true end - table.insert(self.menu_table["KOMenu:menu_buttons"][1], v) + table.insert(menu_table["KOMenu:menu_buttons"][1], v) end + return menu_table["KOMenu:menu_buttons"] end --- Returns a menu item by ID. @@ -197,10 +164,8 @@ function MenuSorter:findById(tbl, needle_id) while k do if type(k) == "number" or k == "sub_item_table" then 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) return v elseif type(v) == "table" and v.id then - DEBUG("GOING DEEPER", v.id) table.insert(items, v) end end diff --git a/spec/unit/menusorter_spec.lua b/spec/unit/menusorter_spec.lua index 4357a83841c1..cb1aecfce728 100644 --- a/spec/unit/menusorter_spec.lua +++ b/spec/unit/menusorter_spec.lua @@ -81,12 +81,44 @@ describe("MenuSorter module", function() end end) it("should attach separator=true to previous item", function() + local menu_items = { + ["KOMenu:menu_buttons"] = {}, + first = {}, + second = {}, + third1 = {}, + third2 = {}, + } + local order = { + ["KOMenu:menu_buttons"] = {"first",}, + first = {"second"}, + second = {"third1", "----------------------------", "third2"}, + } + local test_menu = MenuSorter:sort(menu_items, order) + + assert(test_menu[1].id == "first") + assert(test_menu[1][1].id == "second") + assert(test_menu[1][2][1].id == "third1" and test_menu[1][2][1].separator == true) + assert(test_menu[1][2][2].id == "third2") end) it("should compress menus when items from order are missing", function() + local menu_items = { + ["KOMenu:menu_buttons"] = {}, + first = {}, + second = {}, + third2 = {}, + third4 = {}, + } + local order = { + ["KOMenu:menu_buttons"] = {"first",}, + first = {"second", "third1", "third2", "third3", "third4"}, + } - end) - it("should allow for attaching menu items multiple times", function() + local test_menu = MenuSorter:sort(menu_items, order) + assert(test_menu[1].id == "first") + assert(test_menu[1][1].id == "second") + assert(test_menu[1][2].id == "third2") + assert(test_menu[1][3].id == "third4") end) end)