Some History fixes and enhancements (#3247)

Made the onHold buttons table similar to the one of File browser.
Added "Purge .sdr" and "Delete" to these buttons.
Moved the purgeSettings and removeFileFromHistoryIfWanted
logic into filemanagerutil functions.
Stay on the same page when manipulating history (previously, we were
always put back on first page).
Really keep deleted files in history (unless setting says otherwise).
Show deleted files in grey or dimmed in classic History and all
CoverBrowser display modes.

frontend/apps/filemanager/filemanager.lua

@@ -146,28 +146,9 @@ function FileManager:init()
                             text = util.template(_("Purge .sdr to reset settings for this document?\n\n%1"), self.file_dialog.title),
                             ok_text = _("Purge"),
                             ok_callback = function()
-                                local file_abs_path = util.realpath(file)
+                                filemanagerutil.purgeSettings(file)
-                                if file_abs_path then
+                                filemanagerutil.removeFileFromHistoryIfWanted(file)
-                                    local autoremove_deleted_items_from_history = G_reader_settings:readSetting("autoremove_deleted_items_from_history") or false
+                                self:refreshPath()
-                                    os.remove(DocSettings:getSidecarFile(file_abs_path))
-                                    -- also remove backup, otherwise it will be used if we re-open this document
-                                    -- (it also allows for the sidecar folder to be empty and removed)
-                                    os.remove(DocSettings:getSidecarFile(file_abs_path)..".old")
-                                    -- If the sidecar folder is empty, os.remove() can
-                                    -- delete it. Otherwise, the following statement has no
-                                    -- effect.
-                                    os.remove(DocSettings:getSidecarDir(file_abs_path))
-                                    self:refreshPath()
-                                    -- also delete from history and update lastfile to top item in
-                                    -- history if autoremove_deleted_items_from_history is enabled
-                                    if autoremove_deleted_items_from_history then
-                                        local readhistory = require("readhistory")
-                                        readhistory:removeItemByPath(file_abs_path)
-                                        if G_reader_settings:readSetting("lastfile") == file_abs_path then
-                                            G_reader_settings:saveSetting("lastfile", #readhistory.hist > 0 and readhistory.hist[1].file or nil)
-                                        end
-                                    end
-                                end
                                 UIManager:close(self.file_dialog)
                             end,
                         })
@@ -186,26 +167,14 @@ function FileManager:init()
                     text = _("Delete"),
                     callback = function()
                         local ConfirmBox = require("ui/widget/confirmbox")
