Move to archive plugin by mwoz123 · Pull Request #6101 · koreader/koreader

mwoz123 · 2020-04-28T18:57:06Z

Plugin to move/copy current reading book to archive folder

This change is

Frenzie

Interesting concept!

Frenzie · 2020-04-29T10:25:37Z

frontend/ui/elements/reader_menu_order.lua

        "zsync",
        "news_downloader",
        "send2ebook",
+        "moveToArchive",


This pushes more plugins to the next page, not necessarily the best move.

ok will be last here.

plugins/moveToArchive.koplugin/main.lua

Robertmccalister747 · 2020-04-29T11:20:49Z

Thanks. Maybe you can send me some code

…

On Wed, Apr 29, 2020, 3:33 AM Frans de Jonge ***@***.***> wrote: ***@***.**** commented on this pull request. Interesting concept! ------------------------------ In frontend/ui/elements/reader_menu_order.lua <#6101 (comment)>: > @@ -120,6 +120,7 @@ local order = { "zsync", "news_downloader", "send2ebook", + "moveToArchive", This pushes more plugins to the next page, not necessarily the best move. ------------------------------ In plugins/moveToArchive.koplugin/main.lua <#6101 (comment)>: > + text = _("Move to Archive"), + sub_item_table = { + { + text = _("Move current book to archive"), + callback = self.moveToArchive, + }, + { + text = _("Copy current book to archive"), + callback = self.copyToArchive, + }, + { + text = _("Go to archive folder"), + callback = function() + if not archive_dir_path then + UIManager:show(InfoMessage:new{ + text = _("No archive directory. Please set it first") You could make it a simple ConfirmBox. Please set an archive folder first. Cancel/Set archive folder ------------------------------ In plugins/moveToArchive.koplugin/main.lua <#6101 (comment)>: > + }) + return + end + if FileManager.instance then + FileManager.instance:reinit(archive_dir_path) + else + FileManager:showFiles(archive_dir_path) + end + end, + }, + { + text = _("Go to last copied/moved from folder"), + callback = function() + if not last_copied_from_dir then + UIManager:show(InfoMessage:new{ + text = _("No previous dir found") ⬇️ Suggested change - text = _("No previous dir found") + text = _("No previous folder found.") ------------------------------ In plugins/moveToArchive.koplugin/main.lua <#6101 (comment)>: > + end + end, + }, + { + text = _("Set archive directory"), + keep_menu_open = true, + callback = self.setArchiveDirectory, + } + }, + } +end + +function MoveToArchive:moveToArchive() + if not archive_dir_path then + UIManager:show(InfoMessage:new{ + text = _("No archive directory. Please set it first") Same as above ------------------------------ In plugins/moveToArchive.koplugin/main.lua <#6101 (comment)>: > + text = _("No archive directory. Please set it first") + }) + return + end + local document_full_path = G_reader_settings:readSetting("lastfile") + last_copied_from_dir = util.splitFilePathName(document_full_path) + logger.dbg("MoveToArchive: last_copied_from_dir :", last_copied_from_dir) + + FileManager:moveFile(document_full_path, archive_dir_path) + + move_to_archive_settings:saveSetting(last_copied_from_config_key, ("%s/"):format(last_copied_from_dir)) + + ReadHistory:removeItemByPath(document_full_path) + + UIManager:show(InfoMessage:new{ + text = _("Book moved. Please reopen it from archive location.") I'd be inclined to leave off the second sentence or to make it a ConfirmBox. ------------------------------ In plugins/moveToArchive.koplugin/main.lua <#6101 (comment)>: > + + FileManager:moveFile(document_full_path, archive_dir_path) + + move_to_archive_settings:saveSetting(last_copied_from_config_key, ("%s/"):format(last_copied_from_dir)) + + ReadHistory:removeItemByPath(document_full_path) + + UIManager:show(InfoMessage:new{ + text = _("Book moved. Please reopen it from archive location.") + }) +end + +function MoveToArchive:copyToArchive() + if not archive_dir_path then + UIManager:show(InfoMessage:new{ + text = _("No archive directory. Please set it first") Same as above ------------------------------ In plugins/moveToArchive.koplugin/main.lua <#6101 (comment)>: > + if not archive_dir_path then + UIManager:show(InfoMessage:new{ + text = _("No archive directory. Please set it first") + }) + return + end + local document_full_path = G_reader_settings:readSetting("lastfile") + last_copied_from_dir = util.splitFilePathName(document_full_path) + + logger.dbg("MoveToArchive: last_copied_from_dir :", last_copied_from_dir) + move_to_archive_settings:saveSetting(last_copied_from_config_key, ("%s/"):format(last_copied_from_dir)) + + FileManager:cpFile(document_full_path, archive_dir_path) + + UIManager:show(InfoMessage:new{ + text = _("Book copied. Please reopen it from archive location if needed.") Same as above — You are receiving this because you are subscribed to this thread. Reply to this email directly, view it on GitHub <#6101 (review)>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/APIOXUJAVLJMTYLRMTZXIBTRO77AHANCNFSM4MTCVEDA> .

