Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Del inside Trash doesn't work #777

Closed
ljgdasfhk opened this issue Sep 16, 2018 · 17 comments
Closed

Del inside Trash doesn't work #777

ljgdasfhk opened this issue Sep 16, 2018 · 17 comments
Labels

Comments

@ljgdasfhk
Copy link

https://www.youtube.com/watch?v=McV9Sffd8B0

@tsujan
Copy link
Member

tsujan commented Sep 16, 2018

Doesn't happen with the latest git. Which version do you have?

@tsujan tsujan closed this as completed Sep 16, 2018
@ljgdasfhk
Copy link
Author

ljgdasfhk commented Sep 17, 2018

Doesn't happen with the latest git.

Пока не проверял, но сильно сомневаюсь, что "не наблюдается".
Версия из состава Archlinux : pcmanfm-qt 0.13.0-1
При попытке удаления файла в корзину pcmanfm-qt нагружает процессор на 100 %.
Это видно на записи.

closed

это надо понимать как "у нас все работает" ?

@tsujan
Copy link
Member

tsujan commented Sep 17, 2018

The "report" was useless enough; and now a comment in Russian?!

@agaida
Copy link
Member

agaida commented Sep 17, 2018

I have not checked it yet, but I strongly doubt that it "is not observed."
The version from the Archlinux: pcmanfm-qt 0.13.0-1
When you try to delete a file in the recycle bin, pcmanfm-qt loads the processor 100%.
This can be seen on the record.

@tsujan - the reporter might be right - ok, its a bit sensless, but a bug in our trash handling. The important difference is that real users use our things. Easy to reproduce:

  • Open pcmanfm-qt
  • open trash
  • mark a file and hit <Del> - a delete dialog come up and try to move the file from trash to trash 🕶️ - last forever of course.
  • <Shift><Del> works fine as expected

PS: Sometimes i hate github flavoured markdown, damn morons - never write <DEL> without masking the opening bracket with a backslash

@tsujan tsujan changed the title can not move files to trash (archlinux) Del inside Trash doesn't work Sep 17, 2018
@tsujan
Copy link
Member

tsujan commented Sep 17, 2018

@agaida OK, this is very different: your comment is a clear and complete bug report in English -- of course, because you care.

Yes, I can reproduce that. It's an important bug.

@tsujan tsujan reopened this Sep 17, 2018
@tsujan tsujan added the bug label Sep 17, 2018
@agaida
Copy link
Member

agaida commented Sep 17, 2018

@ljgdasfhk - hi, given that i had to learn the russian language in school thirty years ago and never used since it was not enough to translate your report - so please give us a chance and use google or another translator. Thats what i did :P thanks.

tsujan added a commit that referenced this issue Sep 17, 2018
Fixes #777

Previously, `FileOperation::trashFiles` was misused.
@agaida agaida added this to High priority (release-blockers) in Issues Sep 19, 2018
@agaida
Copy link
Member

agaida commented Sep 20, 2018

@tsujan - just encountered without your current patch applied - same behaviour in remote filesystems. So disabling "Move to trash" for remote fs seems to be a good idea too.

@tsujan
Copy link
Member

tsujan commented Sep 20, 2018

@agaida Good point!

We'd better find a general method of knowing where it's impossible, instead of artificially disabling it here and there. I'll think about it.

So, please don't merge #778

@tsujan
Copy link
Member

tsujan commented Sep 21, 2018

Coming back to this:

@agaida Ignore my last comment! #778 can be merged as it is (of course, if it works -- as it does here). The problem you found with remote filesystems should be fixed in libfm-qt and its fix seems easy: a break is missing :D I'll make another PR for it.

@tsujan
Copy link
Member

tsujan commented Sep 21, 2018

Here, in addition to #778 : lxqt/libfm-qt#277

I think I've also found a problem in GIO but it isn't important: trashed folders are reported to be non-deletable, although they can be deleted of course. As a result, Delete doesn't work with trashed folders; only Shift+Delete works. A workaround is possible but wouldn't have much usage.

agaida pushed a commit that referenced this issue Sep 21, 2018
Fixes #777

Previously, `FileOperation::trashFiles` was misused.
Issues automation moved this from High priority (release-blockers) to Closed Sep 21, 2018
@tsujan
Copy link
Member

tsujan commented Sep 22, 2018

@agaida A dangerous question: Is your SMB OK now? I can't see any problem in other places, although I may have illusion.

@agaida
Copy link
Member

agaida commented Sep 22, 2018

Most things are o.k - a little but:

  • Context menu shows move to trash (impossible on smb://) instead only delete
  • Deletions work fine, but the current view in pcmanfm-qt isn't updated. (console shows, that now filemonitor can be created)

@tsujan
Copy link
Member

tsujan commented Sep 22, 2018

Context menu shows move to trash

But "Move to Trash" offers deletion after being used; am I right?

Removing "Move to Trash" in such cases seems impossible with the current code (although, in theory, nothing's impossible) because moving to trash should be tried first and only if unsuccessful, deletion should be offered.

but the current view in pcmanfm-qt isn't updated

That's OK because it's like search://: no file monitor can be created for SMB.

@agaida
Copy link
Member

agaida commented Sep 22, 2018

yes to both - is it possible to trigger an update of the view in such cases? pcmanfm does it.

@tsujan
Copy link
Member

tsujan commented Sep 22, 2018

is it possible to trigger an update of the view in such cases?

A file may be deleted/added from outside pcmanfm-qt and, because there's no file monitor, pcmanfm-qt wouldn't know about that in order to trigger a reload. Moreover, a reload can be expensive.

Does Nautilus or another GTK file manager update the view without reloading? I can't test because I don't have SMB.

@agaida
Copy link
Member

agaida commented Sep 22, 2018

pcmanfm does - will test nautilus and dolphin

@tsujan
Copy link
Member

tsujan commented Sep 22, 2018

pcmanfm does

That's more than enough -- no need to test Nautilus. It shows something is missing from libfm-qt. I'll look into it.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
Issues
  
Closed
Issues
  
Done
Development

No branches or pull requests

3 participants