GitPlugin::remove: delete files, not trash them
This is the only usage of KIO::trash() in KDevelop's code base. I don't think it is appropriate here. The trashing occurs only if the file list contains a file not under version control or contains only directories, no files. Otherwise, `git rm -r --force` is used, which simply deletes files. So using KIO::trash() introduces a gratuitous inconsistency. I noticed this issue by examining my Trash. It contained lots of directories named "dir" and "emptydir", lots of files named "bar" and "testfile" removed from /tmp/kdevGit_testdir/. These files and directories are put to Trash each time test_kdevgit runs. 4b50220a introduced the first call to KIO::trash(). Later commits that touched this code simply followed suit or ported to a new API version. The author of the original commit Aleix Pol Gonzalez explained in the review: "the initial idea was that this was a way for never removing unrecoverable data. git rm will still have it in the git history, those that aren't part of it would go to the trash." The only use of the affected function GitPlugin::remove() (or rather the function it overloads IBasicVersionControl::remove()) outside of tests (as found by KDevelop's Show uses), is in KDevelop::removeUrl(). This single caller invokes KIO::del() if the URL to be removed is not under version control. The removeUrl() function itself is called safely enough: after a "Do you really want to delete these %1 items?" prompt OR in a move operation after successful copying OR in a rename operation after successful saving-as. So KDevelop deletes files unrecoverably already, but under certain conditions puts them to Trash instead. Best to get rid of this surprising inconsistency.
Please register or sign in to comment