Files fed to listbox can be deleted
AbandonedPublic

Authored by asensi on Jun 12 2018, 5:17 PM.

Details

Reviewers
abika
Group Reviewers
Krusader
Summary

As it's stated in https://bugs.kde.org/show_bug.cgi?id=394896: Files fed to listbox cannot be deleted; so the present proposal removes a "files()->isRoot()" condition, and that change allows that the aforementioned files can be deleted, though more changes may be necessary. As Krusader's virtual filesystem code is being talked about, the improvements of Krusader's virtual filesystem experts, search experts, etc. are welcome :-)

FIXED: [ 394896 ] Files fed to listbox cannot be deleted
BUG: 394896

Test Plan

I searched for several files and folders, pressed "Feed to listbox" and deleted them from the "Search results"; the physical files remained.

Diff Detail

Repository
R167 Krusader
Lint
Lint Skipped
Unit
Unit Tests Skipped
asensi requested review of this revision.Jun 12 2018, 5:17 PM
asensi created this revision.

the physical files remained.

All works as explained by asensi on my machine.

I might be wrong but I thought the idea was to remove physical files as well.

It can be read from code that confirmDeletion() in panelfunc.cpp allows physical removal, but on the other hand, removeVirtualFiles() forbids it.

Removal of items from virtual lists is not a very handful feature after all...

Can somebody comment on this? Thanks in advance.

nmel added a subscriber: nmel.Jun 13 2018, 7:48 AM

I might be wrong but I thought the idea was to remove physical files as well.
Removal of items from virtual lists is not a very handful feature after all...
Can somebody comment on this? Thanks in advance.

I second. The question is why the design decision to not remove physical files has been made before... (check out the message "Do you really want to delete this virtual item (physical files stay untouched)?")

> Removal of items from virtual lists is not a very handful feature after all...

I second.

I third :-). I agree with Nikita and Yuri.

Also, using Krusader 2.4.0-beta3, when "files fed to list box" are deleted... they are physically deleted.

abika added a subscriber: abika.Jun 13 2018, 7:26 PM

Explanation, as this is my code:
The files()->isRoot() condition is intended. Virtual deletion is only possible when the current directory is the root of vfs:// containing virtual directories. Deletion (== physical deletion OR moveToTrash) should be possible inside the virtual directories the normal way with the Delete key, F11, etc.. This has always been like that (or as far as i know Krusader), I only refactored the code.

If you want to introduce virtual deletion inside virtual directories, I recommend to create a new shortcut for this, to keep the two other deletion modes still possible. So just another way to invoke removeVirtualFiles() is needed now, that's why I extracted the code into this method:)

Bug 394896 is about not being able to delete (again: == physical deletion OR moveToTrash) files inside virtual directories. This should work and did some time ago. I'll investigate the next time I have time for Krusader if nobody else will fill in until then...

abika requested changes to this revision.Jun 13 2018, 7:27 PM
This revision now requires changes to proceed.Jun 13 2018, 7:27 PM
nmel added a comment.Jun 14 2018, 3:37 AM

Deletion (== physical deletion OR moveToTrash) should be possible inside the virtual directories the normal way with the Delete key, F11, etc.. This has always been like that (or as far as i know Krusader), I only refactored the code.

The point is, it doesn't work. I just checked this by myself. It asks if you want to physically delete and if UI Delete button is pressed, gives "Could not delete file ."

asensi added a comment.EditedJun 14 2018, 11:08 AM

I confirm that what Nikita tried doesn't work. Thanks, Nikita!

I'll investigate the next time I have time for Krusader if nobody else will fill in until then...

Excellent! Thanks again for all of your work, Alex!

asensi abandoned this revision.Jun 17 2018, 1:44 PM

Nice! There's more information in https://phabricator.kde.org/D13572