From da65ac8b02e29bcdf9379f29e891d1dbb26274e8 Mon Sep 17 00:00:00 2001 From: NiLuJe Date: Wed, 12 Oct 2022 19:59:48 +0200 Subject: [PATCH] Cleanup various varargs shenanigans (#9624) * Iterate over varargs directly via select if possible * Use table.pack otherwise (https://github.com/koreader/koreader-base/pull/1535). * This allows us to simplify a few Logger calls, as logger now handles nil values. --- .luacheckrc | 2 ++ base | 2 +- doc/Events.md | 2 +- frontend/apps/reader/modules/readerrolling.lua | 15 ++++++++------- frontend/apps/reader/modules/readerthumbnail.lua | 3 ++- frontend/dbg.lua | 5 ++--- frontend/device/pocketbook/device.lua | 10 ++++++---- frontend/logger.lua | 6 ++++-- frontend/ui/data/keyboardlayouts/ja_keyboard.lua | 2 +- frontend/ui/event.lua | 5 +---- frontend/ui/trapper.lua | 5 ++--- frontend/ui/uimanager.lua | 15 ++++++++------- frontend/ui/widget/eventlistener.lua | 2 +- frontend/util.lua | 6 ------ platform/android/luajit-launcher | 2 +- plugins/japanese.koplugin/deinflector.lua | 3 ++- plugins/opds.koplugin/opdsbrowser.lua | 4 ++-- 17 files changed, 44 insertions(+), 45 deletions(-) diff --git a/.luacheckrc b/.luacheckrc index 1fa4ee1ab..3d792c876 100644 --- a/.luacheckrc +++ b/.luacheckrc @@ -6,6 +6,8 @@ self = false globals = { "G_reader_settings", "G_defaults", + "table.pack", + "table.unpack", } read_globals = { diff --git a/base b/base index 661e46790..4216c40d6 160000 --- a/base +++ b/base @@ -1 +1 @@ -Subproject commit 661e46790dbc74795deae5c3b00c04fa13bc14c9 +Subproject commit 4216c40d662660c9bbc48e79893b437e97518254 diff --git a/doc/Events.md b/doc/Events.md index b9816d84f..e904e30c2 100644 --- a/doc/Events.md +++ b/doc/Events.md @@ -41,7 +41,7 @@ for _, widget in ipairs(self) do end end -- If not consumed by children, consume it ourself -return self["on"..event.name](self, unpack(event.args, 1, event.argc)) +return self["on"..event.name](self, unpack(event.args)) ``` ## Event system diff --git a/frontend/apps/reader/modules/readerrolling.lua b/frontend/apps/reader/modules/readerrolling.lua index c75b04bff..32e08c943 100644 --- a/frontend/apps/reader/modules/readerrolling.lua +++ b/frontend/apps/reader/modules/readerrolling.lua @@ -1145,34 +1145,35 @@ function ReaderRolling:updateBatteryState() end function ReaderRolling:handleEngineCallback(ev, ...) - local args = {...} - -- logger.info("handleCallback: got", ev, args and #args > 0 and args[1] or nil) + local argc = select("#", ...) + local arg = argc > 0 and select(1, ...) + -- logger.info("handleCallback: got", ev, ...) if ev == "OnLoadFileStart" then -- Start of book loading self:showEngineProgress(0) -- Start initial delay countdown elseif ev == "OnLoadFileProgress" then -- Initial load from file (step 1/2) or from cache (step 1/1) - self:showEngineProgress(args[1]*(1/100/2)) + self:showEngineProgress(arg*(1/100/2)) elseif ev == "OnNodeStylesUpdateStart" then -- Start of re-rendering self:showEngineProgress(0) -- Start initial delay countdown elseif ev == "OnNodeStylesUpdateProgress" then -- Update node styles (step 1/2 on re-rendering) - self:showEngineProgress(args[1]*(1/100/2)) + self:showEngineProgress(arg*(1/100/2)) elseif ev == "OnFormatStart" then -- Start of step 2/2 self:showEngineProgress(1/2) -- 50%, in case of no OnFormatProgress elseif ev == "OnFormatProgress" then -- Paragraph formatting and page splitting (step 2/2 after load -- from file, step 2/2 on re-rendering) - self:showEngineProgress(1/2 + args[1]*(1/100/2)) + self:showEngineProgress(1/2 + arg*(1/100/2)) elseif ev == "OnSaveCacheFileStart" then -- Start of cache file save self:showEngineProgress(1) -- Start initial delay countdown, fully filled elseif ev == "OnSaveCacheFileProgress" then -- Cache file save (when closing book after initial load from -- file or re-rendering) - self:showEngineProgress(1 - args[1]*(1/100)) -- unfill progress + self:showEngineProgress(1 - arg*(1/100)) -- unfill progress elseif ev == "OnDocumentReady" or ev == "OnSaveCacheFileEnd" then self:showEngineProgress() -- cleanup elseif ev == "OnLoadFileError" then - logger.warn("Cre error loading file:", args[1]) + logger.warn("Cre error loading file:", arg) end -- ignore other events end diff --git a/frontend/apps/reader/modules/readerthumbnail.lua b/frontend/apps/reader/modules/readerthumbnail.lua index e959b7750..4c32d2868 100644 --- a/frontend/apps/reader/modules/readerthumbnail.lua +++ b/frontend/apps/reader/modules/readerthumbnail.lua @@ -182,7 +182,8 @@ end function ReaderThumbnail:resetCachedPagesForBookmarks(...) -- Multiple bookmarks may be provided local start_page, end_page - for _, bm in ipairs({...}) do + for i = 1, select("#", ...) do + local bm = select(i, ...) if self.ui.rolling then -- Look at all properties that may be xpointers for _, k in ipairs({"page", "pos0", "pos1"}) do diff --git a/frontend/dbg.lua b/frontend/dbg.lua index 2fca467a0..06a77665a 100644 --- a/frontend/dbg.lua +++ b/frontend/dbg.lua @@ -19,7 +19,6 @@ These functions don't do anything when debugging is turned off. --]]-- local logger = require("logger") -local util = require("util") local Dbg = { -- set to nil so first debug:turnOff call won't be skipped @@ -46,11 +45,11 @@ function Dbg:turnOn() if pre_guard then pre_guard(...) end - local values = util.table_pack(old_method(...)) + local values = table.pack(old_method(...)) if post_guard then post_guard(...) end - return unpack(values, 1, values.n) + return unpack(values) end end --- Use this instead of a regular Lua @{assert}(). diff --git a/frontend/device/pocketbook/device.lua b/frontend/device/pocketbook/device.lua index 00b756ab0..951390a78 100644 --- a/frontend/device/pocketbook/device.lua +++ b/frontend/device/pocketbook/device.lua @@ -261,13 +261,15 @@ function PocketBook:init() end function PocketBook:notifyBookState(title, document) - local fn = document and document.file or nil - logger.dbg("Notify book state", title or "[nil]", fn or "[nil]") + local fn = document and document.file + logger.dbg("Notify book state", title, fn) os.remove("/tmp/.current") if fn then local fo = io.open("/tmp/.current", "w+") - fo:write(fn) - fo:close() + if fo then + fo:write(fn) + fo:close() + end end inkview.SetSubtaskInfo(inkview.GetCurrentTask(), 0, title and (title .. " - koreader") or "koreader", fn or _("N/A")) end diff --git a/frontend/logger.lua b/frontend/logger.lua index f712e2acd..0c0c26897 100644 --- a/frontend/logger.lua +++ b/frontend/logger.lua @@ -57,7 +57,8 @@ if isAndroid then log = function(log_lvl, ...) local line = {} - for _, v in ipairs({...}) do + for i = 1, select("#", ...) do + local v = select(i, ...) if type(v) == "table" then table.insert(line, serpent.block(v, serpent_opts)) else @@ -72,7 +73,8 @@ else os.date("%x-%X"), LOG_PREFIX[log_lvl], } - for _, v in ipairs({...}) do + for i = 1, select("#", ...) do + local v = select(i, ...) if type(v) == "table" then table.insert(line, serpent.block(v, serpent_opts)) else diff --git a/frontend/ui/data/keyboardlayouts/ja_keyboard.lua b/frontend/ui/data/keyboardlayouts/ja_keyboard.lua index 881655477..63454d3fc 100644 --- a/frontend/ui/data/keyboardlayouts/ja_keyboard.lua +++ b/frontend/ui/data/keyboardlayouts/ja_keyboard.lua @@ -72,7 +72,7 @@ local function wrappedAddChars(inputbox, char) end -- Replace character if there was a valid replacement. - logger.dbg("ja_kbd: applying", char, "key to", current_char, "yielded", new_char or "") + logger.dbg("ja_kbd: applying", char, "key to", current_char, "yielded", new_char) if not current_char then return end -- no character to modify if new_char then -- Use the raw methods to avoid calling the callbacks. diff --git a/frontend/ui/event.lua b/frontend/ui/event.lua index 946178456..3e042eb2a 100644 --- a/frontend/ui/event.lua +++ b/frontend/ui/event.lua @@ -30,10 +30,7 @@ Event:new("GotoPage", 1) function Event:new(name, ...) local o = { handler = "on"..name, - -- Minor trickery to handle nils, c.f., http://lua-users.org/wiki/VarargTheSecondClassCitizen - --- @fixme: Move to table.pack() (which stores the count in the field `n`) here & table.unpack() in @{ui.widget.eventlistener|EventListener} once we build LuaJIT w/ 5.2 compat. - argc = select("#", ...), - args = {...} + args = table.pack(...), } setmetatable(o, self) self.__index = self diff --git a/frontend/ui/trapper.lua b/frontend/ui/trapper.lua index 6ee43386a..ffd91c363 100644 --- a/frontend/ui/trapper.lua +++ b/frontend/ui/trapper.lua @@ -16,7 +16,6 @@ local UIManager = require("ui/uimanager") local buffer = require("string.buffer") local ffiutil = require("ffi/util") local logger = require("logger") -local util = require("util") local _ = require("gettext") local Trapper = {} @@ -558,7 +557,7 @@ function Trapper:dismissableRunInSubprocess(task, trap_widget_or_string, task_re -- task may return complex data structures, that we serialize. -- NOTE: LuaJIT's serializer currently doesn't support: -- functions, coroutines, non-numerical FFI cdata & full userdata. - local results = util.table_pack(task()) + local results = table.pack(task()) local ok, str = pcall(buffer.encode, results) if not ok then logger.warn("cannot serialize", tostring(results), "->", str) @@ -675,7 +674,7 @@ function Trapper:dismissableRunInSubprocess(task, trap_widget_or_string, task_re if task_returns_simple_string then return completed, ret_values else - return completed, unpack(ret_values, 1, ret_values.n) + return completed, unpack(ret_values) end end return completed diff --git a/frontend/ui/uimanager.lua b/frontend/ui/uimanager.lua index 38865ac62..325dda358 100644 --- a/frontend/ui/uimanager.lua +++ b/frontend/ui/uimanager.lua @@ -247,8 +247,7 @@ function UIManager:schedule(sched_time, action, ...) table.insert(self._task_queue, lo, { time = sched_time, action = action, - argc = select("#", ...), - args = {...}, + args = table.pack(...), }) self._task_queue_dirty = true end @@ -474,7 +473,9 @@ UIManager:setDirty(self.widget, function() return "ui", self.someelement.dimen e @bool refreshdither `true` if widget requires dithering (optional) ]] function UIManager:setDirty(widget, refreshtype, refreshregion, refreshdither) + local widget_name if widget then + widget_name = widget.name or widget.id or tostring(widget) if widget == "all" then -- special case: set all top-level widgets as being "dirty". for _, window in ipairs(self._window_stack) do @@ -500,7 +501,7 @@ function UIManager:setDirty(widget, refreshtype, refreshregion, refreshdither) local w = self._window_stack[i].widget if handle_alpha then self._dirty[w] = true - logger.dbg("setDirty: Marking as dirty widget:", w.name or w.id or tostring(w), "because it's below translucent widget:", widget.name or widget.id or tostring(widget)) + logger.dbg("setDirty: Marking as dirty widget:", w.name or w.id or tostring(w), "because it's below translucent widget:", widget_name) -- Stop flagging widgets at the uppermost one that covers the full screen if w.covers_fullscreen then break @@ -548,16 +549,16 @@ function UIManager:setDirty(widget, refreshtype, refreshregion, refreshdither) -- NOTE: It's too early to tell what the function will return (especially the region), because the widget hasn't been painted yet. -- Consuming the lambda now also appears have nasty side-effects that render it useless later, subtly breaking a whole lot of things... -- Thankfully, we can track them in _refresh()'s logging very soon after that... - logger.dbg("setDirty via a func from widget", widget and (widget.name or widget.id or tostring(widget)) or "nil") + logger.dbg("setDirty via a func from widget", widget_name) end else -- otherwise, enqueue refresh self:_refresh(refreshtype, refreshregion, refreshdither) if dbg.is_on then if refreshregion then - logger.dbg("setDirty", refreshtype and refreshtype or "nil", "from widget", widget and (widget.name or widget.id or tostring(widget)) or "nil", "w/ region", refreshregion.x, refreshregion.y, refreshregion.w, refreshregion.h, refreshdither and "AND w/ HW dithering" or "") + logger.dbg("setDirty", refreshtype, "from widget", widget_name, "w/ region", refreshregion.x, refreshregion.y, refreshregion.w, refreshregion.h, refreshdither and "AND w/ HW dithering" or "") else - logger.dbg("setDirty", refreshtype and refreshtype or "nil", "from widget", widget and (widget.name or widget.id or tostring(widget)) or "nil", "w/ NO region", refreshdither and "AND w/ HW dithering" or "") + logger.dbg("setDirty", refreshtype, "from widget", widget_name, "w/ NO region", refreshdither and "AND w/ HW dithering" or "") end end end @@ -895,7 +896,7 @@ function UIManager:_checkTasks() -- NOTE: Said task's action might modify _task_queue. -- To avoid race conditions and catch new upcoming tasks during this call, -- we repeatedly check the head of the queue (c.f., #1758). - task.action(unpack(task.args, 1, task.argc)) + task.action(unpack(task.args)) else -- As the queue is sorted in ascending order, it's safe to assume all items are currently future tasks. wait_until = task_time diff --git a/frontend/ui/widget/eventlistener.lua b/frontend/ui/widget/eventlistener.lua index f4cb768a3..9d6a2b183 100644 --- a/frontend/ui/widget/eventlistener.lua +++ b/frontend/ui/widget/eventlistener.lua @@ -34,7 +34,7 @@ By default, it's `"on"..Event.name`. function EventListener:handleEvent(event) if self[event.handler] then --print("EventListener:handleEvent:", event.handler, "handled by", debug.getinfo(self[event.handler], "S").short_src, self) - return self[event.handler](self, unpack(event.args, 1, event.argc)) + return self[event.handler](self, unpack(event.args)) end end diff --git a/frontend/util.lua b/frontend/util.lua index 2a35ea402..65b8d5313 100644 --- a/frontend/util.lua +++ b/frontend/util.lua @@ -1535,12 +1535,6 @@ local WrappedFunction_mt = { end, } ---- Helper function to help dealing with nils in varargs, since we don't have table.pack w/o Lua 5.2 compat... ---- Unpack w/ unpack(t, 1, t.n) -function util.table_pack(...) - return { n = select("#", ...), ... } -end - --- Wrap (or replace) a table method with a custom method, in a revertable way. -- This allows you extend the features of an existing module by modifying its -- internal methods, and then revert them back to normal later if necessary. diff --git a/platform/android/luajit-launcher b/platform/android/luajit-launcher index 76ba54461..2228f9309 160000 --- a/platform/android/luajit-launcher +++ b/platform/android/luajit-launcher @@ -1 +1 @@ -Subproject commit 76ba5446108112a81de38b2aad9a271440eace2c +Subproject commit 2228f930992cdf76b49a843eb872a43a1bf565d8 diff --git a/plugins/japanese.koplugin/deinflector.lua b/plugins/japanese.koplugin/deinflector.lua index eb1a8018e..5dc79fc14 100644 --- a/plugins/japanese.koplugin/deinflector.lua +++ b/plugins/japanese.koplugin/deinflector.lua @@ -38,7 +38,8 @@ local RULE_TYPES = { local function toRuleTypes(...) local final = 0 - for _, ruleType in ipairs({...}) do + for i = 1, select("#", ...) do + local ruleType = select(i, ...) if RULE_TYPES[ruleType] then final = bit.bor(final, RULE_TYPES[ruleType]) end diff --git a/plugins/opds.koplugin/opdsbrowser.lua b/plugins/opds.koplugin/opdsbrowser.lua index 68ecae613..47d0854e2 100644 --- a/plugins/opds.koplugin/opdsbrowser.lua +++ b/plugins/opds.koplugin/opdsbrowser.lua @@ -393,7 +393,7 @@ end function OPDSBrowser:getCatalog(item_url, username, password) local ok, catalog = pcall(self.parseFeed, self, item_url, username, password) if not ok and catalog then - logger.info("Cannot get catalog info from", item_url or "nil", catalog) + logger.info("Cannot get catalog info from", item_url, catalog) UIManager:show(InfoMessage:new{ text = T(_("Cannot get catalog info from %1"), (item_url and BD.url(item_url) or "nil")), }) @@ -882,7 +882,7 @@ function OPDSBrowser:showDownloads(item) end function OPDSBrowser:browse(browse_url, username, password) - logger.dbg("Browse OPDS url", browse_url or "nil") + logger.dbg("Browse OPDS url", browse_url) table.insert(self.paths, { url = browse_url, username = username,