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

Fixed an issue where there was a delay when deleting items #2738

Merged
merged 3 commits into from Dec 29, 2020

Conversation

Don-Vito
Copy link
Contributor

@Don-Vito Don-Vito commented Dec 28, 2020

Fixes #2672
Fixes #2732

There is a significant lag on deletion, especially when numerous files are deleted.
This is significantly impacted by checking whether the file is in Recycle Bin.

This PR somewhat helps by:

  • Delaying the check until the dialog presentation
    (resolves the issue for those who disabled the dialog)
  • Instead of searching in recycle bin just use path to decide
    if the item is under recycle bin

@gave92
Copy link
Member

gave92 commented Dec 28, 2020

Thanks a lot for this. I'm not sure why we check if we are deleting a file from recycle bin that way when we could just check if the file path starts with "X:\$Recycle.Bin"..

@Don-Vito
Copy link
Contributor Author

@gave92 - agree. Do you want me to implement this?

@gave92
Copy link
Member

gave92 commented Dec 28, 2020

Yes please! (Obvious note: the file path of a recycle bin element can start with any drive letter, not just "C:\")

@gave92
Copy link
Member

gave92 commented Dec 28, 2020

The same happens in FilesystemOperations.cs -> you can maybe just change the implementation of IsRecycleBinItem

@Don-Vito
Copy link
Contributor Author

IsRecycleBinItem

I prefer to introduce a new helper: as probably we still want to have a function that actively checks if the file is in recycle bin

@Don-Vito Don-Vito marked this pull request as draft December 28, 2020 10:19
@gave92
Copy link
Member

gave92 commented Dec 28, 2020

Depending on the scope of this PR you might also optimize the call to List<ShellFileItem> items = await recycleBinHelpers.EnumerateRecycleBin(); in FilesystemOperations so it's only called once when deleting many items.

@Don-Vito
Copy link
Contributor Author

@gave92 - finally had a lunch time to squeeze a bit of coding 😄
I am not sure what is the best way to approach the List<ShellFileItem> items = await recycleBinHelpers.EnumerateRecycleBin(); as it seems that we really need shell file items there.
Probably this could be postponed.

@gave92
Copy link
Member

gave92 commented Dec 28, 2020

@Don-Vito yup I'm didn't mean to remove it, I meant that when you delete a bunch of file that thing is called for every file, while it would be more efficient to call it once at the end.
It's not necessary to do this change here though

@Don-Vito
Copy link
Contributor Author

@Don-Vito yup I'm didn't mean to remove it, I meant that when you delete a bunch of file that thing is called for every file, while it would be more efficient to call it once at the end.
It's not necessary to do this change here though

I can followup on this 😊 I will try to create an issue later on.

@Don-Vito Don-Vito marked this pull request as ready for review December 28, 2020 13:47
@gave92
Copy link
Member

gave92 commented Dec 28, 2020

@Don-Vito for reference: Files Code-Style

@Don-Vito
Copy link
Contributor Author

@d2dyno1, @gave92 - thanks.. and sorry - didn't touch C# fort the last 8 years :)

@Don-Vito
Copy link
Contributor Author

@gave92 - anything I can do to help pushing it forward? 👼

@gave92
Copy link
Member

gave92 commented Dec 28, 2020

@Don-Vito this change looks good to me, I'll approve it as soon as I can test it 👍

@gave92 gave92 self-requested a review December 28, 2020 20:37
@yaira2 yaira2 changed the title Delay and optimize recycle bin check upon deletion Fixed an issue where there was a delay when deleting items Dec 29, 2020
@yaira2 yaira2 merged commit 518510c into files-community:master Dec 29, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
4 participants