mwoz123 · 2020-04-29T11:22:08Z

@Robertmccalister747 I don't understand what you mean. Code is in this repo/PR.

mwoz123 · 2020-04-29T14:06:49Z

Interesting concept!

Thanks:)

mwoz123 · 2020-04-29T15:11:12Z

Changes done.

plugins/moveToArchive.koplugin/_meta.lua

frontend/ui/elements/reader_menu_order.lua

mwoz123 · 2020-04-29T15:43:34Z

Changes done.

Frenzie

A few minor string nitpicks

plugins/moveToArchive.koplugin/main.lua

Frenzie · 2020-04-29T16:05:34Z

plugins/moveToArchive.koplugin/main.lua

+end
+
+function MoveToArchive:copyToArchive()
+    local copy_done_text =_("Book copied. \nDo you want to open it from archive folder?")


Suggested change

local copy_done_text =_("Book copied. \nDo you want to open it from archive folder?")

local copy_done_text =_("Book copied.\nDo you want to open it from the archive folder?")

plugins/moveToArchive.koplugin/main.lua

Frenzie · 2020-04-29T16:08:01Z

plugins/moveToArchive.koplugin/main.lua

+
+function MoveToArchive:addToMainMenu(menu_items)
+    menu_items.move_to_archive = {
+        text = _("Move to Archive"),


Suggested change

text = _("Move to Archive"),

text = _("Move to archive"),

Frenzie · 2020-04-29T16:09:27Z

frontend/ui/elements/reader_menu_order.lua

        "news_downloader",
        "send2ebook",
        "text_editor",
+        "move_to_archive",


I'm inclined to think this would fit better near progress_sync if we insert it in this part of the menu. The more_plugins menu will be pushed down no matter where above it you insert an extra entry.

Pinging @NiLuJe @poire-z @pazos for some thoughts.

I guess the more_plugins menu isn't really needed anymore, now that the user can disable the plugins he doesn't need.
If you put some in more_plugins, no matter the user cleaning, he would have to tap once more to reach that unfortunate plugin we put into more_plugins.
If we'd want to keep more_plugins, only non-user-action plugins (those with only settings or some permanent behaviour) should go in there.
So, I wouldn't mind an initial Plugins menu with 20 items :) as I can clean it up to the 5 that matters to me.

Yes, I have only a few plugins activated, and definitely do not need More plugins menu.

I agree that the more plugins menu in not necessary, and just annoying for the plugins that i use that are there. (I only use ~5 plugins)

mwoz123 · 2020-04-29T17:28:54Z

A few minor string nitpicks

Resolved

Frenzie

lgtm, letting it sit for another day or maybe two in case anybody else has any comments

mwoz123 · 2020-04-29T17:51:51Z

Btw off topic : do we still need "more plugins" as there's something like "next page" in menu if there're too many entries?

Imho it's misleading.

Frenzie · 2020-04-29T17:57:33Z

Like @poire-z said we should organize it differently.

poire-z · 2020-04-29T18:23:37Z

Just unsure what's happening when moving:

you're currently reading a book
you move the book file (only the file, not the docsetting (book.sdr/metadata.lua ?)
you quit the book, the docsetting is saved and stays at the old place?

If that is what's really happening, you have:

and old docsetting unused and dangling
possibilities for undefined behaviours in other contexts as that's a case we don't expect until now: having the book opened but no underlying file anymore

In FileManager, when we move/rename a file (and we do that the book closed), there are a bit more stuff taken care of:

koreader/frontend/apps/filemanager/filemanager.lua

Lines 782 to 806 in be72203

    
           local function infoMoveFile() 
        
               -- if we move a file, also move its sidecar directory 
        
               if DocSettings:hasSidecarFile(orig) then 
        
                   self:moveFile(DocSettings:getSidecarDir(orig), dest) -- dest is always a directory 
        
               end 
        
               if self:moveFile(orig, dest) then 
        
                   -- Update history and collections. 
        
                   local dest_file = string.format("%s/%s", dest, BaseUtil.basename(orig)) 
        
                   require("readhistory"):updateItemByPath(orig, dest_file) 
        
                   ReadCollection:updateItemByPath(orig, dest_file) 
        
                   -- Update last open file. 
        
                   if G_reader_settings:readSetting("lastfile") == orig then 
        
                       G_reader_settings:saveSetting("lastfile", dest_file) 
        
                   end 
        
                   UIManager:show(InfoMessage:new { 
        
                       text = T(_("Moved to: %1"), BD.dirpath(dest)), 
        
                       timeout = 2, 
        
                   }) 
        
               else 
        
                   UIManager:show(InfoMessage:new { 
        
                       text = T(_("An error occurred while trying to move %1"), BD.filepath(orig)), 
        
                       timeout = 2, 
        
                   }) 
        
               end 
        
           end

mwoz123 · 2020-04-29T18:45:11Z

Ok, cool off topic extracted to : #6105

mwoz123 · 2020-04-29T19:04:01Z

@poire-z right sdr is currently not moved.

This was designed mainly for wallabag /nd/S2E so it shouldn't be a big issue.

But anyway I wanted to add this next PR.
Are you ok with that?

poire-z · 2020-04-29T19:39:53Z

This was designed mainly for wallabag /nd/S2E so it shouldn't be a big issue.
But anyway I wanted to add this next PR.
Are you ok with that?

Files lying around is an issue for me :) People might test it tomorrow if merged, and that's rather not cool leaving mess on their device.
It's a plugin, so I wouldn't really mind it having bugs. But now that you know of them, it would be a bit unfortunate to not fix them before merging :)

