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

FR: don't delete the .sdr directory when deleting a book #8281

Closed
Kazurin-775 opened this issue Sep 30, 2021 · 19 comments · Fixed by #8348
Closed

FR: don't delete the .sdr directory when deleting a book #8281

Kazurin-775 opened this issue Sep 30, 2021 · 19 comments · Fixed by #8348
Milestone

Comments

@Kazurin-775
Copy link

I have a book with 2 different formats, one named KOReader.pdf and the other KOReader.mobi. These 2 formats share the same .sdr directory, so when I try to delete KOReader.pdf, the .sdr directory for KOReader.mobi just goes away as well, so I will lose all my bookmarks in KOReader.mobi.

There are 2 solutions I could think of:

  • Warn the user if a .sdr directory will be deleted, or even better, allow the user to keep the .sdr directory.
  • Include the file extension in the name of the .sdr directory, e.g. KOReader.pdf.sdr/. (However, this will become a breaking change.)

  • KOReader version: 2021.09
  • Device: Android (Xiaomi Mi Reader Pro)
@hius07
Copy link
Member

hius07 commented Sep 30, 2021

That makes sense.
Besides, Purge .sdr is not much clear to a user, both Delete and Purge buttons might be combined to something like

1

@Zeyadas
Copy link

Zeyadas commented Sep 30, 2021

The .sdr is really annoying me, I remember talking about it lately here on github but can't remember where, each file (book) I open using koreader is creating .sdr file so my original folder (my library) now is a combination of pdf files and sdr folders, I hope you can just collect all the .sdr files into One folder and it'll be better if it's in the (koreader) folder with koreader files.. Or at least it could be hidden.

@poire-z
Copy link
Contributor

poire-z commented Sep 30, 2021

Besides, Purge .sdr is not much clear to a user, both Delete and Purge buttons might be combined to something like

I'm not against this, it looks quite sensible (except for the granularity: is there a reason to delete the book and keep the sdr? Does not make real sense to me.)
But this would need many little discussions, like:

  • how to reorganize the buttons on the various long-press popups, if Purge. sdr and Delete become a single one
  • how to reorder these choice, because the most dangerous usually go to the left (Purge sdr should be more reachable than Delete).
  • (the fact that I often Purge.sdr and I like them accessible as 2 direct buttons - the reorganisation will need some new habits and may cause some unwanted quick actions out of habits)

We could also just replace "Purge .sdr" with "Delete settings" (but "Delete settings" sounds even more dangerous than "Delete book"), or find an even better name.

@Zeyadas
Copy link

Zeyadas commented Sep 30, 2021

The .sdr is really annoying me, I remember talking about it lately here on github but can't remember where, each file (book) I open using koreader is creating .sdr file so my original folder (my library) now is a combination of pdf files and sdr folders, I hope you can just collect all the .sdr files into One folder and it'll be better if it's in the (koreader) folder with koreader files.. Or at least it could be hidden.

#8155

@Frenzie
Copy link
Member

Frenzie commented Sep 30, 2021

@poire-z reset instead of delete?

@poire-z
Copy link
Contributor

poire-z commented Sep 30, 2021

Yes, sounds safer :)

@Zeyadas also #4831. It's not super complicated to have it work - the problem is that there are MANY little places where we assumed things were in a .sdr folder alongside the book, places that will need to be 1) looked for and 2) adapted, so everything in KOreader works just as well (like, showing the dogear hint that a book is opened in file browser).
So, a lot of work and risks, for a use case none of us main developpers really care about :/
That's possibly true too for the top post issue - easier to delete the directory than having to find out what's in it that should be kept or not... for a rare use case.

@Zeyadas
Copy link

Zeyadas commented Sep 30, 2021

Understood, thanks guys.

@Kazurin-775
Copy link
Author

Sorry, the issue's author here. Later I discovered that the metadata of KOReader.mobi is actually named KOReader.sdr/metadata.mobi.lua -- this means that the metadata is actually file-format-aware, and therefore, anything I do to KOReader.mobi should never influence the metadata of KOReader.pdf.

But as far as I'm concerned, this is not the case when it comes to deleting a book (or its metadata). When deleting a book's settings, the whole .sdr directory is deleted, instead of only .sdr/metadata.*.lua and .sdr/metadata.*.lua.old. This will indeed delete something wrongly in some cases (for example mine).

Therefore, aside from any improvements in the UI, I hope that this problem can be fixed as well -- always check for extra files before deleting a directory.

@poire-z
Copy link
Contributor

poire-z commented Oct 15, 2021

Hijacking this issue to add more links about off-topic #8281 (comment) discussed above:
Was having a quick look around about this thing about not using .sdr. Just linking some related issues and discussions:
#8155 #7090 #7648
#4831 (comment) (patch.lua solution for Android, if you really want it)
But the initial move from having them all in koreader/history/ to having them in book.sdr/metadata.epub.lua was done and discussed with pro & cons in: #1962

@poire-z
Copy link
Contributor

poire-z commented Oct 15, 2021

Btw, .sdr comes from Kindle, and KOReader was re-using it for storing its own stuff, kinda as a favor to Kindle users so they don't get multiple directories.
Is this still a thing on Kindles ? @hius07 ?
How are Kindle's own files named in there, if we were to preserve them when purging/deleting a book ?

