Add LuaData and Dictionary Lookup History (#3161)

* Add dictionary history

Fixes #2033, fixes #2998.

* Add LuaData

* table handling in base settings

* Add LuaData spec

frontend/apps/reader/modules/readerdictionary.lua

@@ -5,6 +5,8 @@ local DictQuickLookup = require("ui/widget/dictquicklookup")
 local InfoMessage = require("ui/widget/infomessage")
 local InputContainer = require("ui/widget/container/inputcontainer")
 local JSON = require("json")
+local KeyValuePage = require("ui/widget/keyvaluepage")
+local LuaData = require("luadata")
 local Trapper = require("ui/trapper")
 local UIManager = require("ui/uimanager")
 local logger = require("logger")
@@ -16,6 +18,7 @@ local T = require("ffi/util").template
 -- We'll store the list of available dictionaries as a module local
 -- so we only have to look for them on the first :init()
 local available_ifos = nil
+local lookup_history = nil
 
 local function getIfosInDir(path)
     -- Get all the .ifo under directory path.
@@ -50,7 +53,8 @@ end
 local ReaderDictionary = InputContainer:new{
     data_dir = nil,
     dict_window_list = {},
-    lookup_msg = _("Searching dictionary for:\n%1")
+    disable_lookup_history = G_reader_settings:isTrue("disable_lookup_history"),
+    lookup_msg = _("Searching dictionary for:\n%1"),
 }
 
 function ReaderDictionary:init()
@@ -96,6 +100,9 @@ function ReaderDictionary:init()
     end
     -- Prepare the -u options to give to sdcv if some dictionaries are disabled
     self:updateSdcvDictNamesOptions()
+    if not lookup_history then
+        lookup_history = LuaData:open(DataStorage:getSettingsDir() .. "/lookup_history.lua", { name = "LookupHistory" })
+    end
 end
 
 function ReaderDictionary:updateSdcvDictNamesOptions()
@@ -142,6 +149,35 @@ function ReaderDictionary:addToMainMenu(menu_items)
             end,
         },
     }