But I reckon it's not easy fixing... Even if you move the docsettings, it will be saved at the old place when the book is closed. So, ideally, when moving, you would ask/tell the user that the file will be moved and re-opened, and if he says OK: close the book, move file + docsettings, re-open the book from the archived file (and if you propose and option to not re-open the book, what to do from there?).
And when you close the book, your plugin is gone... so... not easy (may be adding a after_close_callback in ReaderUI:switchDocument() like it's done for ReaderUI:reloadDocument(after_close_callback) )

Anyway, I let @Frenzie decide how critical that is :)
(I won't use this plugin, I don't really mind people testing it having mess on their devices, and it would probably take us quite a bit of time to help you getting it fully right :)

Frenzie · 2020-04-29T20:21:53Z

I didn't realize it didn't deal with SDRs properly; I'd definitely prefer to see that fixed first too.

mwoz123 · 2020-04-29T20:26:39Z

Ok will try.
Can you help me how to force close current book or force save unsaved book progress/changes in sdr?

poire-z · 2020-04-29T21:11:12Z

Can you help me how to force close current book or force save unsaved book progress/changes in sdr?

$ rgrep ui:onClose frontend/
frontend/apps/reader/modules/readermenu.lua:                self.ui:onClose()
frontend/apps/reader/modules/readermenu.lua:        self.ui:onClose()
frontend/apps/reader/modules/readergesture.lua:        self.ui:onClose()
frontend/apps/reader/modules/readerstatus.lua:    self.ui:onClose()

But it mostly depends on what you want to do after (open another document, go to file browser, exit koreader).

It's a bit unclear to me what's your workflow with this plugin (and what it will be for other people):

do you really need/want to stay reading the book after you archived it? Isn't it something you do when you're done with it?
if that's something you invoke at end of book, it could just be a new item in "Gear > Document > End of document action" and in it's "ask with popup dialog" - in which case, one just have to "go to end of book" to invoke this action/popup.
does "copy to archive folder" has some use case?

Co-authored-by: Frans de Jonge <fransdejonge@gmail.com>

frontend/apps/filemanager/filemanager.lua

Frenzie · 2020-05-02T17:37:25Z

plugins/movetoarchive.koplugin/main.lua

