From ced9e45d92d6d598998d4deea32b5849b610bf0e Mon Sep 17 00:00:00 2001 From: Frans de Jonge Date: Sat, 10 Feb 2018 18:36:18 +0100 Subject: [PATCH] [fix] DocumentRegistry: don't create empty sdr and add hasProvider() (#3675) As suggested by @poire-z https://github.com/koreader/koreader/pull/3653#issuecomment-364663156 --- frontend/apps/cloudstorage/dropboxapi.lua | 8 +++--- frontend/apps/cloudstorage/ftpapi.lua | 2 +- frontend/apps/filemanager/filemanager.lua | 8 +++--- .../filemanager/filemanagerfilesearcher.lua | 2 +- .../apps/filemanager/filemanagersearch.lua | 2 +- frontend/document/documentregistry.lua | 28 +++++++++++++++---- spec/unit/document_registry_spec.lua | 8 +++--- 7 files changed, 37 insertions(+), 21 deletions(-) diff --git a/frontend/apps/cloudstorage/dropboxapi.lua b/frontend/apps/cloudstorage/dropboxapi.lua index 85326a86a..212a5d68a 100644 --- a/frontend/apps/cloudstorage/dropboxapi.lua +++ b/frontend/apps/cloudstorage/dropboxapi.lua @@ -104,14 +104,14 @@ function DropBoxApi:listFolder(path, token) table.insert(dropbox_list, { text = text, url = files.path_display, - type = tag + type = tag, }) --show only file with supported formats - elseif tag == "file" and DocumentRegistry:getProvider(text) then + elseif tag == "file" and DocumentRegistry:hasProvider(text) then table.insert(dropbox_file, { text = text, url = files.path_display, - type = tag + type = tag, }) end end @@ -126,7 +126,7 @@ function DropBoxApi:listFolder(path, token) table.insert(dropbox_list, { text = files.text, url = files.url, - type = files.type + type = files.type, }) end return dropbox_list diff --git a/frontend/apps/cloudstorage/ftpapi.lua b/frontend/apps/cloudstorage/ftpapi.lua index 8b6ff5c75..faca68040 100644 --- a/frontend/apps/cloudstorage/ftpapi.lua +++ b/frontend/apps/cloudstorage/ftpapi.lua @@ -38,7 +38,7 @@ function FtpApi:listFolder(address_path,folder_path) type = type, }) --show only file with supported formats - elseif extension and DocumentRegistry:getProvider(item) then + elseif extension and DocumentRegistry:hasProvider(item) then type = "file" table.insert(ftp_file, { text = file_name, diff --git a/frontend/apps/filemanager/filemanager.lua b/frontend/apps/filemanager/filemanager.lua index 2051c08a2..7b68a62b4 100644 --- a/frontend/apps/filemanager/filemanager.lua +++ b/frontend/apps/filemanager/filemanager.lua @@ -149,7 +149,7 @@ function FileManager:init() has_close_button = true, perpage = G_reader_settings:readSetting("items_per_page"), file_filter = function(filename) - if DocumentRegistry:getProvider(filename) then + if DocumentRegistry:hasProvider(filename) then return true end end, @@ -631,14 +631,14 @@ function FileManager:deleteFile(file) return end - local is_doc = DocumentRegistry:getProvider(file_abs_path) + local is_doc = DocumentRegistry:hasProvider(file_abs_path) if lfs.attributes(file_abs_path, "mode") == "file" then ok, err = os.remove(file_abs_path) else ok, err = util.purgeDir(file_abs_path) end - if ok and err == nil then - if is_doc ~= nil then + if ok and not err then + if is_doc then DocSettings:open(file):purge() end UIManager:show(InfoMessage:new{ diff --git a/frontend/apps/filemanager/filemanagerfilesearcher.lua b/frontend/apps/filemanager/filemanagerfilesearcher.lua index 6be83617f..3d622bc37 100644 --- a/frontend/apps/filemanager/filemanagerfilesearcher.lua +++ b/frontend/apps/filemanager/filemanagerfilesearcher.lua @@ -41,7 +41,7 @@ function FileSearcher:readDir() if attributes.mode == "directory" and f ~= "." and f~=".." then table.insert(new_dirs, fullpath) table.insert(self.files, {name = f, path = fullpath, attr = attributes}) - elseif attributes.mode == "file" and DocumentRegistry:getProvider(fullpath) then + elseif attributes.mode == "file" and DocumentRegistry:hasProvider(fullpath) then table.insert(self.files, {name = f, path = fullpath, attr = attributes}) end end diff --git a/frontend/apps/filemanager/filemanagersearch.lua b/frontend/apps/filemanager/filemanagersearch.lua index afe5c7add..69d5f07e9 100644 --- a/frontend/apps/filemanager/filemanagersearch.lua +++ b/frontend/apps/filemanager/filemanagersearch.lua @@ -346,7 +346,7 @@ function Search:find(option) self.browse_tags[string.sub(j,2)] = (self.browse_tags[string.sub(j,2)] or 0) + 1 end end - if DocumentRegistry:getProvider(self.data[i][self.path]) then + if DocumentRegistry:hasProvider(self.data[i][self.path]) then if upsearch ~= "" then if string.find(search_content,upsearch,nil,true) then i = i + 1 diff --git a/frontend/document/documentregistry.lua b/frontend/document/documentregistry.lua index 32bde394e..b33a298ea 100644 --- a/frontend/document/documentregistry.lua +++ b/frontend/document/documentregistry.lua @@ -13,6 +13,7 @@ local T = require("ffi/util").template local DocumentRegistry = { registry = {}, providers = {}, + filetype_provider = {}, } function DocumentRegistry:addProvider(extension, mimetype, provider, weight) @@ -22,6 +23,19 @@ function DocumentRegistry:addProvider(extension, mimetype, provider, weight) provider = provider, weight = weight or 100, }) + self.filetype_provider[extension] = true +end + +--- Returns true if file has provider. +-- @string file +-- @treturn boolean +function DocumentRegistry:hasProvider(file) + local filename_suffix = util.getFileNameSuffix(file) + + if self.filetype_provider[filename_suffix] then + return true + end + return false end --- Returns the preferred registered document handler. @@ -32,12 +46,14 @@ function DocumentRegistry:getProvider(file) if providers then -- provider for document - local doc_settings_provider = require("docsettings"):open(file):readSetting("provider") - - if doc_settings_provider then - for _, provider in ipairs(providers) do - if provider.provider.provider == doc_settings_provider then - return provider.provider + local DocSettings = require("docsettings") + if DocSettings:hasSidecarFile(file) then + local doc_settings_provider = DocSettings:open(file):readSetting("provider") + if doc_settings_provider then + for _, provider in ipairs(providers) do + if provider.provider.provider == doc_settings_provider then + return provider.provider + end end end end diff --git a/spec/unit/document_registry_spec.lua b/spec/unit/document_registry_spec.lua index 3bf61730c..ea15e0d07 100644 --- a/spec/unit/document_registry_spec.lua +++ b/spec/unit/document_registry_spec.lua @@ -16,10 +16,10 @@ describe("document registry module", function() it("should return all supported rendering engines", function() local providers = DocumentRegistry:getProviders("bla.epub") - assert.is_equal("Cool Reader Engine", - providers[1].provider.provider_name) - assert.is_equal("MuPDF", - providers[2].provider.provider_name) + assert.is_equal("crengine", + providers[1].provider.provider) + assert.is_equal("mupdf", + providers[2].provider.provider) end) it("should set per-document setting for rendering engine", function()