-                        UIManager:close(self.file_dialog)
                         UIManager:show(ConfirmBox:new{
                             text = _("Are you sure that you want to delete this file?\n") .. file .. ("\n") .. _("If you delete a file, it is permanently lost."),
                             ok_text = _("Delete"),
                             ok_callback = function()
-                                local autoremove_deleted_items_from_history = G_reader_settings:readSetting("autoremove_deleted_items_from_history") or false
-                                local file_abs_path = util.realpath(file)
                                 deleteFile(file)
-                                -- also delete from history and update lastfile to top item in
+                                filemanagerutil.removeFileFromHistoryIfWanted(file)
-                                -- history if autoremove_deleted_items_from_history is enabled
-                                if autoremove_deleted_items_from_history then
-                                    if file_abs_path then
-                                        local readhistory = require("readhistory")
-                                        readhistory:removeItemByPath(file_abs_path)
-                                        if G_reader_settings:readSetting("lastfile") == file_abs_path then
-                                            G_reader_settings:saveSetting("lastfile", #readhistory.hist > 0 and readhistory.hist[1].file or nil)
-                                        end
-                                    end
-                                end
                                 self:refreshPath()
+                                UIManager:close(self.file_dialog)
                             end,
                         })
                     end,

frontend/apps/filemanager/filemanagerhistory.lua

@@ -1,13 +1,13 @@
-local ButtonDialog = require("ui/widget/buttondialog")
+local ButtonDialogTitle = require("ui/widget/buttondialogtitle")
+local DocSettings = require("docsettings")
 local FileManagerBookInfo = require("apps/filemanager/filemanagerbookinfo")
-local Font = require("ui/font")
 local InputContainer = require("ui/widget/container/inputcontainer")
 local Menu = require("ui/widget/menu")
 local UIManager = require("ui/uimanager")
-local RenderText = require("ui/rendertext")
 local Screen = require("device").screen
+local filemanagerutil = require("apps/filemanager/filemanagerutil")
+local util = require("ffi/util")
 local _ = require("gettext")
-local T = require("ffi/util").template
 
 local FileManagerHistory = InputContainer:extend{
     hist_menu_title = _("History"),
@@ -28,8 +28,13 @@ function FileManagerHistory:addToMainMenu(menu_items)
 end
 
 function FileManagerHistory:updateItemTable()
+    -- try to stay on current page
+    local select_number = nil
+    if self.hist_menu.page and self.hist_menu.perpage then
+        select_number = (self.hist_menu.page - 1) * self.hist_menu.perpage + 1
+    end
     self.hist_menu:switchItemTable(self.hist_menu_title,
-                                  require("readhistory").hist)
+                                  require("readhistory").hist, select_number)
 end
 
 function FileManagerHistory:onSetDimensions(dimen)
@@ -37,57 +42,81 @@ function FileManagerHistory:onSetDimensions(dimen)
 end
 
 function FileManagerHistory:onMenuHold(item)
-    local font_size = Font:getFace("tfont")
+    self.histfile_dialog = nil
-    local text_remove_hist = _("Remove \"%1\" from history")
+    local buttons = {
-    local text_remove_without_item = T(text_remove_hist, "")
+        {
-    local text_remove_hist_width = (RenderText:sizeUtf8Text(
-        0, self.width, font_size, text_remove_without_item).x )
-    local text_item_width = (RenderText:sizeUtf8Text(
-        0, self.width , font_size, item.text).x )
-
-    local item_trun
-    if self.width < text_remove_hist_width + text_item_width then
-        item_trun = RenderText:truncateTextByWidth(item.text, font_size, 1.2 * self.width - text_remove_hist_width)
-    else
-        item_trun = item.text
-    end
-    local text_remove = T(text_remove_hist, item_trun)
-
-    self.histfile_dialog = ButtonDialog:new{
-        buttons = {
             {
-                {
+                text = _("Purge .sdr"),
-                    text = text_remove,
+                enabled = DocSettings:hasSidecarFile(util.realpath(item.file)),
-                    callback = function()
+                callback = function()
-                        require("readhistory"):removeItem(item)
+                    local ConfirmBox = require("ui/widget/confirmbox")
-                        self._manager:updateItemTable()
+                    UIManager:show(ConfirmBox:new{
-                        UIManager:close(self.histfile_dialog)
+                        text = util.template(_("Purge .sdr to reset settings for this document?\n\n%1"), item.text),
-                    end,
+                        ok_text = _("Purge"),
-                },
+                        ok_callback = function()
+                            filemanagerutil.purgeSettings(item.file)
+                            filemanagerutil.removeFileFromHistoryIfWanted(item.file)
+                            self._manager:updateItemTable()
+                            UIManager:close(self.histfile_dialog)
+                        end,
+                    })
+                end,
             },
             {
-                {
+                text = _("Remove from history"),
-                    text = _("Book information"),
+                callback = function()
-                    enabled = FileManagerBookInfo:isSupported(item.file),
+                    require("readhistory"):removeItem(item)
-                    callback = function()
+                    self._manager:updateItemTable()
-                        FileManagerBookInfo:show(item.file)
+                    UIManager:close(self.histfile_dialog)
-                        UIManager:close(self.histfile_dialog)
+                end,
-                    end,
-                 },
             },
-            {},
+        },
+        {
             {
-                {
+                text = _("Delete"),
-                    text = _("Clear history of deleted files"),
+                enabled = lfs.attributes(item.file, "mode") and true or false,
-                    callback = function()
+                callback = function()
-                        require("readhistory"):clearMissing()
+                    local ConfirmBox = require("ui/widget/confirmbox")
-                        self._manager:updateItemTable()
+                    UIManager:show(ConfirmBox:new{
-                        UIManager:close(self.histfile_dialog)
+                        text = _("Are you sure that you want to delete this file?\n") .. item.file .. ("\n") .. _("If you delete a file, it is permanently lost."),
-                    end,
+                        ok_text = _("Delete"),
-                 },
+                        ok_callback = function()
+                            local FileManager = require("apps/filemanager/filemanager")
+                            FileManager:deleteFile(item.file)
+                            filemanagerutil.removeFileFromHistoryIfWanted(item.file)
+                            require("readhistory"):setDeleted(item)
+                            self._manager:updateItemTable()
+                            UIManager:close(self.histfile_dialog)
+                        end,
+                    })
+                end,
             },
+            {
+                text = _("Book information"),
+                enabled = FileManagerBookInfo:isSupported(item.file),
+                callback = function()
+                    FileManagerBookInfo:show(item.file)
+                    UIManager:close(self.histfile_dialog)
+                end,
+             },
+        },
+        {},
+        {
+            {
+                text = _("Clear history of deleted files"),
+                callback = function()
+                    require("readhistory"):clearMissing()
+                    self._manager:updateItemTable()
+                    UIManager:close(self.histfile_dialog)
+                end,
+             },
         },
     }
+    self.histfile_dialog = ButtonDialogTitle:new{
+        title = item.text:match("([^/]+)$"),
+        title_align = "center",
+        buttons = buttons,
+    }
     UIManager:show(self.histfile_dialog)
     return true
 end

frontend/apps/filemanager/filemanagerutil.lua

@@ -3,6 +3,8 @@ This module contains miscellaneous helper functions for FileManager
 ]]
 
 local Device = require("device")
+local DocSettings = require("docsettings")
+local util = require("ffi/util")
 
 local filemanagerutil = {}
 
@@ -31,4 +33,30 @@ function filemanagerutil.abbreviate(path)
     return path
 end
 
+-- Purge doc settings in sidecar directory,
+function filemanagerutil.purgeSettings(file)
+    local file_abs_path = util.realpath(file)
+    if file_abs_path then
+        os.remove(DocSettings:getSidecarFile(file_abs_path))
+        -- Also remove backup, otherwise it will be used if we re-open this document
+        -- (it also allows for the sidecar folder to be empty and removed)
+        os.remove(DocSettings:getSidecarFile(file_abs_path)..".old")
+        -- If the sidecar folder is empty, os.remove() can delete it.
+        -- Otherwise, the following statement has no effect.
+        os.remove(DocSettings:getSidecarDir(file_abs_path))
+    end
+end
+
+-- Remove from history and update lastfile to top item in history
+-- if autoremove_deleted_items_from_history is enabled
+function filemanagerutil.removeFileFromHistoryIfWanted(file)
+    if G_reader_settings:readSetting("autoremove_deleted_items_from_history") then
+        local readhistory = require("readhistory")
+        readhistory:removeItemByPath(file)
+        if G_reader_settings:readSetting("lastfile") == file then
+            G_reader_settings:saveSetting("lastfile", #readhistory.hist > 0 and readhistory.hist[1].file or nil)
+        end
+    end
+end
+
 return filemanagerutil

frontend/readhistory.lua

@@ -16,7 +16,8 @@ local function buildEntry(input_time, input_file)
     return {
         time = input_time,
         text = input_file:gsub(".*/", ""),
-        file = realpath(input_file),
+        file = realpath(input_file) or input_file, -- keep orig file path of deleted files
+        dim = lfs.attributes(input_file, "mode") ~= "file", -- "dim", as expected by Menu
         callback = function()
             local ReaderUI = require("apps/reader/readerui")
             ReaderUI:showReader(input_file)
@@ -55,11 +56,6 @@ function ReadHistory:_sort()
     if autoremove_deleted_items_from_history then
         self:clearMissing()
     end
-    for i = #self.hist, 1, -1 do
-        if self.hist[i].file == nil then
-            table.remove(self.hist, i)
-        end
-    end
     table.sort(self.hist, fileFirstOrdering)
     -- TODO(zijiehe): Use binary insert instead of a loop to deduplicate.
     for i = #self.hist, 2, -1 do
@@ -179,6 +175,13 @@ function ReadHistory:addItem(file)
     end
 end
 
+function ReadHistory:setDeleted(item)
+    assert(self ~= nil)
+    if self.hist[item.index] then
+        self.hist[item.index].dim = true
+    end
+end
+
 --- Reloads history from history_file.
 -- @treturn boolean true if history_file has been updated and reload happened.
 function ReadHistory:reload()

frontend/ui/widget/menu.lua

@@ -209,6 +209,7 @@ function MenuItem:init()
                 text = self.text,
                 face = self.face,
                 bold = self.bold,
+                fgcolor = self.dim and Blitbuffer.COLOR_GREY or nil,
             }
         }
     }
