stefanhaller
commented
10 months ago Thanks for the detailed write-up, the thoroughness of this is impressive!
I propose that in any list view, you can press 'v' to start selecting a range of items, such that when you move your cursor, the items between the cursor and the start item are highlighted and are part of the selection.
Not being a vim person I don't like the 'v' binding in the staging view. The main thing I dislike about it is its modal-ness (modality?) or stickiness (I feel that vim people like modes more than other people).
For me, the most natural interaction to select a contiguous range of things is shift-arrow; this is very common pattern (e.g. in Finder windows in list view). It's less sticky, i.e. pressing arrow without shift collapses the range. In my fork I have a very hacky commit that implements this for the staging view; I've been using it for months and wouldn't want to miss it any more. I'll see if I can clean it up enough to contribute it as a PR.
We'll probably have to keep the 'v' selection too, for people who are used to it.
Each action that supports multi-select decides whether the selection is cleared by the action. Most actions should clear the selection,
I'm torn about this. I see how this is useful with the more sticky 'v' type of selection, but the shift-arrow selection is so easy to collapse that I don't feel it's necessary to auto-collapse it after actions. I think it would feel more predictable if it doesn't.
The selection is also cleared if you leave the current view (unless you're going to a popup)
Not sure about this one either; I suspect it could be annoying at times to lose the selection just because you change panels, but I can't give examples for concrete scenarios. Just a feeling.
Contiguous items only?
I agree that it makes sense to limit ourselves to contiguous ranges for simplicity. However:
- Code complexity: Moving a contiguous commits up/down is straightforward to implement, but what does it mean to move a non-contiguous set of commits up/down?
This wouldn't be a reason for me; at least for the case of moving the items by one step, it seems very straight-forward to me. Just move them one by one. It would be more complex if we allowed moving things across a longer distance, which is also something that I find desirable; but this needs a different UX anyway (similar to copy/paste commits, maybe) because you need to be able to move to the destination to invoke the action of moving the selected items there.
Multi-select in file trees
What does it mean when your selected range includes both files and directories, and you hit the space key to stage them? For a given directory in the range, should you stage all of it's files, or only the ones that happen to be included in your selection? Or should you just ignore directories and only deal with files? This doesn't matter so much staging because it's low-stakes, but it matters a lot more with discarding changes (if we want to support that).
I find it often helpful to see how other software deals with this. Apple is often good at UX design (not always mind you, but often), so take a Finder window in list view, unfold a directory, multiselect a directory and some of the files in it, and press Command-Backspace to move to the trash, this seems like the closest analogy to discarding changes.
The way I'd expect this to work is that for each selected directory the action applies to the whole directory. Whether any individual files inside the directory are selected or not doesn't make a difference then.
Implementation
- I'm not sure where to store the selection range state i.e. globally or on the context. I'm thinking on the context at the moment
Shouldn't it be stored in ListCursor?
- Should we use the DisabledReason field on our keybindings to disable all actions that don't support multi-select when we're in multi-select mode? And if so, how do we do that without a tonne of boilerplate? An opt-in approach sounds cleaner.
Yes, I'd like to use DisabledReason for that. Right now, most of our list controllers have a checkSelected helper for keybindings that require an item to be selected; they do nothing if no item is selected (likely because the list is empty). local_commits_controller is the only one yet that has been changed to use DisabledReason for checking for selected items; I'm currently working on doing the same for branches_controller, and I think we should do it for all list controllers. Once we have that, almost all keybindings will have a DisabledReason callback (many of them wrapped in an abstraction such as callGetDisabledReasonFuncWithSelectedCommit), and it should then be easier to bulk-change them to do multi-selection checks (probably opt-in, since that's more robust, and the signature of those handlers needs to change anyway).
- I'm interested in exploring having completely separate handlers for multi-select vs single-select actions, where we define which is used at the keybinding level. This will likely spare us some inline conditional logic in the handlers, and it'll let us set different labels/tooltips depending on what mode we're in (e.g. 'fixup commit' vs 'fixup commits'). But I'm not fully sold on the idea.
Interesting idea; I'm undecided, it probably needs to be tried to see if it's useful.
- Should we name this feature 'multi-select' or 'selection-ranges' or something else?
Range selection?
jesseduffield
Problem
Often you want to perform an action on multiple items in a list view, and the only way to do that is to navigate to each item in sequence and perform the action for each.
Example 1: Squashing a range of commits
A common example for me is having a bunch of WIP commits that I want to squash (fixup) into one. Currently the fastest way to do this is:
(You can also do this without an explicit interactive rebase but I find it slower due to each action requiring its own rebase)
It would be much better if you could just select the range of commits first, and then perform the action i.e.:
I'm using the 'v' key for starting the range because that's what we're already doing in the staging view when selecting a range of lines, and I'm pretty sure we chose that key because it's a vim thing.
Example 2: Staging a range of files
This is especially painful on windows which has an extra penalty per git invocation. Same as the example with squashing commits, if you want to stage multiple files you need to press space, down, space, down, etc. Much easier to start a range with 'v', move the cursor to the last file you want to stage, and press space.
Example 3: Moving multiple commits
This is something that I do not need often, but I coincidentally needed it yesterday. With the other examples, selecting a range spares you from having to visit each item and perform an action on it, but in this case it's even more laborious without a multi-select feature. I had a heap of WIP commits with a couple of specific commits spread-out which I wanted to bring down to the base of my feature and squash down. I'd have liked to move down the top commit until it collided with the next one and then move the two in tandem to the bottom, but I instead had to go one-by-one.
Proposal
I propose that in any list view, you can press 'v' to start selecting a range of items, such that when you move your cursor, the items between the cursor and the start item are highlighted and are part of the selection. All that needs to be stored in state is the index of the start item.
Then, if you perform an action that supports multi-select, the action applies to the range. If you attempt to perform an action that does not support multi-select (e.g. 'create a new branch' is meaningless in the context of multi-select) you'll see an error. Each action will need to specify whether or not it supports multi select.
Each action that supports multi-select decides whether the selection is cleared by the action. Most actions should clear the selection, but there are exceptions like when moving multiple commits (it would be annoying to have to re-select the range of commits each time you move them down by one).
If you've got a selected range, pressing escape will clear the selection. The escape key is used for various things but clearing a selection should be the highest-priority before anything else. We should also do this for selecting a range of lines which currently only cancels the range if you press 'v' again. In all cases, pressing 'v' should clear the selection if one already exists.
The selection is also cleared if you leave the current view (unless you're going to a popup)
Contiguous items only?
A crucial part of this proposal is that multi-select will only be for contiguous items, so if you have items A,B,C,D,E in a list view, you can't select A,B,C, and E and then perform an action. You'll instead need to perform the action for A,B,C in one go and then for E separately. I'm open to changing the design to support non-contiguous multi-select but the reasons I want to stick with contiguous are:
The main benefit of non-contiguous selection that comes to mind is being able to select files for staging, in a situation where it's expensive to do so. But I don't know if it justifies the complexity. There are already situations where non-contiguous multi-select is basically built in e.g. including files in a custom patch and copying commits to cherry-pick.
Multi-select in file trees
What does it mean when your selected range includes both files and directories, and you hit the space key to stage them? For a given directory in the range, should you stage all of it's files, or only the ones that happen to be included in your selection? Or should you just ignore directories and only deal with files? This doesn't matter so much staging because it's low-stakes, but it matters a lot more with discarding changes (if we want to support that).
Clear confirmations
When a range is selected, and a confirmation popup appears for some action, we'll need to be clear about which items are included in the range so that there's no surprises for the user. This is especially important for destructive actions.
Cherry-picking
The 'v' key is currently used in the commits view to paste copied commits (i.e. cherry-pick). It's nice having 'c' and 'v' for that because they correspond to copy/paste on our computers. Nonetheless, if we can't find a better key than 'v' for starting the multi-select, I propose we change the commit paste keybinding to ctrl+v. Obviously if we do this, we can't upgrade 'c' to ctrl+c because everybody expects ctrl+c to quit the application.
Funnily, it's always bugged me that we have two different ways of doing multi-select in the app now: it's the 'v' approach for selecting a range of lines to stage, but with cherry-picking in order to copy a range of commits you press shift+c to copy from the last copied commit to the currently selected commit. I much prefer the former approach so we can scrap the shift+c keybinding in favour of pressing 'v' to select the range and then 'c' to copy the commits.
Implementation
I haven't got the implementation worked out but some things that come to mind:
More use cases
I've gone through every action in the keybindings cheatsheet and worked out every place where we could use multi-select. I've categorised them into use cases I would actually use vs use cases of more dubious value.
Use cases I can see myself using
Use cases that are possible but less needed
Destructive actions lend themselves well to multi-select (I've included a couple above that I'd use). But for the following use cases, do we really need them?
Other actions:
Use cases that exploit multi-select for the sake of specifying two terminal items
If more use cases along these lines come up we can consider them, but at this stage I think we should hold off on multi-select-for-terminals for now just because it doesn't bring much advantage over existing approaches.
Open questions
Conclusion
What do people think? Keen to get your feedback.