diff --git a/base b/base index ba97174c9..dd2013f28 160000 --- a/base +++ b/base @@ -1 +1 @@ -Subproject commit ba97174c9a73754ec6e7b7ee38d7ecddd7fce0cb +Subproject commit dd2013f28352f6d0ca38204eecf9218b507c4c24 diff --git a/frontend/apps/reader/modules/readerdictionary.lua b/frontend/apps/reader/modules/readerdictionary.lua index dd8f7c300..752b1c8c0 100644 --- a/frontend/apps/reader/modules/readerdictionary.lua +++ b/frontend/apps/reader/modules/readerdictionary.lua @@ -5,6 +5,7 @@ local DictQuickLookup = require("ui/widget/dictquicklookup") local InfoMessage = require("ui/widget/infomessage") local InputContainer = require("ui/widget/container/inputcontainer") local JSON = require("json") +local Trapper = require("ui/trapper") local UIManager = require("ui/uimanager") local logger = require("logger") local util = require("util") @@ -88,7 +89,7 @@ function ReaderDictionary:init() if not G_reader_settings:readSetting("dicts_disabled") then -- Create an empty dict for this setting, so that we can - -- access and update it directly thru G_reader_settings + -- access and update it directly through G_reader_settings -- and it will automatically be saved. G_reader_settings:saveSetting("dicts_disabled", {}) end @@ -196,7 +197,10 @@ end function ReaderDictionary:onLookupWord(word, box, highlight, link) self.highlight = highlight - self:stardictLookup(word, box, link) + -- Wrapped through Trapper, as we may be using Trapper:dismissablePopen() in it + Trapper:wrap(function() + self:stardictLookup(word, box, link) + end) return true end @@ -353,8 +357,12 @@ function ReaderDictionary:stardictLookup(word, box, link) self:showDict(word, final_results, box) return end + local lookup_cancelled = false local common_options = self.disable_fuzzy_search and "-njf" or "-nj" for _, dict_dir in ipairs(dict_dirs) do + if lookup_cancelled then + break -- don't do any more lookup on additional dict_dirs + end local results_str = nil if Device:isAndroid() then local A = require("android") @@ -370,26 +378,51 @@ function ReaderDictionary:stardictLookup(word, box, link) if self.sdcv_dictnames_options_escaped then cmd = cmd .. " " .. self.sdcv_dictnames_options_escaped end - local std_out = io.popen(cmd, "r") - if std_out then - results_str = std_out:read("*all") - std_out:close() + -- cmd = "sleep 7 ; " .. cmd -- uncomment to simulate long lookup time + + if self.lookup_progress_msg then + -- Some sdcv lookups, when using fuzzy search with many dictionaries + -- and a really bad selected text, can take up to 10 seconds. + -- It is nice to be able to cancel it when noticing wrong text was selected. + -- As we have a lookup_progress_msg (that can be used to catch a tap + -- and trigger cancellation), and because sdcv starts outputing its + -- output only at the end when it has done its work, we can + -- use Trapper:dismissablePopen() to cancel it as long as we are waiting + -- for output. + -- We must ensure we will have some output to be readable (if no + -- definition found, sdcv will output some message on stderr, and + -- let stdout empty) by appending an "echo": + cmd = cmd .. "; echo" + local completed + completed, results_str = Trapper:dismissablePopen(cmd, self.lookup_progress_msg) + lookup_cancelled = not completed + else + -- Fuzzy search disabled, usual option for people who don't want + -- a "Looking up..." InfoMessage and usually fast: do a classic + -- blocking io.popen() + local std_out = io.popen(cmd, "r") + if std_out then + results_str = std_out:read("*all") + std_out:close() + end end end - local ok, results = pcall(JSON.decode, results_str) - if ok and results then - -- we may get duplicates (sdcv may do multiple queries, - -- in fixed mode then in fuzzy mode), we have to remove them - local h - for _,r in ipairs(results) do - h = r.dict .. r.word .. r.definition - if seen_results[h] == nil then - table.insert(final_results, r) - seen_results[h] = true + if results_str and results_str ~= "\n" then -- \n is when lookup was cancelled + local ok, results = pcall(JSON.decode, results_str) + if ok and results then + -- we may get duplicates (sdcv may do multiple queries, + -- in fixed mode then in fuzzy mode), we have to remove them + local h + for _,r in ipairs(results) do + h = r.dict .. r.word .. r.definition + if seen_results[h] == nil then + table.insert(final_results, r) + seen_results[h] = true + end end + else + logger.warn("JSON data cannot be decoded", results) end - else - logger.warn("JSON data cannot be decoded", results) end end if #final_results == 0 then @@ -398,7 +431,7 @@ function ReaderDictionary:stardictLookup(word, box, link) { dict = "", word = word, - definition = _("No definition found."), + definition = lookup_cancelled and _("Dictionary lookup canceled.") or _("No definition found."), } } end diff --git a/frontend/ui/trapper.lua b/frontend/ui/trapper.lua index 35926ce08..0f2e4f5a5 100644 --- a/frontend/ui/trapper.lua +++ b/frontend/ui/trapper.lua @@ -12,6 +12,7 @@ Mostly done with coroutines, but hides their usage for simplicity. local ConfirmBox = require("ui/widget/confirmbox") local InfoMessage = require("ui/widget/infomessage") local UIManager = require("ui/uimanager") +local ffiutil = require("ffi/util") local logger = require("logger") local _ = require("gettext") @@ -275,4 +276,123 @@ function Trapper:confirm(text, cancel_text, ok_text) return ret end + +--[[-- +Dismissable wrapper for @{io.popen|io.popen(`cmd`)}. + +Notes and limitations: + +1) It is dismissable as long as `cmd` as not yet output anything. + Once output has started, the reading will block till it is done. + (Some shell tricks, included in `cmd`, could probably be used to + accumulate `cmd` output in some variable, and to output the whole + variable to stdout at the end.) + +2) `cmd` needs to output something (we will wait till some data is available) + If there are chances for it to not output anything, append `"; echo"` to `cmd` + +3) We need an @{ui.widget.infomessage|InfoMessage}, that, as a modal, will catch + any @{ui.event|Tap event} happening during `cmd` execution. This can be + provided as a string (a new InfoMessage will be created), or can be an + existing already displayed InfoMessage. + +If we really need to have more control, we would need to use `select()` via `ffi` +or do low level non-blocking reading on the file descriptor. +If there are `cmd` that may not exit, that we would be trying to +collect indefinitely, the best option would be to compile any `timeout.c` +and use it as a wrapper. + +@string cmd shell `cmd` to execute and get output from +@param infomessage string or already shown @{ui.widget.infomessage|InfoMessage} widget instance +@int check_interval_sec[opt=0.1] float interval in second for checking pipe for available output +@treturn boolean completed (`true` if not interrupted, `false` if dismissed) +@treturn string output of command +]] +function Trapper:dismissablePopen(cmd, infomessage, check_interval_sec) + local _coroutine = coroutine.running() + -- assert(_coroutine ~= nil, "Need to be called from a coroutine") + if not _coroutine then + logger.warn("unwrapped dismissablePopen(), falling back to blocking io.popen()") + local std_out = io.popen(cmd, "r") + if std_out then + local output = std_out:read("*all") + std_out:close() + return true, output + end + return false + end + + local own_infomessage = false + if type(infomessage) == "string" then + infomessage = InfoMessage:new{text = infomessage} + UIManager:show(infomessage) + UIManager:forceRePaint() + own_infomessage = true + end + infomessage.dismiss_callback = function() + -- this callback will resume us at coroutine.yield() below + -- with a go_on = false + coroutine.resume(_coroutine, false) + end + + if not check_interval_sec then + check_interval_sec = 0.1 -- default: check for output every 100ms + end + local collect_interval_sec = 5 -- collect cancelled cmd every 5 second, no hurry + local completed = false + local output = nil + + local std_out = io.popen(cmd, "r") + if std_out then + -- We check regularly if data is available to be read, and we give + -- control in the meantime to UIManager so our InfoMessage.dismiss_callback + -- get a chance to be triggered, in which case we won't wait for reading, + -- We'll schedule a background function to collect the uneeded output and + -- close the pipe later. + while true do + -- The following function will resume us at coroutine.yield() below + -- with a go_on = true + local go_on_func = function() coroutine.resume(_coroutine, true) end + UIManager:scheduleIn(check_interval_sec, go_on_func) -- called in 100ms by default + local go_on = coroutine.yield() -- gives control back to UIManager + if not go_on then -- the dismiss_callback resumed us + UIManager:unschedule(go_on_func) + -- We forget cmd here, but something has to collect + -- its output and close the pipe to not leak file handles and + -- zombie processes. + local collect_and_clean + collect_and_clean = function() + if ffiutil.getNonBlockingReadSize(std_out) ~= 0 then -- cmd started outputing + std_out:read("*all") + std_out:close() + logger.dbg("collected cancelled cmd output") + else -- no output yet, reschedule + UIManager:scheduleIn(collect_interval_sec, collect_and_clean) + logger.dbg("cancelled cmd output not yet collectable") + end + end + UIManager:scheduleIn(collect_interval_sec, collect_and_clean) + break + end + -- the go_on_func resumed us, check if pipe is ready to be read + if ffiutil.getNonBlockingReadSize(std_out) ~= 0 then + -- Some data is available for reading: read it all, + -- but we may block from now on + output = std_out:read("*all") + std_out:close() + completed = true + break + end + -- logger.dbg("no cmd output yet, will check again soon") + end + end + if own_infomessage then + -- Remove our own infomessage + UIManager:close(infomessage) + UIManager:forceRePaint() + end + -- return what we got or not to our caller + return completed, output +end + return Trapper