@@ -219,6 +220,7 @@ function MenuItem:init()
             text = mandatory,
             face = self.info_face,
             bold = self.bold,
+            fgcolor = self.dim and Blitbuffer.COLOR_GREY or nil,
         }
     }
 
@@ -651,6 +653,7 @@ function Menu:updateItems(select_number)
                 text = getMenuText(self.item_table[i]),
                 mandatory = self.item_table[i].mandatory,
                 bold = self.item_table.current == i or self.item_table[i].bold == true,
+                dim = self.item_table[i].dim,
                 face = self.cface,
                 dimen = self.item_dimen:new(),
                 shortcut = item_shortcut,

plugins/coverbrowser.koplugin/covermenu.lua

@@ -326,22 +326,23 @@ function CoverMenu:onHistoryMenuHold(item)
         return true
     end
 
-    -- Remember some of this original ButtonDialog properties
+    -- Remember some of this original ButtonDialogTitle properties
+    local orig_title = self.histfile_dialog.title
+    local orig_title_align = self.histfile_dialog.title_align
     local orig_buttons = self.histfile_dialog.buttons
     -- Close original ButtonDialog (it has not yet been painted
     -- on screen, so we won't see it)
     UIManager:close(self.histfile_dialog)
 
     -- Replace Book information callback to use directly our bookinfo
-    orig_buttons[2][1].callback = function()
+    orig_buttons[2][2].callback = function()
         FileManagerBookInfo:show(file, bookinfo)
         UIManager:close(self.histfile_dialog)
     end
-    -- Re-organise buttons to make them more coherent with those we're going to add
+
-    -- Move up "Clear history of deleted items" and down "Book information", so
+    -- Remove last button ("Clear history of deleted files"), we'll
-    -- it's now similar to File browser's onFileHold
+    -- add it back after our buttons
-    -- (The original organisation is fine in classic mode)
+    local last_button = table.remove(orig_buttons)
-    orig_buttons[2], orig_buttons[4] = orig_buttons[4], orig_buttons[2]
 
     -- Add some new buttons to original buttons set
     table.insert(orig_buttons, {
@@ -412,10 +413,15 @@ function CoverMenu:onHistoryMenuHold(item)
             end,
         },
     })
