Allow to move content files to trash instead of deleting them #20252
Conversation
glassez
added
WebUI
This comment was marked as outdated.
This comment was marked as outdated.
glassez
force-pushed
the
delete-torrent
branch
from
e4a4c39
to
7a9907a
Compare
src/base/bittorrent/sessionimpl.cpp
Outdated
| { | ||
| if (errorMessage.isEmpty()) | ||
| { | ||
| LogMsg(tr("Torrent removed. Torrent: \"%1\"").arg(torrentName)); |
There was a problem hiding this comment.
I think mentioning whether it was moved to trash or permanently deleted would be useful.
Also:
| LogMsg(tr("Torrent removed. Torrent: \"%1\"").arg(torrentName)); | |
| LogMsg(tr("Torrent and its contents have been removed. Torrent: \"%1\"").arg(torrentName)); |
or keep the previous Removed torrent and deleted its content
src/base/bittorrent/sessionimpl.cpp
Outdated
| } | ||
| else | ||
| { | ||
| LogMsg(tr("Torrent removing done with error(s). Torrent: \"%1\". Error: \"%2\"") |
There was a problem hiding this comment.
done
Is it impossible to fail? If not, something like (not sure if it's correct):
| LogMsg(tr("Torrent removing done with error(s). Torrent: \"%1\". Error: \"%2\"") | |
| LogMsg(tr("Torrent removal has encountered error(s). Torrent: \"%1\". Error: \"%2\"") |
p.s. Can it have multiple errors and if yes shouldn't this also be changed Error: \"%2\ -> Error(s): \"%2\?
There was a problem hiding this comment.
Is it impossible to fail?
Well, theoretically this is only possible if you fail to delete the resume data, so that the torrent will be downloaded again next time. But we've never considered this case previously, and I don't remember having any problems with it. At least I'm not going to change that in this PR.
There was a problem hiding this comment.
Torrent removal has encountered error(s).
Still, wouldn't it be better to emphasize that the errors do not relate to the removal of the torrent itself?
There was a problem hiding this comment.
Can it have multiple errors
The files are deleted one by one, and it tries to delete all of them despite the encountered failures, so we may have a number of errors equal to the number of files. But libtorrent provided only one of errors, so I followed its behavior. Usually, if there is a problem with deleting files, then it is the same for all of them, so a single error message is quite enough, I see no point in clogging the log with them.
There was a problem hiding this comment.
Still, wouldn't it be better to emphasize that the errors do not relate to the removal of the torrent itself?
Yeah probably, but i've got nothing.
There was a problem hiding this comment.
Changed it to display messages separately for torrent removal and content removal events:
There was a problem hiding this comment.
Changed it to display messages separately for torrent removal and content removal events:
Now it gives 2 log entries for one action...
Btw i just saw the share limits log messages:
Removed torrent.
Removed torrent and deleted its content.
Why not re-use them? If you insist to use different strings, you should probably update the share limit strings too.
There was a problem hiding this comment.
Changed it to display messages separately for torrent removal and content removal events:
Now it gives 2 log entries for one action...
As stated in my comment above, they are two related consecutive events, IMO. So if no one strongly object it I would prefer to handle them separately.
Btw i just saw the share limits log messages
Share limits log messages are really misleading. They claim that "torrent removed etc." before it's done. They must be replaced by something like "The torrent has reached the share limit, it will be removed along with its content."
There was a problem hiding this comment.
Share limits log messages are really misleading. They claim that "torrent removed etc." before it's done. They must be replaced by something like "The torrent has reached the share limit, it will be removed along with its content."
Done.
|
Don't know if you can do anything about it: In Linux, all files in trash which were deleted by qBt have IMO the usefulness of this feature is severely limited if torrent content folder structure isn't preserved in trash. |
glassez
changed the title
glassez
force-pushed
the
delete-torrent
branch
from
7a9907a
to
8c77506
Compare
Have you tried to restore these files from the trash in the usual way? |
No because it requires their containing folder to be present but naturally qBt had deleted it too and i was lazy to re-create it manually for testing. I did so now and they are restored without the suffix. |
glassez
force-pushed
the
delete-torrent
branch
from
4610820
to
2eaa1ff
Compare
|
When deleting, files can't always be deleted, but there is no file deletion error in the log. With permanently deleting I have not been able to reproduce this, the client always reports an error. 2024-01-27.20-51-33.mp4 |
Could you test it now?
Could you provide error message? |
The files remained in the folder, but there was still no error message. 2024-02-04.15-06-52.mp4
2024-02-04.15-13-03.mp4 |
|
@stalkerok |
|
Tested on https://github.com/qbittorrent/qBittorrent/actions/runs/7780124818. |
|
@stalkerok |
This comment was marked as resolved.
This comment was marked as resolved.
This comment was marked as resolved.
This comment was marked as resolved.
I tested this several times on Linux Mint and it didn't require that "their containing folder to be present". I just selected all the files in the trash and restored them and their folder was restored along with them. |
|
With "Move files to trash (if possible)" set. With "Delete files permanently" set. |
|
@stalkerok |
It looks strange. The fact is that the Windows function used returns a successful completion code, but the file has still not been moved to the trash. Since it uses not the WinAPI function, but Windows Shell functions, I suppose it could provide some kind of delayed execution "magic" when it can't be done immediately (as in your case). |
Huh, i guess it's a DE problem then (KDE for me). |
Does it behave the same way if you "manually" move some files from a folder to the trash, and then delete the folder itself and try to restore these files? |
Yep. |
I would call this a clear disadvantage of DE. Unfortunately we can't do anything about it. |
No, there is no antivirus on the system.
Actually, this doesn't always happen, sometimes files are deleted normally, but the ones that aren't deleted remain in the folder and are never removed from there unless manually deleted. |
But after all, when deleting in the usual way (using the built-in libtorrent method, as it was before this PR), it is always deleted for you, right? Or did you just not do enough repetitions of this test to encounter a deletion error? |
Yes, the files are always deleted.
I tested deleting in https://github.com/qbittorrent/qBittorrent/actions/runs/7780124818 a few more times, the files are always deleted from the folder. |
|
I noticed that this doesn't happen if I pause and then delete the torrents, then the files are deleted normally in both modes. |
I also had the idea to test this option. |
Some files still remain in the folder after deleting torrents. |
Well, that's sad. I'm still wondering why I couldn't reproduce it. |
Perhaps someone else can reproduce this too and confirm. I'll test this on a different version of the W10 OS later.
NVMe
This happens with any download speed.
No, default settings. |
|
@thalieht @xavier2k6 |
I used last CI from this PR for windows with libt 2.0.9. |
|
I tested with lt 1.2.19. |
Could someone test lt-1.2 vs lt-2.0 based builds under the same conditions? |
I have, same test as #20252 (comment) only in windows with build from CI. |
Preferences:
GUI:
WebUI:
Log:
