File browser, Collection: improve group actions #11178

hius07 · 2023-11-29T06:31:01Z

In file group actions (copy, move, delete files or folders) we maintain History and Collection.
With these changes flushing History and Collection is done not for every file but at the end of the action.

ReadCollection (rewritten) now does not ask the storage on each favorites status request.
(Opens the door for long awaiting favorites marker)

Currently one collection only is supported (Favorites) but the infrastructure is done ready for other collections, UI changes only needed.

This change is

poire-z

That's a lot, just had a quick glance, trusting you.
Thanks for the needed revamping of readcollection :)

poire-z · 2023-11-29T09:42:33Z

frontend/apps/filemanager/filemanager.lua

+function FileManager:pasteFileFromClipboard(file)
    local orig_file = BaseUtil.realpath(self.clipboard)


Just asking: can we get here with not a filepath in the clipboard (eg. selected text from a book)? And what would happen?

Filemanager self.clipboard is not a system clipboard, currently there is no way to fill it with anything other than a filepath.

poire-z · 2023-11-29T09:45:29Z

frontend/apps/filemanager/filemanager.lua

+        local text1, text2
+        if self.cutfile then
+            text1 = "1 file was not moved"
+            text2 = "%1 files were not moved"
+        else
+            text1 = "1 file was not copied"
+            text2 = "%1 files were not copied"
+        end
+        UIManager:show(InfoMessage:new{
+            text = T(N_(text1, text2, skipped_nb), skipped_nb),


@Frenzie : will this be properly picked up by the translatable strings extraction stuff?

It won't, it's just a relatively dumb text match.

poire-z · 2023-11-29T09:50:46Z

frontend/readcollection.lua

-        if v.file == item then
-            table.remove(coll, k)
-            break
+function ReadCollection:isAdded(file, collection_name)


Reminds me of #11108 (comment) :)
:hasFile() ?

poire-z · 2023-11-29T09:53:46Z

frontend/readcollection.lua

 end

+function ReadCollection:updateItemsByPath(path, new_path)
+    path = "^" .. path .. "/"


(Reminds me of another recent comment :)
Possible risk if using :gsub() and path contains some pattern, might be safer to use string comparison like util.stringStartsWith() ?

Yes, I remember #11090 (comment) about string.match.
But here we need to do actual gsub, to replace old path with the new one.
One more place is

koreader/frontend/readhistory.lua

Lines 194 to 198 in dce12de

function ReadHistory:updateItemsByPath(old_path, new_path)

old_path = "^"..old_path

local history_updated

for i, v in ipairs(self.hist) do

local file, count = v.file:gsub(old_path, new_path)

But here we need to do actual gsub, to replace old path with the new one

But can this :gsub fail if paths happen to have crap characters interpreted as pattern ? (If yes, we may need to try to escape these chars.)

That's an interesting conundrum.

Escaping ought to be relatively simple (e.g., string:gsub("%W", "%%%1"), but I imagine you'll need to unescape it after that, and it might get trickier there ;p.

Maybe string:gsub("%%(%W)", "%1") ought to do the trick to handle unescaping...

poire-z · 2023-12-04T08:31:00Z

frontend/readhistory.lua

+        if v.file:sub(1, l) == old_path then
+            self.hist[i].file = new_path .. v.file:sub(l + 1)


local l = #old_path

The choice of l as the variable name makes this hard to read :) may be call it len ?

NiLuJe · 2023-12-10T17:50:13Z

frontend/readhistory.lua

@@ -19,7 +20,7 @@ local ReadHistory = {

 local function getMandatory(date_time)
    return G_reader_settings:isTrue("history_datetime_short")
-        and datetime.secondsToDate(date_time):sub(3) or datetime.secondsToDateTime(date_time)
+        and os.date(C_("Date string", "%y-%m-%d"), date_time) or datetime.secondsToDateTime(date_time)


Missed that one; I would perhaps have extended datetime.secondsToDate to provide the century-less format directly via an extra argument instead of hard-coding it here?

updateItemByPath has recently been replaced with updateItem in both readhistory and readcollection. Fixes: aabd6d7 ("File browser, Collection: improve group actions (koreader#11178)")

updateItemByPath has recently been replaced with updateItem in both readhistory and readcollection. Fixes: aabd6d7 ("File browser, Collection: improve group actions (koreader#11178)") Fixes: koreader#11320

updateItemByPath has recently been replaced with updateItem in both readhistory and readcollection. Fixes: aabd6d7 ("File browser, Collection: improve group actions (#11178)") Fixes: #11320

hius07 added 8 commits November 29, 2023 08:17

readcollection: full update

d0ec7b9

readhistory: group actions

68b49ea

filemanagerutil: update ReadCollection

0b424dc

filemanagercollection: update ReadCollection

5f017d7

filemanager: update group actions

55ef7a1

Update filemanager_spec.lua

8f2f37b

Update filemanager.lua

1ece0b4

Update filemanager.lua

30e5d16

poire-z reviewed Nov 29, 2023

View reviewed changes

Frenzie added this to the 2023.12 milestone Nov 29, 2023

hius07 added 8 commits November 29, 2023 19:41

readcollection: hasFile()

9463f97

filemanagerutil: hasFile()

4c5bbc6

filemanagercollection: hasFile()

e43a764

filemanager: correct N_()

3d4c161

readhistory: fix translation of shortened datetime

2e08be6

filemanagershortcuts: pasteFileFromClipboard() (new name)

672945f

readcollection: avoid gsub on path

507abf6

readhistory: avoid gsub on path

604ce27

poire-z reviewed Dec 4, 2023

View reviewed changes

hius07 added 2 commits December 4, 2023 11:07

readcollection: len

afac195

readhistory: len

f5416d8

hius07 merged commit aabd6d7 into koreader:master Dec 10, 2023
3 checks passed

hius07 deleted the coll-group branch December 10, 2023 06:05

NiLuJe reviewed Dec 10, 2023

View reviewed changes

liskin mentioned this pull request Jan 5, 2024

Fix "Move current book to archive" #11321

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

File browser, Collection: improve group actions #11178

File browser, Collection: improve group actions #11178

hius07 commented Nov 29, 2023 •

edited by Frenzie

poire-z left a comment

poire-z Nov 29, 2023

hius07 Nov 29, 2023

poire-z Nov 29, 2023

Frenzie Nov 29, 2023

poire-z Nov 29, 2023

poire-z Nov 29, 2023

hius07 Nov 29, 2023

poire-z Dec 3, 2023

NiLuJe Dec 3, 2023 •

edited

NiLuJe Dec 3, 2023

poire-z Dec 4, 2023

NiLuJe Dec 10, 2023

		function FileManager:pasteFileFromClipboard(file)
		local orig_file = BaseUtil.realpath(self.clipboard)

	function ReadHistory:updateItemsByPath(old_path, new_path)
	old_path = "^"..old_path
	local history_updated
	for i, v in ipairs(self.hist) do
	local file, count = v.file:gsub(old_path, new_path)

		if v.file:sub(1, l) == old_path then
		self.hist[i].file = new_path .. v.file:sub(l + 1)

File browser, Collection: improve group actions #11178

File browser, Collection: improve group actions #11178

Conversation

hius07 commented Nov 29, 2023 • edited by Frenzie

poire-z left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

NiLuJe Dec 3, 2023 • edited

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

hius07 commented Nov 29, 2023 •

edited by Frenzie

NiLuJe Dec 3, 2023 •

edited