Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Re-add file sort by "last date read" #10682

Merged
merged 2 commits into from Jul 12, 2023
Merged

Conversation

snelg
Copy link
Contributor

@snelg snelg commented Jul 12, 2023

Partial un-do of #10627 : access is not the same as modified, so we want to keep the "last read date" file sort option in addition to the "date modified" option


This change is Reviewable

@NiLuJe
Copy link
Member

NiLuJe commented Jul 12, 2023

See #10627 (comment)


I can confirm that behavior, but I'm slightly confused.

Do we touch the file or timestamps ourselves somewhere?

When I tested the atime things the first time we decided to kill it, I was doing synthetic tests w/ stuff like touch, less & vim. And that didn't do much, as expected (noatime & co).

But, if I actually open a book in KOReader (doesn't matter if CRe or PDF), we do affect both its atime and chtime...

(At... slightly different times, as can be seen on a real system with high-res timestamps. And, yes, this is a noatime-mounted tmpfs ;o)).

┌─(niluje@ahsoka:pts/16)───────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────(/var/tmp/niluje/koreader/koreader-emulator-x86_64-pc-linux-gnu-debug/koreader)─┐
└─(0.17:18%:03:54:%)── stat /var/tmp/niluje/koreader/test/juliet.epub                                                                                                                                                                                                                                                                                  ──(Wed, Jul 12)─┘
  File: /var/tmp/niluje/koreader/test/juliet.epub
  Size: 839144          Blocks: 1640       IO Block: 4096   regular file
Device: 0,33    Inode: 1262789     Links: 1
Access: (0644/-rw-r--r--)  Uid: ( 1000/  niluje)   Gid: ( 1000/  niluje)
Access: 2023-07-12 03:12:38.000000000 +0200
Modify: 2023-07-12 02:44:23.000000000 +0200
Change: 2023-07-12 03:12:38.533349776 +0200
 Birth: 2023-07-12 02:44:23.825471810 +0200
┌─(niluje@ahsoka:pts/16)───────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────(/var/tmp/niluje/koreader/koreader-emulator-x86_64-pc-linux-gnu-debug/koreader)─┐
└─(0.17:18%:03:54:%)── stat /var/tmp/niluje/koreader/test/juliet.epub                                                                                                                                                                                                                                                                                  ──(Wed, Jul 12)─┘
  File: /var/tmp/niluje/koreader/test/juliet.epub
  Size: 839144          Blocks: 1640       IO Block: 4096   regular file
Device: 0,33    Inode: 1262789     Links: 1
Access: (0644/-rw-r--r--)  Uid: ( 1000/  niluje)   Gid: ( 1000/  niluje)
Access: 2023-07-12 03:54:42.000000000 +0200
Modify: 2023-07-12 02:44:23.000000000 +0200
Change: 2023-07-12 03:54:42.496024101 +0200
 Birth: 2023-07-12 02:44:23.825471810 +0200

@NiLuJe
Copy link
Member

NiLuJe commented Jul 12, 2023

Do we touch the file or timestamps ourselves somewhere?

Okay, mystery solved ;).

lfs.touch(file, now, mtime) -- update book access time for sorting by last read date

TL;DR: Yeah, this makes sense, and both that and this PR are desirable.

@snelg
Copy link
Contributor Author

snelg commented Jul 12, 2023

And to be clear, I don't mind if the proper solution is to make it so the mtime updates each time a file is opened, thus making the "date modified" the only needed Date-sorting option.
My patch here was just the easiest code change I saw since it simply un-does a few line changes.

@hius07
Copy link
Member

hius07 commented Jul 12, 2023

I agree, it makes sense to have both of them.

@NiLuJe
Copy link
Member

NiLuJe commented Jul 12, 2023

(Rebased it to make GH happy)

@poire-z poire-z merged commit 73c4f09 into koreader:master Jul 12, 2023
3 checks passed
@poire-z poire-z added this to the 2023.07 milestone Jul 12, 2023
@snelg snelg deleted the sort-by-last-read branch July 14, 2023 16:32
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.

None yet

5 participants