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

@@ -16,13 +16,31 @@ local DocSettings = LuaSettings:extend{}
 
 local HISTORY_DIR = DataStorage:getHistoryDir()
 
-local function buildCandidate(file_path)
+local function buildCandidates(list)
-    -- Ignore empty files.
+    local candidates = {}
-    if file_path and lfs.attributes(file_path, "mode") == "file" then
+
-        return { file_path, lfs.attributes(file_path, "modification") }
+    for i, file_path in ipairs(list) do
-    else
+        -- Ignore missing files.
-        return nil
+        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 = {}
+    -- Candidates list, in order of priority:
-    -- New sidecar file
+    local candidates_list = {
-    table.insert(candidates, buildCandidate(new.sidecar_file))
+        -- New sidecar file
-    -- Backup file of new sidecar file
+        new.sidecar_file or "",
-    table.insert(candidates, buildCandidate(new.sidecar_file and (new.sidecar_file .. ".old")))
+        -- Backup file of new sidecar file
-    -- Legacy sidecar file
+        new.sidecar_file and (new.sidecar_file .. ".old") or "",
-    table.insert(candidates, buildCandidate(new.legacy_sidecar_file))
+        -- Legacy sidecar file
-    -- Legacy history folder
+        new.legacy_sidecar_file or "",
-    table.insert(candidates, buildCandidate(new.history_file))
+        -- Legacy history folder
-    -- Backup file in legacy history folder
+        new.history_file,
-    table.insert(candidates, buildCandidate(new.history_file .. ".old"))
+        -- Backup file in legacy history folder
-    -- Legacy kpdfview setting
+        new.history_file .. ".old",
-    table.insert(candidates, buildCandidate(docfile..".kpdfview.lua"))
+        -- Legacy kpdfview setting
-    table.sort(candidates, function(l, r)
+        docfile..".kpdfview.lua",
-                               if l == nil then
+    }
-                                   return false
+    -- We get back an array of tables for *existing* candidates, sorted MRU first (insertion order breaks ties).
-                               elseif r == nil then
+    local candidates = buildCandidates(candidates_list)
-                                   return true
+
-                               else
-                                   return l[2] > r[2]
-                               end
-                           end)
     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
+        if lfs.attributes(candidate_path, "size") > 0 then
-            ok, stored = pcall(dofile, k[1])
+            ok, stored = pcall(dofile, candidate_path)
-            -- Ignore the empty table.
+            -- Ignore empty tables
             if ok and next(stored) ~= nil then
-                logger.dbg("data is read from ", k[1])
+                logger.dbg("DocSettings: data is read from", candidate_path)
-                filepath = k[1]
+                filepath = candidate_path
                 break
             end
         end
-        logger.dbg(k[1], " is invalid, remove.")
+        logger.dbg("DocSettings:", candidate_path, "is invalid, removed.")
-        os.remove(k[1])
+        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
+                for _, t in ipairs(self.candidates) do
-                    if k[1] ~= f and k[1] ~= f .. ".old" then
+                    local candidate_path = t.path
-                        logger.dbg("Remove legacy file ", k[1])
+                    if candidate_path ~= f and candidate_path ~= f .. ".old" then
-                        os.remove(k[1])
+                        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.

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

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

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