ShellAnything icon indicating copy to clipboard operation
ShellAnything copied to clipboard

Disable selection.mimetype (and others) if too many files are selected.

Open end2endzone opened this issue 3 years ago • 4 comments

Describe the bug The properties selection.mimetype, selection.descrption and selection.charset should be disabled if too many files are selected. In a use case where a user right-click on his vacations picture collection with 10000+ files, File Explorer will appear Not responding.

Based on the code in Context::RegisterProperties(), ShellAnything will extract the mimetype, description and character set of all 10000+ files which will certainly take a ridiculous (and probably unnecessary) amount of time.

This is because, by design, all properties are updated even if they are not required/used in a Configuration.

This bug was introduced when implementing #92 and #94 .

To Reproduce Steps to reproduce the behavior:

  1. Select multiple files (like 10000+) in File Explorer
  2. Right-Click all files. File Explorer hangs for along period of time.

Expected behavior When more than X files are selected, it usually means that user is not interested in filtering by mimetype, description or character set. The appropriate value for X should be discussed below.

Environment

  • OS: N/A
  • Version 0.7.0

Additional context Add any other context about the problem here.

end2endzone avatar Sep 22 '21 17:09 end2endzone

I am think about 10, but opened to other suggestion.

end2endzone avatar Sep 22 '21 17:09 end2endzone

The user should be given options and let the user decide.

Before that, I also considered the issue of too many files are selected.

I have a few thoughts: One is to merge multiple selections into one when the attributes are the same, but this scheme is not practical, also not easy to implement. Another option is a filter. For example, I selected several files of *.exe and other format files, and I set to filter "exe", then selection.filename should only have *.exe after filtering. In order to achieve this function, a has() macro should also be implemented. But then I thought that it would be more appropriate for the filter to be implemented by a third-party program. The has macro seems to be useful, but it is little unnecessary.

In addition, although libmagic's IO overhead is small, it is not non-existent, especially for some low-speed storage media. Perhaps it is better to let the user decide whether to enable this feature, or it is better to implement this feature as a plug-in (in my imagination, plug-ins are loaded on demand)

Cirn09 avatar Oct 06 '21 15:10 Cirn09

First I would like to say thank you for trying to help. It is much appreciated to get feedback and new ideas from other people that uses this application.

I have looked at your proposed option. I do not understand the merge option. More specifically because I do not understand what you mean by "attributes". Are you referring to file extension? Like once the first ".jpg" file has gone through libmagic and we get the mime type, we should use the same mime type for all other .jpg files?

I am also afraid that your second option is currently impossible to implement with the current code base. The current implementation is refreshing properties (including ${selection.mimetype}) before visibility/validity filter are verified. It must be done in this order since the validation process often involves properties. This is a problem similar to "who comes first the egg or the chicken". If I understand correctly, what you are proposing is, at property refresh time, to "peek" into all menu's visibility/validity nodes and if all of them are filtering for "exe only", then we can eliminate evaluating the property for .png and .jpg files. I do not think this is the way to go.

Perhaps it is better to let the user decide whether to enable this feature, or [not].

I agree that user should have control and be able to decide. However, my point is the application should have limitations in place "by default" to prevent performance issue. This is to prevent situations where the popup menu takes too long to show. In other words, a user that is unaware of this feature/functionality would not experience slowdown because he does not know. On the opposite, a user that actually needs to evaluate many files will be able to control the app and for example, increase the "default" limit size.

I am not a fan of relying on plugins in an application for every aspect of a feature. It does provide maximum flexibility but it certainly increase the complexity and limits this ability to highly technical users.

I see 2 ways of limiting this risk:

  1. We can limit how many files this feature should evaluate before it stops peeking into files.
  2. We can limit how long this feature can take before it stops peeking into files.

I think the easiest way would be to define properties for such limitations. Properties can be modified in Configuration Files easily. They can be documented in the User Manual so that advanced users can modify their values. For example we could implement the following new properties: selection.mimetype.maxfiles and selection.mimetype.timeout.

From my experience, a 2 seconds delay before the menu shows up is way too long. I like my system to as responsive as possible. Also, my previous proposition of a maximum of 10 files is a bit ridiculous.

I think default values for each property should be something similar to these:

  1. selection.mimetype.maxfiles = 2000
  2. selection.mimetype.timeout = 5000 ms

Does that look like good alternative to you ? Do you have another better idea ? How to do you feel about those values ? Again, looking for your feedback.

end2endzone avatar Oct 23 '21 18:10 end2endzone

Any update on the proposed new variables to limit the "application is not responding" use cases ? What do you think of

  1. selection.mimetype.maxfiles = 2000
  2. selection.mimetype.timeout = 5000 ms

end2endzone avatar Nov 07 '21 15:11 end2endzone