+    table.insert(orig_buttons, {}) -- separator
+    -- Put back "Clear history of deleted files"
+    table.insert(orig_buttons, last_button)
 
     -- Create the new ButtonDialog, and let UIManager show it
-    local ButtonDialog = require("ui/widget/buttondialog")
+    local ButtonDialogTitle = require("ui/widget/buttondialogtitle")
-    self.histfile_dialog = ButtonDialog:new{
+    self.histfile_dialog = ButtonDialogTitle:new{
+        title = orig_title,
+        title_align = orig_title_align,
         buttons = orig_buttons,
     }
     UIManager:show(self.histfile_dialog)

plugins/coverbrowser.koplugin/listmenu.lua

@@ -187,7 +187,8 @@ function ListMenuItem:update()
         h = self.height - 2 * self.underline_h
     }
 
-    if lfs.attributes(self.filepath, "mode") == "directory" then
+    local file_mode = lfs.attributes(self.filepath, "mode")
+    if file_mode == "directory" then
         self.is_directory = true
         -- nb items on the right, directory name on the left
         local wright = TextWidget:new{
@@ -220,6 +221,9 @@ function ListMenuItem:update()
             },
         }
     else
+        if file_mode ~= "file" then
+            self.file_deleted = true
+        end
         -- File
         local border_size = 1
         local max_img_w = dimen.h - 2*border_size -- width = height, squared