+    menu_items.dictionary_lookup_history = {
+        text = _("Dictionary lookup history"),
+        enabled_func = function()
+            return lookup_history:has("lookup_history")
+        end,
+        callback = function()
+            local lookup_history_table = lookup_history:readSetting("lookup_history")
+            local kv_pairs = {}
+            local previous_title
+            for i = #lookup_history_table, 1, -1 do
+                local value = lookup_history_table[i]
+                if value.book_title ~= previous_title then
+                    table.insert(kv_pairs, { value.book_title..":", "" })
+                end
+                previous_title = value.book_title
+                table.insert(kv_pairs, {
+                    os.date("%Y-%m-%d %H:%M:%S", value.time),
+                    value.word,
+                    callback = function()
+                        self:onLookupWord(value.word)
+                    end
+                })
+            end
+            UIManager:show(KeyValuePage:new{
+                title = _("Dictionary lookup history"),
+                kv_pairs = kv_pairs,
+            })
+        end,
+    }
     menu_items.dictionary_settings = {
         text = _("Dictionary settings"),
         sub_item_table = {
@@ -182,6 +218,29 @@ If you'd like to change the order in which dictionaries are queried (and their r
                     self:makeDisableFuzzyDefault(self.disable_fuzzy_search)
                 end,
             },
+            {
+                text = _("Disable dictionary lookup history"),
+                checked_func = function()
+                    return self.disable_lookup_history
+                end,
+                callback = function()
+                    self.disable_lookup_history = not self.disable_lookup_history
+                    G_reader_settings:saveSetting("disable_lookup_history", self.disable_lookup_history)
+                end,
+            },
+            {
+                text = _("Clean dictionary lookup history"),
+                callback = function()
+                    UIManager:show(ConfirmBox:new{
+                        text = _("Clean dictionary lookup history?"),
+                        ok_text = _("Clean"),
+                        ok_callback = function()
+                            -- empty data table to replace current one
+                            lookup_history:reset{}
+                        end,
+                    })
+                end,
+            },
             { -- setting used by dictquicklookup
                 text = _("Justify text"),
                 checked_func = function()
@@ -329,6 +388,16 @@ function ReaderDictionary:stardictLookup(word, box, link)
     if word == "" then
         return
     end
+
+    if not self.disable_lookup_history then
+        local book_title = self.ui.doc_settings and self.ui.doc_settings:readSetting("doc_props").title or _("Dictionary lookup")
+        lookup_history:addTableItem("lookup_history", {
+            book_title = book_title,
+            time = os.time(),
+            word = word,
+        })
+    end
+
     if not self.disable_fuzzy_search then
         self:showLookupInfo(word)
     end
@@ -503,6 +572,11 @@ function ReaderDictionary:makeDisableFuzzyDefault(disable_fuzzy_search)
             and _("Disable fuzzy search by default?")
             or _("Enable fuzzy search by default?")
         ),
+        ok_text = T(
+            disable_fuzzy_search
+            and _("Disable")
+            or _("Enable")
+        ),
         ok_callback = function()
             G_reader_settings:saveSetting("disable_fuzzy_search", disable_fuzzy_search)
         end,

frontend/luadata.lua

@@ -0,0 +1,165 @@
+--[[--
+Handles append-mostly data such as KOReader's bookmarks and dictionary search history.
+]]
+
+local LuaSettings = require("luasettings")
+local dbg = require("dbg")
+local dump = require("dump")
+local logger = require("logger")
+local util = require("util")
+
+local LuaData = LuaSettings:new{
+    name = "",
+    max_backups = 9,
+}
+
+--- Creates a new LuaData instance.
+function LuaData:open(file_path, o) -- luacheck: ignore 312
+    if o and type(o) ~= "table" then
+        if dbg.is_on then
+            error("LuaData: got "..type(o)..", table expected")
+        else
+            o = {}
+        end
+    end
+    -- always initiate a new instance
+    -- careful, `o` is already a table so we use parentheses
+    self = LuaData:new(o)
+
+    local new = {file=file_path, data={}}
+
+    -- some magic to allow for self-describing function names
+    local _local = {}
+    _local.__index = _local
+    setmetatable(_G, _local)
+    _local[self.name.."Entry"] = function(table)
+        if table.index then
+            -- we've got a deleted setting, overwrite with nil
+            if not table.data then new.data[table.index] = nil end
+            new.data[table.index] = new.data[table.index] or {}
+            local size = util.tableSize(table.data)
+            if size == 1 then
+                for key, value in pairs(table.data) do
+                    new.data[table.index][key] = value
+                end
+            else
+                new.data[table.index] = table.data
+            end
+        -- we've got it all at once
+        else
+            new.data = table
+        end
+    end
+
+    local ok = pcall(dofile, new.file)
+
+    if ok then
+        logger.dbg("data is read from ", new.file)
+    else
+        logger.dbg(new.file, " is invalid, remove.")
+        os.remove(new.file)
+        for i=1, self.max_backups, 1 do
+            local backup_file = new.file..".old."..i
+            if pcall(dofile, backup_file) then
+                logger.dbg("data is read from ", backup_file)
+                break
+            else
+                logger.dbg(backup_file, " is invalid, remove.")
+                os.remove(backup_file)
+            end
+        end
+    end
+
+    return setmetatable(new, {__index = self})
+end
+
+--- Saves a setting.
+function LuaData:saveSetting(key, value)
+    self.data[key] = value
+    self:append{
+        index = key,
+        data = value,
+    }
+    return self
+end
+
+--- Deletes a setting.
+function LuaData:delSetting(key)
+    self.data[key] = nil
+    self:append{
+        index = key,
+    }
+    return self
+end
+
+--- Adds item to table.
+function LuaData:addTableItem(table_name, value)
+    local settings_table = self:has(table_name) and self:readSetting(table_name) or {}
+    table.insert(settings_table, value)
+    self.data[table_name] = settings_table
+    self:append{
+        index = table_name,
+        data = {[#settings_table] = value},
+    }
+end
+
+local _orig_removeTableItem = LuaSettings.removeTableItem
+--- Removes index from table.
+function LuaData:removeTableItem(key, index)
+    _orig_removeTableItem(self, key, index)
+    self:flush()
+    return self
+end
+
+--- Appends settings to disk.
+function LuaData:append(data)
+    if not self.file then return end
+    local f_out = io.open(self.file, "a")
+    if f_out ~= nil then
+        os.setlocale('C', 'numeric')
+        f_out:write(self.name.."Entry")
+        f_out:write(dump(data))
+        f_out:write("\n")
+        f_out:close()
+    end
+    return self
+end
+
+--- Replaces existing settings with table.
+function LuaData:reset(table)
+    self.data = table
+    self:flush()
+    return self
+end
+
+--- Writes all settings to disk (does not append).
+function LuaData:flush()
+    if not self.file then return end
+
+    if lfs.attributes(self.file, "mode") == "file" then
+        for i=1, self.max_backups, 1 do
+            if lfs.attributes(self.file..".old."..i, "mode") == "file" then
+                logger.dbg("LuaData: Rename ", self.file .. ".old." .. i, " to ", self.file .. ".old." .. i+1)
+                os.rename(self.file, self.file .. ".old." .. i+1)
+            else
+                break
+            end
+        end
+        logger.dbg("LuaData: Rename ", self.file, " to ", self.file .. ".old.1")
+        os.rename(self.file, self.file .. ".old.1")
+    end
+
+    logger.dbg("LuaData: Write to ", self.file)
+    local f_out = io.open(self.file, "w")
+    if f_out ~= nil then
+        os.setlocale('C', 'numeric')
+        f_out:write("-- we can read Lua syntax here!\n")
+        f_out:write(self.name.."Entry")
+        f_out:write(dump(self.data))
+        f_out:write("\n")
+        f_out:close()
+    end
+    return self
+end
+
+return LuaData

frontend/luasettings.lua

@@ -6,6 +6,13 @@ local dump = require("dump")
 
 local LuaSettings = {}
 
+function LuaSettings:new(o)
+    o = o or {}
+    setmetatable(o, self)
+    self.__index = self
+    return o
+end
+
 --- Opens a settings file.
 function LuaSettings:open(file_path)
     local new = {file=file_path}
@@ -133,6 +140,22 @@ function LuaSettings:flipFalse(key)
     return self
 end
 
+--- Adds item to table.
+function LuaSettings:addTableItem(key, value)
+    local settings_table = self:has(key) and self:readSetting(key) or {}
+    table.insert(settings_table, value)
+    self:saveSetting(key, settings_table)
+    return self
+end
+
+--- Removes index from table.
+function LuaSettings:removeTableItem(key, index)
+    local settings_table = self:has(key) and self:readSetting(key) or {}
+    table.remove(settings_table, index)
+    self:saveSetting(key, settings_table)
+    return self
+end
+
 --- Replaces existing settings with table.
 function LuaSettings:reset(table)
     self.data = table

frontend/ui/elements/filemanager_menu_order.lua

@@ -53,6 +53,7 @@ local order = {
     },
     search = {
         "dictionary_lookup",
+        "dictionary_lookup_history",
         "dictionary_settings",
         "----------------------------",
         "wikipedia_lookup",

frontend/ui/elements/reader_menu_order.lua

@@ -71,6 +71,7 @@ local order = {
     },
     search = {
         "dictionary_lookup",
+        "dictionary_lookup_history",
         "dictionary_settings",
         "----------------------------",
         "wikipedia_lookup",

spec/unit/luadata_spec.lua

@@ -0,0 +1,145 @@
+describe("luadata module", function()
+    local Settings
+    setup(function()
+        require("commonrequire")
+        Settings = require("frontend/luadata"):open("this-is-not-a-valid-file")
+    end)
+
+    it("should handle undefined keys", function()
+        Settings:delSetting("abc")
+
+        assert.True(Settings:hasNot("abc"))
+        assert.True(Settings:nilOrTrue("abc"))
+        assert.False(Settings:isTrue("abc"))
+        Settings:saveSetting("abc", true)
+        assert.True(Settings:has("abc"))
+        assert.True(Settings:nilOrTrue("abc"))
+        assert.True(Settings:isTrue("abc"))
+    end)
+
+    it("should flip bool values", function()
+        Settings:delSetting("abc")
+
+        assert.True(Settings:hasNot("abc"))
+        Settings:flipNilOrTrue("abc")
+        assert.False(Settings:nilOrTrue("abc"))
+        assert.True(Settings:has("abc"))
+        assert.False(Settings:isTrue("abc"))
+        Settings:flipNilOrTrue("abc")
+        assert.True(Settings:nilOrTrue("abc"))
+        assert.True(Settings:hasNot("abc"))
+        assert.False(Settings:isTrue("abc"))
+
+        Settings:flipTrue("abc")
+        assert.True(Settings:has("abc"))
+        assert.True(Settings:isTrue("abc"))
+        assert.True(Settings:nilOrTrue("abc"))
+        Settings:flipTrue("abc")
+        assert.False(Settings:has("abc"))
+        assert.False(Settings:isTrue("abc"))
+        assert.True(Settings:nilOrTrue("abc"))
+    end)
+
+    it("should create child settings", function()
+        Settings:delSetting("key")
+
+        Settings:saveSetting("key", {
+            a = "b",
+            c = "true",
+            d = false,
+        })
+
+        local child = Settings:child("key")
+
+        assert.is_not_nil(child)
+        assert.True(child:has("a"))
+        assert.are.equal(child:readSetting("a"), "b")
+        assert.True(child:has("c"))
+        assert.True(child:isTrue("c"))
+        assert.True(child:has("d"))
+        assert.True(child:isFalse("d"))
+        assert.False(child:isTrue("e"))
+        child:flipTrue("e")
+        child:close()
+
+        child = Settings:child("key")
+        assert.True(child:isTrue("e"))
+    end)
+
+    describe("table wrapper", function()
+        Settings:delSetting("key")
+
+        it("should add item to table", function()
+            Settings:addTableItem("key", 1)
+            Settings:addTableItem("key", 2)
+            Settings:addTableItem("key", 3)
+
+            assert.are.equal(1, Settings:readSetting("key")[1])
+            assert.are.equal(2, Settings:readSetting("key")[2])
+            assert.are.equal(3, Settings:readSetting("key")[3])
+        end)
+
+        it("should remove item from table", function()
+            Settings:removeTableItem("key", 1)
+
+            assert.are.equal(2, Settings:readSetting("key")[1])
+            assert.are.equal(3, Settings:readSetting("key")[2])
+        end)
+    end)
+
+    describe("backup data file", function()
+        local file = "dummy-test-file"
+        local d = Settings:open(file)
+        it("should generate data file", function()
+            d:saveSetting("a", "a")
+            assert.Equals("file", lfs.attributes(d.file, "mode"))
+        end)
+        it("should generate backup data file on flush", function()
+            d:flush()
+            -- file and file.old.1 should be generated.
+            assert.Equals("file", lfs.attributes(d.file, "mode"))
+            assert.Equals("file", lfs.attributes(d.file .. ".old.1", "mode"))
+            d:close()
+        end)
+        it("should remove garbage data file", function()
+            -- write some garbage to sidecar-file.
+            local f_out = io.open(d.file, "w")
+            f_out:write("bla bla bla")
+            f_out:close()
+
+            d = Settings:open(file)
+            -- file should be removed.
+            assert.are.not_equal("file", lfs.attributes(d.file, "mode"))
+            assert.Equals("file", lfs.attributes(d.file .. ".old.2", "mode"))
+            assert.Equals("a", d:readSetting("a"))
+            d:saveSetting("a", "b")
+            d:close()
+            -- backup should be generated.
+            assert.Equals("file", lfs.attributes(d.file, "mode"))
+            assert.Equals("file", lfs.attributes(d.file .. ".old.1", "mode"))
+            -- The contents in file and file.old.1 are different.
+            -- a:b v.s. a:a
+        end)
+        it("should open backup data file after garbage removal", function()
+            d = Settings:open(file)
+            -- We should get the right result.
+            assert.Equals("b", d:readSetting("a"))
+            -- write some garbage to file.
+            local f_out = io.open(d.file, "w")
+            f_out:write("bla bla bla")
+            f_out:close()
+
+            -- do not flush the result, open docsettings again.
+            d = Settings:open(file)
+            -- data file should be removed.
+            assert.are.not_equal("file", lfs.attributes(d.file, "mode"))
+            assert.Equals("file", lfs.attributes(d.file .. ".old.2", "mode"))
+            -- The content should come from file.old.2.
+            assert.Equals("a", d:readSetting("a"))
+            d:close()
+            -- data file should be generated and last good backup should not change name.
+            assert.Equals("file", lfs.attributes(d.file, "mode"))
+            assert.Equals("file", lfs.attributes(d.file .. ".old.2", "mode"))
+        end)
+    end)
+end)

spec/unit/luasettings_spec.lua

@@ -41,6 +41,8 @@ describe("luasettings module", function()
     end)
 
     it("should create child settings", function()
+        Settings:delSetting("key")
+
         Settings:saveSetting("key", {
             a = "b",
             c = "true",
@@ -63,4 +65,25 @@ describe("luasettings module", function()
         child = Settings:child("key")
         assert.True(child:isTrue("e"))
     end)
+
+    describe("table wrapper", function()
+        Settings:delSetting("key")
+
+        it("should add item to table", function()
+            Settings:addTableItem("key", 1)
+            Settings:addTableItem("key", 2)
+            Settings:addTableItem("key", 3)
+
+            assert.are.equal(1, Settings:readSetting("key")[1])
+            assert.are.equal(2, Settings:readSetting("key")[2])
+            assert.are.equal(3, Settings:readSetting("key")[3])
+        end)
+
+        it("should remove item from table", function()
+            Settings:removeTableItem("key", 1)
+
+            assert.are.equal(2, Settings:readSetting("key")[1])
+            assert.are.equal(3, Settings:readSetting("key")[2])
+        end)
+    end)
 end)