From c9ff0071e384d3ab7491fc4c4d6da967f4d643de Mon Sep 17 00:00:00 2001 From: NiLuJe Date: Sun, 9 Oct 2022 19:32:35 +0200 Subject: [PATCH] DocSettings: Fix candidates sorting (#9607) The candidates array had a very very very high chance of being sparse, which effectively broke the logic. It was mostly harmless since said logic is only here for a long and varied history of backwards compatibility ;). --- frontend/docsettings.lua | 106 ++++++++++++++++++++++----------------- frontend/luadata.lua | 8 +-- frontend/luadefaults.lua | 6 +-- frontend/luasettings.lua | 6 +-- 4 files changed, 71 insertions(+), 55 deletions(-) diff --git a/frontend/docsettings.lua b/frontend/docsettings.lua index 618e69374..2514ba667 100644 --- a/frontend/docsettings.lua +++ b/frontend/docsettings.lua @@ -16,13 +16,31 @@ local DocSettings = LuaSettings:extend{} local HISTORY_DIR = DataStorage:getHistoryDir() -local function buildCandidate(file_path) - -- Ignore empty files. - if file_path and lfs.attributes(file_path, "mode") == "file" then - return { file_path, lfs.attributes(file_path, "modification") } - else - return nil +local function buildCandidates(list) + local candidates = {} + + for i, file_path in ipairs(list) do + -- Ignore missing files. + if file_path ~= "" and lfs.attributes(file_path, "mode") == "file" then + table.insert(candidates, { + path = file_path, + mtime = lfs.attributes(file_path, "modification"), + prio = i, + } + ) + end end + + -- MRU sort, tie breaker is insertion order (higher priority locations were inserted first). + table.sort(candidates, function(l, r) + if l.mtime == r.mtime then + return l.prio < r.prio + else + return l.mtime > r.mtime + end + end) + + return candidates end --- Returns path to sidecar directory (`filename.sdr`). @@ -61,7 +79,7 @@ function DocSettings:hasSidecarFile(doc_path) end function DocSettings:getHistoryPath(fullpath) - return HISTORY_DIR .. "/[" .. fullpath:gsub("(.*/)([^/]+)","%1] %2"):gsub("/","#") .. ".lua" + return HISTORY_DIR .. "/[" .. fullpath:gsub("(.*/)([^/]+)", "%1] %2"):gsub("/", "#") .. ".lua" end function DocSettings:getPathFromHistory(hist_name) @@ -72,7 +90,7 @@ function DocSettings:getPathFromHistory(hist_name) if s == nil or s == '' then return '' end -- 2. crop the bracket-sign from both sides -- 3. and finally replace decorative signs '#' to dir-char '/' - return string.gsub(string.sub(s,2,-3),"#","/") + return string.gsub(string.sub(s, 2, -3), "#", "/") end function DocSettings:getNameFromHistory(hist_name) @@ -121,42 +139,39 @@ function DocSettings:open(docfile) ffiutil.basename(docfile)..".lua" end - local candidates = {} - -- New sidecar file - table.insert(candidates, buildCandidate(new.sidecar_file)) - -- Backup file of new sidecar file - table.insert(candidates, buildCandidate(new.sidecar_file and (new.sidecar_file .. ".old"))) - -- Legacy sidecar file - table.insert(candidates, buildCandidate(new.legacy_sidecar_file)) - -- Legacy history folder - table.insert(candidates, buildCandidate(new.history_file)) - -- Backup file in legacy history folder - table.insert(candidates, buildCandidate(new.history_file .. ".old")) - -- Legacy kpdfview setting - table.insert(candidates, buildCandidate(docfile..".kpdfview.lua")) - table.sort(candidates, function(l, r) - if l == nil then - return false - elseif r == nil then - return true - else - return l[2] > r[2] - end - end) + -- Candidates list, in order of priority: + local candidates_list = { + -- New sidecar file + new.sidecar_file or "", + -- Backup file of new sidecar file + new.sidecar_file and (new.sidecar_file .. ".old") or "", + -- Legacy sidecar file + new.legacy_sidecar_file or "", + -- Legacy history folder + new.history_file, + -- Backup file in legacy history folder + new.history_file .. ".old", + -- Legacy kpdfview setting + docfile..".kpdfview.lua", + } + -- We get back an array of tables for *existing* candidates, sorted MRU first (insertion order breaks ties). + local candidates = buildCandidates(candidates_list) + local ok, stored, filepath - for _, k in ipairs(candidates) do + for _, t in ipairs(candidates) do + local candidate_path = t.path -- Ignore empty files - if lfs.attributes(k[1], "size") > 0 then - ok, stored = pcall(dofile, k[1]) - -- Ignore the empty table. + if lfs.attributes(candidate_path, "size") > 0 then + ok, stored = pcall(dofile, candidate_path) + -- Ignore empty tables if ok and next(stored) ~= nil then - logger.dbg("data is read from ", k[1]) - filepath = k[1] + logger.dbg("DocSettings: data is read from", candidate_path) + filepath = candidate_path break end end - logger.dbg(k[1], " is invalid, remove.") - os.remove(k[1]) + logger.dbg("DocSettings:", candidate_path, "is invalid, removed.") + os.remove(candidate_path) end if ok and stored then new.data = stored @@ -197,12 +212,12 @@ function DocSettings:flush() -- that the OS may have itself sync'ed that file content in the meantime. local mtime = lfs.attributes(f, "modification") if mtime < os.time() - 60 then - logger.dbg("Rename ", f, " to ", f .. ".old") + logger.dbg("DocSettings: Renamed", f, "to", f .. ".old") os.rename(f, f .. ".old") directory_updated = true -- fsync directory content too below end end - logger.dbg("Write to ", f) + logger.dbg("DocSettings: Writing to", f) local f_out = io.open(f, "w") if f_out ~= nil then f_out:write("-- we can read Lua syntax here!\nreturn ") @@ -213,10 +228,11 @@ function DocSettings:flush() if self.candidates ~= nil and G_reader_settings:nilOrFalse("preserve_legacy_docsetting") then - for _, k in ipairs(self.candidates) do - if k[1] ~= f and k[1] ~= f .. ".old" then - logger.dbg("Remove legacy file ", k[1]) - os.remove(k[1]) + for _, t in ipairs(self.candidates) do + local candidate_path = t.path + if candidate_path ~= f and candidate_path ~= f .. ".old" then + logger.dbg("DocSettings: Removed legacy file", candidate_path) + os.remove(candidate_path) -- We should not remove sidecar folder, as it may -- contain Kindle history files. end @@ -267,7 +283,7 @@ function DocSettings:purge(full) end if to_remove then os.remove(fullpath) - logger.dbg("purge: removed ", fullpath) + logger.dbg("DocSettings: purged:", fullpath) end end -- If the sidecar folder ends up empty, os.remove() can delete it. diff --git a/frontend/luadata.lua b/frontend/luadata.lua index 9bb6384fe..042e5ec2e 100644 --- a/frontend/luadata.lua +++ b/frontend/luadata.lua @@ -71,10 +71,10 @@ function LuaData:open(file_path, name) if lfs.attributes(new.file, "mode") == "file" then ok, err = loadfile(new.file, "t", data_env) if ok then - logger.dbg("data is read from", new.file) + logger.dbg("LuaData: data is read from", new.file) ok() else - logger.dbg(new.file, "is invalid, removed.", err) + logger.dbg("LuaData:", new.file, "is invalid, removed.", err) os.remove(new.file) end end @@ -84,11 +84,11 @@ function LuaData:open(file_path, name) if lfs.attributes(backup_file, "mode") == "file" then ok, err = loadfile(backup_file, "t", data_env) if ok then - logger.dbg("data is read from", backup_file) + logger.dbg("LuaData: data is read from", backup_file) ok() break else - logger.dbg(backup_file, "is invalid, removed.", err) + logger.dbg("LuaData:", backup_file, "is invalid, removed.", err) os.remove(backup_file) end end diff --git a/frontend/luadefaults.lua b/frontend/luadefaults.lua index 6b21222a3..e2fc4d820 100644 --- a/frontend/luadefaults.lua +++ b/frontend/luadefaults.lua @@ -31,14 +31,14 @@ function LuaDefaults:open(path) if ok and stored then new.rw = stored else - if existing then logger.warn("Failed reading", new.file, "(probably corrupted).") end + if existing then logger.warn("LuaDefaults: Failed reading", new.file, "(probably corrupted).") end -- Fallback to .old if it exists ok, stored = pcall(dofile, new.file..".old") if ok and stored then - if existing then logger.warn("read from backup file", new.file..".old") end + if existing then logger.warn("LuaDefaults: read from backup file", new.file..".old") end new.rw = stored else - if existing then logger.warn("no usable backup file for", new.file, "to read from") end + if existing then logger.warn("LuaDefaults: no usable backup file for", new.file, "to read from") end new.rw = {} end end diff --git a/frontend/luasettings.lua b/frontend/luasettings.lua index 5094f2391..bc3c06173 100644 --- a/frontend/luasettings.lua +++ b/frontend/luasettings.lua @@ -32,14 +32,14 @@ function LuaSettings:open(file_path) if ok and stored then new.data = stored else - if existing then logger.warn("Failed reading", new.file, "(probably corrupted).") end + if existing then logger.warn("LuaSettings: Failed reading", new.file, "(probably corrupted).") end -- Fallback to .old if it exists ok, stored = pcall(dofile, new.file..".old") if ok and stored then - if existing then logger.warn("read from backup file", new.file..".old") end + if existing then logger.warn("LuaSettings: read from backup file", new.file..".old") end new.data = stored else - if existing then logger.warn("no usable backup file for", new.file, "to read from") end + if existing then logger.warn("LuaSettings: no usable backup file for", new.file, "to read from") end new.data = {} end end