@@ -267,6 +271,7 @@ function ListMenuItem:update()
                             margin = 0,
                             padding = 0,
                             bordersize = border_size,
+                            dim = self.file_deleted,
                             wimage,
                         }
                     }
@@ -331,10 +336,12 @@ function ListMenuItem:update()
             local wfileinfo = TextWidget:new{
                 text = fileinfo_str,
                 face = Font:getFace("cfont", 14),
+                fgcolor = self.file_deleted and Blitbuffer.COLOR_GREY or nil,
             }
             local wpageinfo = TextWidget:new{
                 text = pages_str,
                 face = Font:getFace("cfont", 14),
+                fgcolor = self.file_deleted and Blitbuffer.COLOR_GREY or nil,
             }
 
             local wright_width = math.max(wfileinfo:getSize().w, wpageinfo:getSize().w)
@@ -421,6 +428,7 @@ function ListMenuItem:update()
                     width = wmain_width,
                     alignment = "left",
                     bold = true,
+                    fgcolor = self.file_deleted and Blitbuffer.COLOR_GREY or nil,
                 }
                 local height = wtitle:getSize().h
                 if authors then
@@ -429,6 +437,7 @@ function ListMenuItem:update()
                         face = Font:getFace(fontname_authors, fontsize_authors),
                         width = wmain_width,
                         alignment = "left",
+                        fgcolor = self.file_deleted and Blitbuffer.COLOR_GREY or nil,
                     }
                     height = height + wauthors:getSize().h
                 end
@@ -513,15 +522,20 @@ function ListMenuItem:update()
                 self.been_opened = true
             end
             -- A real simple widget, nothing fancy
+            local hint = "…" -- display hint it's being loaded
+            if self.file_deleted then -- unless file was deleted (can happen with History)
+                hint = " " .. _("(deleted)")
+            end
             widget = LeftContainer:new{
                 dimen = dimen,
                 HorizontalGroup:new{
                     HorizontalSpan:new{ width = Screen:scaleBySize(10) },
                     TextBoxWidget:new{
-                        text = self.text .. "…", -- display hint it's being loaded
+                        text = self.text .. hint,
                         face = Font:getFace("cfont", 18),
                         width = dimen.w - 2 * Screen:scaleBySize(10),
                         alignment = "left",
+                        fgcolor = self.file_deleted and Blitbuffer.COLOR_GREY or nil,
                     }
                 },
             }
@@ -736,7 +750,7 @@ function ListMenu:_updateItemsBuildUI()
         -- this is for focus manager
         table.insert(self.layout, {item_tmp})
 
-        if not item_tmp.bookinfo_found and not item_tmp.is_directory then
+        if not item_tmp.bookinfo_found and not item_tmp.is_directory and not item_tmp.file_deleted then
             -- Register this item for update
             table.insert(self.items_to_update, item_tmp)
         end

plugins/coverbrowser.koplugin/main.lua

