search

noembryo / KoHighlights

KOHighlights is a utility for viewing KOReader's highlights and/or export them to simple text, html, csv or markdown files.
MIT License
128 stars 5 forks source link

removing some books info removes whole sdr folder #17

Closed richo67 closed 2 years ago

richo67 commented 2 years ago

Hi, today I discover when I select few lua files belonging to the same book (but not all) and press DEL, KoHighlight will remove whole sdr folder with all lua files. Quite shocking. Please take a look at he screen recording (no audio).

https://user-images.githubusercontent.com/9590148/136448142-7e22578d-0b53-471b-bd8d-9fffca9e60ec.mp4

noembryo commented 2 years ago

Well, this is a side effect of the bonus feature that was added after your suggestion.. :wink:

But anyway, what happens is not that shocking. After all, the program is warning you that the "selected books' information" will be deleted. The "selected books' information" is the .sdr folder.

I have to remove the folder, otherwise we end up with .sdr folders that are either empty, containing .old files or whatever else the devs are going to put there.

What looks confusing, is that I don't clear the other rows (of the same book) too. I'll put it in the todo list for the next release..

richo67 commented 2 years ago

Maybe you didn't noticed but I have not removed all .lua but only those additional. So the original one should have stay there together with the .old. And therefore the sdr folder should not be removed.

Also if I have more (5 for example) extra lua file and remove more than one in one DEL press ( 2 for example) whole folder is removed.

And yes the rows of the removed extra .lua files are not removed from UI.

noembryo commented 2 years ago

Maybe you didn't noticed but I have not removed all .lua but only those additional. So the original one should have stay there together with the .old. And therefore the sdr folder should not be removed.

The action that you perform when pressing the del button is the first of three Delete actions (you can see them if you right click and select Delete). That one is "Delete">"Selected books' info". It is not delete selected .lua files. So it doesn't matter how many .lua files you have selected. The book's info gets deleted. And the book's info is the .sdr folder.

richo67 commented 2 years ago

I see, but wouldn't have it much more sense that you would consider book info only the selected .lua file and delete only that file?

If you select the one which is correctly related to a book than you can remove whole folder, because as you correctly wrote, it has no sense to keep the folder if the metadata.*.lua file is not there. I think however if you select to delete not the primary lua file (all those strange file I have there ;-) ), then you should delete only that file.

What do you think?

noembryo commented 2 years ago

My problem with this is that it makes the code more complicated for an edge case scenario. Its much easier to right click > Open location and manually delete the files.

richo67 commented 2 years ago

OK, I do not see in your code so I can understand, but you already have pointer to that particular file once I select merge, isn't it not much more difficult then open and merge a file than only delte it?

noembryo commented 2 years ago

I can always access the file. The problem is that I have to add more checks in the "delete actions" that will complicate and slow down the code (you can delete a lot of files at once). My main objection is that this is an edge case and there is a comfortable workaround already..

richo67 commented 2 years ago

Yes I understand. I hate workarounds though, especially as user ;-)