Allow filtering the keybindings and recent repos menus more directly (simply by typing) - #5985
Merged
Conversation
Originally I thought we'd benefit from this change in this branch; turns out that we didn't after all, because we changed the approach, but it's a nice cleanup anyway, so we include it here.
Two places test for "a character the user typed" by hand, and a third one is about to be needed. Give the test a name. Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com>
When a key matches several bindings of the same view, the first one wins; when it matches several of the view's parent, the last one did. Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com>
Printable keys are withheld from keybindings while the user is typing in a field, so that they end up as text. Decide that from the field that has the focus rather than from the view a binding happens to be registered for: a field can be embedded in another view, and that view's keys must be withheld too, or its bindings would swallow the characters. That makes it worth honouring KeybindOnEdit, which has been documented but ignored ever since it was introduced. A field that sets it sees printable keys offered to the keybindings first, and still gets them if no binding handles them, which is what lets a view keep its keys until the field has something to type into. Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com>
A view can only be drawn with the focused frame and title colors while it is the current view, but a panel made of an outer view and an editable field embedded in it has to look focused as a whole, whichever of the two the keyboard is pointed at. Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com>
The check was a single set of view names, so it also let a click move between two different panels, e.g. from the prompt to the commit message. List the panels instead, and require both views to be in the same one. Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com>
A filter can come from somewhere other than the search prompt: a menu that filters as you type has its own input field, and needs to apply what is typed there without going through the prompt's state. Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com>
The footer is drawn on the bottom border of the list's view, which is not always a free row: a panel that puts something else below the list shares that border with it, and has to render the footer there instead. Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com>
Nothing shows or positions them yet. The row is two views because the input field has to start after the "Filter:" prompt, and a gocui view is a rectangle: the frame view draws the row and the prompt, the field sits inside it. Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com>
The row is reserved for as long as such a menu is open, even while it is still hidden, so that it can appear without moving the menu. That costs two rows of the popup, which is why the screen has to be a little taller before a menu is worth showing at all. The prompt in front of the input field is dropped when the row gets too narrow to type in, and the keybindings menu says what '@' does when it still fits. Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com>
The filter input is where the keyboard points for as long as such a menu is open, so that the first printable key can go straight into it. The menu still gets every key the input doesn't take, because the input view is embedded in the menu view, and the two are drawn as one focused panel. Which keys the input takes changes once there is a filter: until then printable keys still drive the menu, so that the configured navigation keys work as usual, and afterwards they are all filter text. A menu item's own keys are never bound in such a menu, because typing one has to reach the filter rather than execute the item. Escape gives up the filter and leaves the menu open; the next one closes it. The filter prompt behind '/' is gone from these menus: the row already does that job, and a second filter would only be confusing. Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com>
The keys for paging through a menu are ',' and '.' by default, and there is no non-printable alternative for them, so a menu that filters as you type would lose paging altogether as soon as the user typed anything. The same goes for confirming and cancelling if those keys are configured as printable ones. So bind the physical keys for all of it, on top of whatever is configured, and only where they aren't the configured keys anyway. Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com>
Its footer, the hint in the menu's subtitle, where the row sits in relation to the menu and the tooltip, and whether the text cursor is showing are all things the tests for it need to look at. Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com>
Looking up a keybinding is a search, so the menu that lists them is the one that most wants this. Its items do have keys, but only as a reminder of what they do outside the menu, so nothing is lost by not binding them. The prompt in front of the input field says what '@' does. It only ever showed up while the user was typing in the search prompt, so it could afford to be wordy; on a row that is on screen for as long as the menu is, it can't. Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com>
The only menu that ever asked for it was the keybindings menu, which now filters as you type and doesn't use the prompt at all. That leaves every filterable context with the same prompt, so the whole hook can go, and with it the two implementations that only existed to satisfy it. Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com>
Picking a repository out of that list is the other place where the menu is a list to search rather than a set of commands, and its items have no keys that typing could clash with. Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com>
There is no `reservedKeys` any more; the list of keys that menu items must not shadow is `essentialKeys` in the function that creates the menu. Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com>
… view This was implemented quite a while ago.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
All menus in lazygit can be filtered by pressing the
/key; for most menus which only show a handful of choices this is not really needed, but with the two cases where it's useful, it was unnecessarily inconvenient: you first have to press/to open the filter prompt, and then press enter to confirm the filter before you could press enter again to trigger the chosen item. It's much easier to simply type to filter, and still use the arrow keys to select one of the filtered items, or press enter to trigger it while the filter prompt is showing.The consequence of this is that while the keybindings menu is open you can no longer use the displayed key bindings to trigger the commands; I think that's fine, that menu is more for looking up those keybindings rather than for using them from within the menu.
Also: since
j/kare bound to move the list selection by default, it is not possible to filter for something that begins withj/k. I didn't want to change this because I'm concerned that die-hard vim users would perceive it as a regression if they can no longer typejto select the next menu item. The workaround is to type some other letter and backspace; this keeps the filter prompt open, so you can now typejork.