From 2e417cfbd8676bb6097d39f79159a39aed01d7dc Mon Sep 17 00:00:00 2001 From: Qingping Hou Date: Fri, 3 Jun 2016 22:05:14 -0700 Subject: [PATCH] filemanager(refactor): use purge method from docsettings --- .ci/script.sh | 2 +- frontend/apps/filemanager/filemanager.lua | 11 ++------ frontend/docsettings.lua | 34 ++++++++++------------- spec/unit/docsettings_spec.lua | 14 ++++++---- spec/unit/filemanager_spec.lua | 19 +++++++------ spec/unit/readerbookmark_spec.lua | 4 +-- spec/unit/readerui_spec.lua | 15 +++++----- 7 files changed, 46 insertions(+), 53 deletions(-) diff --git a/.ci/script.sh b/.ci/script.sh index 701ff6534..62ff30eda 100755 --- a/.ci/script.sh +++ b/.ci/script.sh @@ -8,4 +8,4 @@ make all retry_cmd 2 make testfront set +o pipefail luajit $(which luacheck) --no-color -q frontend | tee ./luacheck.out -test $(grep Total ./luacheck.out | awk '{print $2}') -le 54 +test $(grep Total ./luacheck.out | awk '{print $2}') -le 53 diff --git a/frontend/apps/filemanager/filemanager.lua b/frontend/apps/filemanager/filemanager.lua index 16e33ed29..9c1be469a 100644 --- a/frontend/apps/filemanager/filemanager.lua +++ b/frontend/apps/filemanager/filemanager.lua @@ -12,7 +12,7 @@ local FileChooser = require("ui/widget/filechooser") local TextWidget = require("ui/widget/textwidget") local Blitbuffer = require("ffi/blitbuffer") local lfs = require("libs/libkoreader-lfs") -local docsettings = require("docsettings") +local DocSettings = require("docsettings") local UIManager = require("ui/uimanager") local Screen = require("device").screen local Geom = require("ui/geometry") @@ -309,14 +309,7 @@ function FileManager:deleteFile(file) ok, err = os.remove(file_abs_path) if ok and err == nil then if is_doc ~= nil then - -- also delete history/settings for documents - local sidecar_dir = docsettings:getSidecarDir(file_abs_path) - if lfs.attributes(sidecar_dir, "mode") == "directory" then - util.purgeDir(sidecar_dir) - end - local legacy_history_file = docsettings:getHistoryPath(file) - -- @todo: check return from os.remove - os.remove(legacy_history_file) + DocSettings:open(file):purge() end UIManager:show(InfoMessage:new{ text = util.template(_("Deleted %1"), file), diff --git a/frontend/docsettings.lua b/frontend/docsettings.lua index 1827edbbd..1943d992e 100644 --- a/frontend/docsettings.lua +++ b/frontend/docsettings.lua @@ -5,7 +5,8 @@ local purgeDir = require("ffi/util").purgeDir local DocSettings = {} -local history_dir = DataStorage:getHistoryDir() +local HISTORY_DIR = DataStorage:getHistoryDir() +local READER_SETTING_FILE = DataStorage:getDataDir() .. "/settings.reader.lua" local function buildCandidate(file_path) if lfs.attributes(file_path, "mode") == "file" then @@ -21,7 +22,7 @@ function DocSettings:getSidecarDir(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) @@ -39,24 +40,19 @@ function DocSettings:getNameFromHistory(hist_name) return string.sub(hist_name, s+2, -5) end -function DocSettings:purgeDocSettings(doc_path) - purgeDir(self:getSidecarDir(doc_path)) - os.remove(self:getHistoryPath(doc_path)) -end - function DocSettings:open(docfile) -- TODO(zijiehe): Remove history_path, use only sidecar. local new = { data = {} } local ok, stored if docfile == ".reader" then -- we handle reader setting as special case - new.history_file = DataStorage:getDataDir() .. "/settings.reader.lua" - + new.history_file = READER_SETTING_FILE ok, stored = pcall(dofile, new.history_file) else new.history_file = self:getHistoryPath(docfile) local sidecar = self:getSidecarDir(docfile) + new.sidecar = sidecar if lfs.attributes(sidecar, "mode") ~= "directory" then lfs.mkdir(sidecar) end @@ -67,19 +63,19 @@ function DocSettings:open(docfile) -- can handle two files with only different suffixes. new.sidecar_file = sidecar.."/metadata.".. docfile:match(".*%.(.*)")..".lua" + new.legacy_sidecar_file = sidecar.."/".. + docfile:match(".*%/(.*)")..".lua" end local candidates = {} -- New sidecar file - table.insert(candidates, buildCandidate(new.sidecar_file)); + table.insert(candidates, buildCandidate(new.sidecar_file)) -- Legacy sidecar file - table.insert(candidates, buildCandidate( - self:getSidecarDir(docfile).."/".. - docfile:match(".*%/(.*)")..".lua")) + table.insert(candidates, buildCandidate(new.legacy_sidecar_file)) -- Legacy history folder - table.insert(candidates, buildCandidate(new.history_file)); + table.insert(candidates, buildCandidate(new.history_file)) -- Legacy kpdfview setting - table.insert(candidates, buildCandidate(docfile..".kpdfview.lua")); + table.insert(candidates, buildCandidate(docfile..".kpdfview.lua")) table.sort(candidates, function(l, r) if l == nil then return false @@ -100,7 +96,7 @@ function DocSettings:open(docfile) new.data = stored end - return setmetatable(new, { __index = DocSettings}) + return setmetatable(new, {__index = DocSettings}) end function DocSettings:readSetting(key) @@ -144,12 +140,12 @@ function DocSettings:close() self:flush() end -function DocSettings:clear() +function DocSettings:purge() if self.history_file then os.remove(self.history_file) end - if self.sidecar_file then - os.remove(self.sidecar_file) + if lfs.attributes(self.sidecar, "mode") == "directory" then + purgeDir(self.sidecar) end end diff --git a/spec/unit/docsettings_spec.lua b/spec/unit/docsettings_spec.lua index a7a79edb4..743a8b802 100644 --- a/spec/unit/docsettings_spec.lua +++ b/spec/unit/docsettings_spec.lua @@ -1,10 +1,12 @@ -require("commonrequire") -local doc = require("docsettings") - describe("docsettings module", function() + local docsettings + setup(function() + require("commonrequire") + docsettings = require("docsettings") + end) it("should generate sidecar directory path", function() - assert.Equals("../../foo.sdr", doc:getSidecarDir("../../foo.pdf")) - assert.Equals("/foo/bar.sdr", doc:getSidecarDir("/foo/bar.pdf")) - assert.Equals("baz.sdr", doc:getSidecarDir("baz.pdf")) + assert.Equals("../../foo.sdr", docsettings:getSidecarDir("../../foo.pdf")) + assert.Equals("/foo/bar.sdr", docsettings:getSidecarDir("/foo/bar.pdf")) + assert.Equals("baz.sdr", docsettings:getSidecarDir("baz.pdf")) end) end) diff --git a/spec/unit/filemanager_spec.lua b/spec/unit/filemanager_spec.lua index e1af0fba3..e57303a3f 100644 --- a/spec/unit/filemanager_spec.lua +++ b/spec/unit/filemanager_spec.lua @@ -1,13 +1,14 @@ -require("commonrequire") -local FileManager = require("apps/filemanager/filemanager") -local lfs = require("libs/libkoreader-lfs") -local docsettings = require("docsettings") -local UIManager = require("ui/uimanager") -local Screen = require("device").screen -local util = require("ffi/util") -local DEBUG = require("dbg") - describe("FileManager module", function() + local FileManager, lfs, docsettings, UIManager, Screen, util + setup(function() + require("commonrequire") + FileManager = require("apps/filemanager/filemanager") + lfs = require("libs/libkoreader-lfs") + docsettings = require("docsettings") + UIManager = require("ui/uimanager") + Screen = require("device").screen + util = require("ffi/util") + end) it("should show file manager", function() UIManager:quit() local filemanager = FileManager:new{ diff --git a/spec/unit/readerbookmark_spec.lua b/spec/unit/readerbookmark_spec.lua index 3cc8e6a9a..057a82233 100644 --- a/spec/unit/readerbookmark_spec.lua +++ b/spec/unit/readerbookmark_spec.lua @@ -48,7 +48,7 @@ describe("ReaderBookmark module", function() local page = 10 local readerui setup(function() - DocSettings:purgeDocSettings(sample_epub) + DocSettings:open(sample_epub):purge() readerui = ReaderUI:new{ document = DocumentRegistry:openDocument(sample_epub), } @@ -130,7 +130,7 @@ describe("ReaderBookmark module", function() describe("bookmark for PDF document", function() local readerui setup(function() - DocSettings:purgeDocSettings(sample_pdf) + DocSettings:open(sample_pdf):purge() readerui = ReaderUI:new{ document = DocumentRegistry:openDocument(sample_pdf), } diff --git a/spec/unit/readerui_spec.lua b/spec/unit/readerui_spec.lua index da94bd58a..3ce53eec1 100644 --- a/spec/unit/readerui_spec.lua +++ b/spec/unit/readerui_spec.lua @@ -1,20 +1,21 @@ -require("commonrequire") -local DocumentRegistry = require("document/documentregistry") -local ReaderUI = require("apps/reader/readerui") -local DocSettings = require("docsettings") -local UIManager = require("ui/uimanager") - describe("Readerui module", function() + local DocumentRegistry, ReaderUI, DocSettings, UIManager local sample_epub = "spec/front/unit/data/leaves.epub" local readerui setup(function() + require("commonrequire") + DocumentRegistry = require("document/documentregistry") + ReaderUI = require("apps/reader/readerui") + DocSettings = require("docsettings") + UIManager = require("ui/uimanager") + readerui = ReaderUI:new{ document = DocumentRegistry:openDocument(sample_epub), } end) it("should save settings", function() -- remove history settings and sidecar settings - DocSettings:open(sample_epub):clear() + DocSettings:open(sample_epub):purge() local doc_settings = DocSettings:open(sample_epub) assert.are.same(doc_settings.data, {}) readerui:saveSettings()