Add a DeleteJob for the last file that was being copied/moved when pasting or dropping a file when the copyjob failed because of a user cancelation or because the destination disk was full.
Copy using paste action a file and cancel the action before it finishes, the destination file should be removed
Same operation using a drop "copy" action
Thanks for the patch! For formatting guidelines, please see https://community.kde.org/Infrastructure/Phabricator#Formatting_your_patch
In essence, put "BUG: 383764" on its own line in the Summary section, and remove it from the title.
Also please put "FIXED-IN: KDE Frameworks 5.44" on its own line, since that's the version of KDE Frameworks that this will make it into.
Works great in testing, and I'm excited to get this in. One comment about the code, indicated below, and I'll let the more experienced KDE developers offer a more in-depth code review.
I like to use the full string since not all of our users or people browsing the bug tracker will be familiar enough with all our products and their versioning conventions to able to map a version number to its product. Is there a technical reason why the field should only have the version number instead of the full string?
And they should not need to track the product: the version is per component.
So please fix this page https://community.kde.org/Guidelines_and_HOWTOs/Write_a_version_string (or I can do it) to describe the current status and ask for feedback if you want to change it without deciding for everyone. According the current usage, only the version number is used and needed, with the exception sometimes of applications part of KDE Applications, where some authors decide to use "x.y.z (KDE Applications some.thin.g)".
In general using the version number only does not require looking for the version of the product.
The idea kind of makes sense, but as is, it's very dangerous, what if the destination already existed?
Case 1: copy file:///dir1 to ftp://host/dir2 which already exists. During the 'stat src' or 'stat dest' phase, cancel the job -> it will now delete dir2 !
(see copyjob.cpp:879 which sets m_currentDestURL).
Case 2: *move* dir1 to dir2 which already exists, cancel before the move happens, boom.
I don't have time to think about more cases, but I suspect there are some more.
Unittesting "cancelling at this exact step of the copy" sounds very tricky, unfortunately... but maybe we can add unittest-specific env vars to abort with a disk full error at some points in the code.
Also, why is this done at the PasteJob level? there are many other ways to trigger a copy, including drag-n-drop. It sounds like this should rather be done inside CopyJob, which would also make it possible to have more information about whether it's safe to clean up or not.
@since 5.44 if it has to be public, but please also document what this really is, for people who haven't read the copyjob.cpp source code.
Since you don't test the result of the cast (I assume because by construction only a CopyJob is connected to this slot?), just use static_cast.
this-> isn't really useful, I'd remove it