Skip to content

History: update last book access time#10156

Merged
hius07 merged 8 commits into
koreader:masterfrom
hius07:History-update-last-book-time
Feb 23, 2023
Merged

History: update last book access time#10156
hius07 merged 8 commits into
koreader:masterfrom
hius07:History-update-last-book-time

Conversation

@hius07

@hius07 hius07 commented Feb 22, 2023

Copy link
Copy Markdown
Member

Updates book access time on closing the document.
Do not use sdr mtime to get the last access time. See #10149 (comment)
(Minor optimization of calling secondsToDateTime).


This change is Reviewable

Comment thread frontend/readhistory.lua
Comment on lines -47 to +33
select_enabled_func = function()
return lfs.attributes(file_path, "mode") == "file"
end,
dim = is_file_deleted,
mandatory = datetime.secondsToDateTime(input_time),
select_enabled = not is_file_deleted,

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

No need for _func ? If we delete a file from history (with the option "removed deleted file from history" disabled), are we sure we go thru this buildEntry() to get updated info ?
edit: OK, I see below that you update this field when deleting. Keeping that comment just for you to answer "all is good and thought of" :)

Comment thread frontend/readhistory.lua Outdated
local now = os.time()
self.hist[1].time = now
self.hist[1].mandatory = datetime.secondsToDateTime(now)
self:_flush()

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Just asking, probably not worth complicating things :):
When switching documents, we will write the history to disk when closing the previous document, and also when loading the new document. No way to avoid one of them ? (ie. updating in memory self.hist, but avoiding the _flush() as not essential if it's still the last document?)

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Maybe the tearing_down flag can help, I'll look at it.

@NiLuJe NiLuJe left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Reviewed 3 of 5 files at r1, 2 of 2 files at r2, all commit messages.
Reviewable status: all files reviewed, 2 unresolved discussions (waiting on @hius07 and @poire-z)

@Frenzie Frenzie added this to the 2023.02 milestone Feb 22, 2023
@hius07 hius07 merged commit 5c9ba53 into koreader:master Feb 23, 2023
@hius07 hius07 deleted the History-update-last-book-time branch February 23, 2023 17:23
@Frenzie

Frenzie commented Feb 23, 2023

Copy link
Copy Markdown
Member

When I try to open the history I get this:

./luajit: frontend/ui/widget/touchmenu.lua:651: loop or previous error loading module 'apps/reader/readerui'
stack traceback:
        [C]: in function 'require'
        frontend/ui/widget/touchmenu.lua:651: in function 'handleEvent'
        frontend/ui/widget/container/widgetcontainer.lua:82: in function 'propagateEvent'
        frontend/ui/widget/container/widgetcontainer.lua:100: in function 'handleEvent'
        frontend/ui/uimanager.lua:191: in function 'close'
        frontend/apps/filemanager/filemanagermenu.lua:862: in function 'onCloseFileManagerMenu'
        frontend/apps/filemanager/filemanagermenu.lua:848: in function 'close_callback'
        frontend/ui/widget/touchmenu.lua:819: in function 'closeMenu'
        frontend/ui/widget/touchmenu.lua:920: in function 'onMenuSelect'
        frontend/ui/widget/touchmenu.lua:216: in function 'handleEvent'
        frontend/ui/widget/container/inputcontainer.lua:261: in function 'handleEvent'
        ...
        frontend/ui/widget/container/widgetcontainer.lua:100: in function 'handleEvent'
        frontend/ui/widget/container/widgetcontainer.lua:82: in function 'propagateEvent'
        frontend/ui/widget/container/widgetcontainer.lua:100: in function 'handleEvent'
        frontend/ui/uimanager.lua:875: in function 'sendEvent'
        frontend/ui/uimanager.lua:51: in function '__default__'
        frontend/ui/uimanager.lua:1404: in function 'handleInputEvent'
        frontend/ui/uimanager.lua:1502: in function 'handleInput'
        frontend/ui/uimanager.lua:1549: in function 'run'
        ./reader.lua:315: in main chunk
        [C]: at 0x55fa333127ab
~/src/koreader

Edit: never mind, doesn't seem to be due to this PR.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants