huijunchen9260 / dmenufm

A simple file manager using dmenu
GNU General Public License v3.0
227 stars 15 forks source link

Add basic multi selection support #8

Closed camnw closed 4 years ago

camnw commented 4 years ago

Hey! Submitting this pull request so we can be on the same page for this feature and its implementation as we work on it. You may review this to see if this is the solution we want to work forward with to make this feature happen. I have made it work with the RMM action, you select the "Multi Select" option and then select files until you press "End Selection" and when you confirm it will delete all selected files. I am interested to know where else you think we should add multiple selection support for right now. Please let me know what you think / make changes where you see fit so we can hammer down this eature. Thanks!

huijunchen9260 commented 4 years ago

Thank you so much for providing this project such an elegant and simple way to implement multi-selection!

I hope that later on we can extract the code in FM_RM into a function called "multiselect", and we can apply it in every function that I think it would be good to have multi-select function

camnw commented 4 years ago

Currently, there is an issue where if you press escape inside of multi select it gets hung in the loop with no dmenu showing at all, I am not quite sure why this is occurring and cannot seem to get it to work no matter what I try. If you attempt to return it before the loop, it will work, however no more action commands will work and will go right back to the normal file browser. Maybe you can come up with some solution to this I am not seeing at this time.

huijunchen9260 commented 4 years ago

I believe that I have found the problem and I solved them.

There are two problems:

  1. If $allowmulti is empty, then when we press ESC, the $actCHOICE is also empty, and thus the if condition will choose to match with $allowmulti, and thus the $multiselection will always be true, the while loop never ends.
  2. Even if I set $allowmulti="NotAllowed", as long as I enter the multi-selection mode, the $multiselection is true. Thus, more than one loops will be activate. Solution is to also set $multiselection as empty when $actCHOICE is empty (the else part of the Generate_action_menu)

Thank you again for helping me implement this difficult function using such an elegant way! If you found out any bugs, please tell me again in this place.

Thank you!

huijunchen9260 commented 4 years ago

No I didn't :( Now the multi-select mode only select once :((((( I need to fix it again

camnw commented 4 years ago

Aha, thank you for solving this, was tired and couldn't figure it out. What other features do we want to include multi-select support for before we merge? Should we include the bulk rename feature with this pull or only certain others? Thanks.

huijunchen9260 commented 4 years ago

Thank you for giving the multi-select function more flexibility!

I plan to add multi-select into MV, CP, Rename, "Move to trash", and compress selected file.

This function gives me such a big inspiration in implementing actions! Thank you so much for contributing this project!

camnw commented 4 years ago

Marking this as ready for review as we have all of the existing parts we plan to support (not related to renaming) complete. Compressing files isn't added yet so we can't apply this to it with this pull and bulk rename is another thing we need to add support for before this can be used in it.

huijunchen9260 commented 4 years ago

Thanks! I am now working on MVR/CPP of multi-selection! I think this function is really useful!

huijunchen9260 commented 4 years ago

I have finished implementing the MVR/CPP multi-selection. However, there is one minor bugs.

When we assign the notifymessage, we haven't determined the $destname. Thus, the message will not correctly show the $destname.

Would you please do me a favor and try to solve this issue?

Thank you!

huijunchen9260 commented 4 years ago

https://github.com/huijunchen9260/dmenufm/commit/7fae4f4db7155b996ffb25c575c27e6ecac0b8f4