I guess this issue requests we only remove the files this books' own.
For the rare case I have of 2 books with same filename but different suffix, I have:

metadata.epub.lua
metadata.epub.lua.old
metadata.epub.lua.old_dom20190310
metadata.html.lua
metadata.html.lua.old
metadata.html.lua.old_dom20180528

So, when removing the epub book, we should remove metadata.epub.lua*, which would preserve metadata.html.lua* and possibly Kindle's own stuff.

Actually, the Purge .sdr button in File browser and History is a bit more careful than DocSettings:purge() (but it forgets to remove migration backups like old_dom20180528):

-- Purge doc settings in sidecar directory,
function filemanagerutil.purgeSettings(file)
local file_abs_path = util.realpath(file)
if file_abs_path then
os.remove(DocSettings:getSidecarFile(file_abs_path))
-- Also remove backup, otherwise it will be used if we re-open this document
-- (it also allows for the sidecar folder to be empty and removed)
os.remove(DocSettings:getSidecarFile(file_abs_path)..".old")
-- If the sidecar folder is empty, os.remove() can delete it.
-- Otherwise, the following statement has no effect.
os.remove(DocSettings:getSidecarDir(file_abs_path))
end
end

I guess DocSettings:purge() should do it a bit like this, but checking for a bit more candidates than just .old.

@hius07
Copy link
Member

hius07 commented Oct 15, 2021

On Kindles all books must be put into the documents folder.
Inside it more subfolders can be created but the home screen shows books mixed all together, not separated by subfolders.
For every book sdr-folder is created near it, it stores dfferent metadata and service files (azw3f, azw3r, pds, pdt, yjf etc).
When deleting a book, sdr folder remains intact, so thousands of them can exist.
There are special programs for cleaning orphan sdr https://www.mobileread.com/forums/showpost.php?p=2493735&postcount=14

@poire-z
Copy link
Contributor

poire-z commented Oct 15, 2021

So, our current behaviour of deleting the whole .sdr folder when deleting its book is:

  • a bug, and a totally unpolite thing to do to delete things that do not belong to us :)
  • or a feature and a practical thing that at least, KOReader cleans all thing behing it when a Kindle owner use it to delete a book ?

I'd rather fix koreader to delete only the things that he has written and are related to this book - but may be Kindle users will complain it does not really clean up anymore ?

(Checking if a thing belongs to this EPUB book, but does belong to another MOBI book, but also wanting to delete KINDLE stuff as a favour to tidy Kindle users, might be complicated to do - we don't want to have to maintaint a list of suffix/files that Kindle software may create.)

@hius07
Copy link
Member

hius07 commented Oct 15, 2021

I do not think Kindle users expect (or use) KOReader to tidy their documents folder.
It is strongly recommended not to store non-Kindle-format files in the document folder (Kindle tries to index them, fails, tries again and drains the battery).
But pdf is a common format, and more delicate deleting sdr folders might be desirable.

@hius07
Copy link
Member

hius07 commented Oct 15, 2021

Something like: delete two metadata files (current and old) attached to the book, then check if sdr folder is empty, and if yes, delete it.
Maybe also allow a user to keep sdr folder if he wants (for notes exporting purpose or something else).

@poire-z
Copy link
Contributor

poire-z commented Oct 15, 2021

Something like: delete two metadata files (current and old) attached to the book, then check if sdr folder is empty, and if yes, delete it.

Yes, that's what I had in mind, how things should be done.

Maybe also allow a user to keep sdr folder if he wants (for notes exporting purpose or something else).

That's an option you allowed in your proposal above #8281 (comment). But it's something I have never seen requested. So, if there are any, users wanting to keep their highlights and deleting the book must already have some kind of workflow to save it before deleting it.

@hius07
Copy link
Member

hius07 commented Oct 15, 2021

But it's something I have never seen requested.

As I wrote, Kindle does so.
I guess it can be useful when storing the library in the cloud, downloading to the device only a few books for reading, then deleting a book. If in a future this book is downloaded again, its notes are intact. (I haven't seen the explanation by Amazon why to keep sdr).
Is such a use case applicable to the calibre libraries?

@poire-z
Copy link
Contributor

poire-z commented Oct 20, 2021

Went without the 3rd option with #8348, mostly because I don't want to think/discuss the 2 points (#8281 (comment)) it would trigger.
But the day we need to add a button in there, moving Delete and Reset settings into a single button + some follow up dialog can then be a solution (and then, we can add the 3rd option).

@ghost
Copy link

ghost commented Oct 2, 2023

Currently after opening pdf using this app, it creates folder in the same path as of pdf. When i navigate through file manager, its annoying to see folders.

Can there be any way that the app keeps all these folders in fixed dedicated path directory rather than on the same folders as of pdf?
May be placing these directory inside koreader in home dir

@poire-z
Copy link
Contributor

poire-z commented Oct 2, 2023

Can there be any way that the app keeps all these folders in fixed dedicated path directory rather than on the same folders as of pdf?
May be placing these directory inside koreader in home dir

Good ideas ! Some people got the same and did all that some months ago :) #10074, available since v2023.03.

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

Successfully merging a pull request may close this issue.

5 participants