+
+local MoveToArchive = WidgetContainer:new{
+    name = "move2archive",
+    move_to_archive_settings = nil,


I can't tell what you're referring to in EverNote, but PerceptionExpander surely does not:

koreader/plugins/perceptionexpander.koplugin/main.lua

Lines 16 to 28 in be72203

local PerceptionExpander = Widget:extend{

is_enabled = nil,

name = "perceptionexpander",

page_counter = 0,

shift_each_pages = 100,

margin = 0.1,

line_thickness = 2,

line_color_intensity = 0.3,

margin_shift = 0.03,

settings = nil,

ALMOST_CENTER_OF_THE_SCREEN = 0.37,

last_screen_mode = nil

}

Note how it says settings, not perception_expander_settings.

NB Other plugins do it too is not a good argument.

Co-authored-by: Frans de Jonge <fransdejonge@gmail.com>

mwoz123 · 2020-05-02T19:52:48Z

done

Frenzie

Should be okay, good by you @poire-z ?

frontend/apps/filemanager/filemanager.lua

plugins/movetoarchive.koplugin/main.lua

poire-z · 2020-05-02T20:59:30Z

Looks fine, no more comment.

Frenzie · 2020-05-02T21:02:49Z

Thanks @mwoz123 !

poire-z · 2020-05-03T15:58:11Z

Can't be disabled. I guess it needs the names to be coherent with the plugin directory name:

--- a/plugins/movetoarchive.koplugin/_meta.lua
+++ b/plugins/movetoarchive.koplugin/_meta.lua
@@ -1,6 +1,6 @@
 local _ = require("gettext")
 return {
-    name = "move_to_archive",
+    name = "movetoarchive",
     fullname = _("Move to archive"),
     description = _([[Moves/copies current book to archive folder]]),
 }
diff --git a/plugins/movetoarchive.koplugin/main.lua b/plugins/movetoarchive.koplugin/main.lua
index f575a849..08cba643 100644
--- a/plugins/movetoarchive.koplugin/main.lua
+++ b/plugins/movetoarchive.koplugin/main.lua
@@ -14,7 +14,7 @@ local BaseUtil = require("ffi/util")
 local _ = require("gettext")

 local MoveToArchive = WidgetContainer:new{
-    name = "move2archive",
+    name = "movetoarchive",
 }

 function MoveToArchive:init()

Mismatches in these names prevent this plugin from being disabled'able. #6101 (comment)

Robertmccalister747 · 2020-05-05T15:02:59Z

Thank you for giving me the time of day lol

…

On Wed, Apr 29, 2020, 8:11 AM mwoz123 ***@***.***> wrote: Changes done. — You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub <#6101 (comment)>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/APIOXUK43GM3XDARNVIC3R3RPA7SDANCNFSM4MTCVEDA> .

mwoz123 added 3 commits April 28, 2020 15:13

wip: moveToArchive koplugin pleaceholder

918004a

move to archive 1.0 version

9010f30

fix linter issues

10baaf3

Frenzie added this to the 2020.05 milestone Apr 29, 2020

Frenzie added the Plugin label Apr 29, 2020

Frenzie reviewed Apr 29, 2020

View reviewed changes

mwoz123 added 2 commits April 29, 2020 15:47

confirm open after move/copy

07de2fa

refactor to common process

bed7430

fix accessing undefined var

1b84614

Frenzie reviewed Apr 29, 2020

View reviewed changes

plugins/moveToArchive.koplugin/_meta.lua Outdated Show resolved Hide resolved

frontend/ui/elements/reader_menu_order.lua Outdated Show resolved Hide resolved

code review change

597e391

Frenzie reviewed Apr 29, 2020

View reviewed changes

Code review changes

ae9b3d9

Frenzie approved these changes Apr 29, 2020

View reviewed changes

mwoz123 mentioned this pull request Apr 29, 2020

Reorganize "more plugins" submenu #6105

Closed

mwoz123 and others added 8 commits May 2, 2020 18:24

Update frontend/apps/filemanager/filemanager.lua

0ba7454

Co-authored-by: Frans de Jonge <fransdejonge@gmail.com>

Update frontend/apps/filemanager/filemanager.lua

3dece43

Co-authored-by: Frans de Jonge <fransdejonge@gmail.com>

Update frontend/apps/filemanager/filemanager.lua

e3193bc

Co-authored-by: Frans de Jonge <fransdejonge@gmail.com>

Update frontend/apps/filemanager/filemanager.lua

4f4588a

Co-authored-by: Frans de Jonge <fransdejonge@gmail.com>

Update frontend/apps/filemanager/filemanager.lua

f46adac

Co-authored-by: Frans de Jonge <fransdejonge@gmail.com>

Update frontend/apps/filemanager/filemanager.lua

9f118ae

Co-authored-by: Frans de Jonge <fransdejonge@gmail.com>

code review fixes

8aa5117

Whitespace

ac51306

Frenzie reviewed May 2, 2020

View reviewed changes

mwoz123 and others added 5 commits May 2, 2020 19:47

Update frontend/apps/filemanager/filemanager.lua

e4e1fff

Co-authored-by: Frans de Jonge <fransdejonge@gmail.com>

Update frontend/apps/filemanager/filemanager.lua

021ca3c

Co-authored-by: Frans de Jonge <fransdejonge@gmail.com>

x

008e050

y

738ce0c

z

bf17a7e

Frenzie approved these changes May 2, 2020

View reviewed changes

Frenzie added 2 commits May 2, 2020 22:53

Update frontend/apps/filemanager/filemanager.lua

e4cd107

Apply suggestions from code review

f97b3e8

Frenzie reviewed May 2, 2020

View reviewed changes

plugins/movetoarchive.koplugin/main.lua Outdated Show resolved Hide resolved

Update plugins/movetoarchive.koplugin/main.lua

7c10355

Frenzie merged commit ce3bf47 into koreader:master May 2, 2020

mwoz123 deleted the move-to-archive branch May 3, 2020 05:06

poire-z mentioned this pull request May 3, 2020

[Plugin] Fix move to archive plugin names #6113

Merged

Frenzie pushed a commit that referenced this pull request May 3, 2020

[Plugin] Fix move to archive plugin names (#6113)

eaabc42

Mismatches in these names prevent this plugin from being disabled'able. #6101 (comment)

poire-z mentioned this pull request Jun 26, 2021

FR: Please allow grouping of History items #7887

Closed

hius07 mentioned this pull request Oct 3, 2022

GH: Extend the issue template to request verbose debug logs for non-crash issues. #9585

Merged

	local copy_done_text =_("Book copied. \nDo you want to open it from archive folder?")
	local copy_done_text =_("Book copied.\nDo you want to open it from the archive folder?")

	local PerceptionExpander = Widget:extend{
	is_enabled = nil,
	name = "perceptionexpander",
	page_counter = 0,
	shift_each_pages = 100,
	margin = 0.1,
	line_thickness = 2,
	line_color_intensity = 0.3,
	margin_shift = 0.03,
	settings = nil,
	ALMOST_CENTER_OF_THE_SCREEN = 0.37,
	last_screen_mode = nil
	}

Uh oh!

Conversation

mwoz123 commented Apr 28, 2020 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Frenzie left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Robertmccalister747 commented Apr 29, 2020 via email

Uh oh!

mwoz123 commented Apr 29, 2020

Uh oh!

mwoz123 commented Apr 29, 2020

Uh oh!

mwoz123 commented Apr 29, 2020

Uh oh!

Uh oh!

Uh oh!

mwoz123 commented Apr 29, 2020

Uh oh!

Frenzie left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

poire-z Apr 29, 2020 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

mwoz123 commented Apr 29, 2020

Uh oh!

Frenzie left a comment

Choose a reason for hiding this comment

Uh oh!

mwoz123 commented Apr 29, 2020 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Frenzie commented Apr 29, 2020 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

poire-z commented Apr 29, 2020

Uh oh!

mwoz123 commented Apr 29, 2020

Uh oh!

mwoz123 commented Apr 29, 2020

Uh oh!

poire-z commented Apr 29, 2020

Uh oh!

Frenzie commented Apr 29, 2020

Uh oh!

mwoz123 commented Apr 29, 2020 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

poire-z commented Apr 29, 2020

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

mwoz123 commented Apr 28, 2020 •

edited

Loading

poire-z Apr 29, 2020 •

edited

Loading

mwoz123 commented Apr 29, 2020 •

edited

Loading

Frenzie commented Apr 29, 2020 •

edited

Loading

mwoz123 commented Apr 29, 2020 •

edited

Loading