@@ -513,8 +513,8 @@ local function _FileManagerHistory_updateItemTable(self)
         hist_menu._do_hint_opened = BookInfoManager:getSetting("history_hint_opened")
     end
 
-    -- We do now the single thing done in FileManagerHistory:updateItemTable():
+    -- And do now what the original does
-    hist_menu:switchItemTable(self.hist_menu_title, require("readhistory").hist)
+    _FileManagerHistory_updateItemTable_orig(self)
 end
 
 function CoverBrowser:setupHistoryDisplayMode(display_mode)

plugins/coverbrowser.koplugin/mosaicmenu.lua

@@ -93,7 +93,9 @@ local FakeCover = FrameContainer:new{
     margin = 0,
     padding = 0,
     bordersize = 1,
+    dim = nil,
     filename = nil,
+    file_deleted = nil,
     title = nil,
     authors = nil,
     -- these font sizes will be scaleBySize'd by Font:getFace()
@@ -244,6 +246,11 @@ function FakeCover:init()
         table.insert(vgroup, VerticalSpan:new{ width = self.bottom_pad })
     end
 
+    if self.file_deleted then
+        self.dim = true
+        self.color = Blitbuffer.COLOR_GREY
+    end
+
     -- As we are a FrameContainer, a border will be painted around self[1]
     self[1] = CenterContainer:new{
         dimen = Geom:new{
@@ -354,7 +361,8 @@ function MosaicMenuItem:update()
         h = self.height - self.underline_h
     }
 
-    if lfs.attributes(self.filepath, "mode") == "directory" then
+    local file_mode = lfs.attributes(self.filepath, "mode")
+    if file_mode == "directory" then
         self.is_directory = true
         -- Directory : rounded corners
         local margin = Screen:scaleBySize(5) -- make directories less wide
@@ -395,6 +403,9 @@ function MosaicMenuItem:update()
             },
         }
     else
+        if file_mode ~= "file" then
+            self.file_deleted = true
+        end
         -- File : various appearances
         -- We'll draw a border around cover images, it may not be
         -- needed with some covers, but it's nicer when cover is
@@ -460,6 +471,8 @@ function MosaicMenuItem:update()
                         margin = 0,
                         padding = 0,
                         bordersize = border_size,
+                        dim = self.file_deleted,
+                        color = self.file_deleted and Blitbuffer.COLOR_GREY or nil,
                         image,
                     }
                 }
@@ -491,6 +504,7 @@ function MosaicMenuItem:update()
                         filename = self.text,
                         title = not bookinfo.ignore_meta and bookinfo.title,
                         authors = not bookinfo.ignore_meta and bookinfo.authors,
+                        file_deleted = self.file_deleted,
                     }
                 }
             end
@@ -521,14 +535,19 @@ function MosaicMenuItem:update()
                 }
             end
             -- Same as real FakeCover, but let it be squared (like a file)
+            local hint = "…" -- display hint it's being loaded
+            if self.file_deleted then -- unless file was deleted (can happen with History)
+                hint = _("(deleted)")
+            end
             widget = CenterContainer:new{
                 dimen = dimen,
                 FakeCover:new{
                     width = dimen.w,
                     height = dimen.h,
                     bordersize = border_size,
-                    filename = self.text .. "\n…", -- display hint it's being loaded
+                    filename = self.text .. "\n" .. hint,
                     initial_sizedec = 4, -- start with a smaller font when filenames only
+                    file_deleted = self.file_deleted,
                 }
             }
         end
@@ -725,7 +744,7 @@ function MosaicMenu:_updateItemsBuildUI()
         -- this is for focus manager
         table.insert(self.layout, {item_tmp})
 
-        if not item_tmp.bookinfo_found and not item_tmp.is_directory then
+        if not item_tmp.bookinfo_found and not item_tmp.is_directory and not item_tmp.file_deleted then
             -- Register this item for update
             table.insert(self.items_to_update, item_tmp)
         end