junegunn
Add keybindings for CTRL, ALT, SHIFT + UP, DOWN, RIGHT, LEFT, HOME, END, BACKSPACE, DELETE & more
This PR implements some missing key bindings.
In particular all possible combinations of
CTRLALTSHIFTALT+SHIFTCTRL+ALTCTRL+SHIFTCTRL+ALT+SHIFT
in conjunction with Up, Down, Left, Right, Home, End, Backspace, Delete, PageUp, PageDown.
I added tests to the LightRenderer beforehand to make sure I wasn't breaking anything, and expanded them when I added new key bindings.
So you can actually do stuff like:
fzf
--bind 'ctrl-up:up+up+up+up+up'
--bind 'ctrl-down:down+down+down+down+down'
--bind 'ctrl-right:forward-word'
--bind 'ctrl-left:backward-word'
--bind 'alt-delete:delete-char'
--bind 'alt-backspace:backward-kill-word'
This PR fixes #1747.
Thanks, I'll review the code when I get some time.
A few things.
- We also have to update tcell renderer for Windows
- man page needs to be updated accordingly
- Terminal emulators I use (kitty, iterm2, etc) don't distinguish between the
CTRL-*chords you added from the ones withoutCTRL. Which terminal emulator do you use on what platform? If these chords are not widely supported by the terminal emulators, we should warn about that on the manual. - Allowing all possible names (
ctrl-alt-shift,alt-shift-ctrl,shift-ctrl-alt, etc) will make the code harder to maintain. I'd just allow one sequence. i.e.ctrl-alt-shift.
No worries, take your time.
We also have to update tcell renderer for Windows
Yep, I checked tcell.go and most of it should be pretty straight forward to implement. My biggest issue is that I do not have any Windows machine to test on. So I can do an implementation, make sure it compiles but that's probably it. Would you mind to assist here and test it? Or is there a way to do this on Linux?
man page needs to be updated accordingly
True. Do you write the man page by hand? I was hoping it might be generated by some docstring, although I didn't find one when I searched for it.
Terminal emulators I use (kitty, iterm2, etc) don't distinguish between the CTRL-* chords you added from the ones without CTRL. Which terminal emulator do you use on what platform? If these chords are not widely supported by the terminal emulators, we should warn about that on the manual.
SGTM. I use gnome-terminal and ddterm both running most of the time zsh + tmux, sometimes just zsh or bash. Platform is Linux.
Allowing all possible names (ctrl-alt-shift, alt-shift-ctrl, shift-ctrl-alt, etc) will make the code harder to maintain. I'd just allow one sequence. i.e. ctrl-alt-shift.
I tried to stick as close to your implementation as possible and saw this case "alt-shift-right", "shift-alt-right": case where you actually support both combinations. But I don't mind removing the redundant ones. Actually I like that choice :smile:
I made the mentioned changes and the remaining exciting question is if the following block (tcell.go:~390) is correct:
case tcell.KeyBackspace2:
if ctrlAlt {
return Event{CtrlAltBackspace, 0, nil}
}
if ctrl {
return Event{CtrlBackspace, 0, nil}
}
if alt {
return Event{AltBackspace, 0, nil}
}
return Event{Backspace, 0, nil}
My educated guess is that it is actually not and has to be covered in the following block:
case tcell.KeyCtrlH:
switch ev.Rune() {
case 0:
if ctrl {
return Event{Backspace, 0, nil}
}
case rune(tcell.KeyCtrlH):
switch {
case ctrl:
return keyfn('h')
case alt:
return Event{AltBackspace, 0, nil}
case none, shift:
return Event{Backspace, 0, nil}
}
}
Or is there a way to do this on Linux?
You can run go run -tags tcell main.go.
Thanks for the hint!
But I am trying to get my hands on a windows machine as well because it all depends on what the OS is really gonna send. I will report back after that.
Nope, not yet.
The Linux implementation is done. Windows implementation potentially as well, but requires a patch in tcell to make the following shortcuts work:
Ctrl-Alt-*Ctrl-Alt-Shift-*
That's why this PR currently points to my own fork of tcell and that keeps this from being reviewed and merged. I created a PR upstream and hope for a quick review.
You can build this PR yourself. That should work.
Does this need to wait for full tcell support of all modifiers in windows, or could it just be annotated (like kitty, iterm2 are) that it's not supported yet? (context: I was hoping to be able to use shift-pageup/down for the preview panel).
Would you be fine splitting off the part depending on tcell changes into another PR @masmu ?
Seems there are other terminals beyond rxvt sending ^[[1~ for HOME – such as the linux VT & my debian machine's tmux 😅
[EDIT]: strangely, \e[1~ seems to be bound in the source for several years already, so I'm somewhat confused why it does not work here 🤔 [EDIT2]: ah, it does work, just that it's bound to go to line start/end, not list start/end.. :man_facepalming:
I would not mind not adding windows support at all. But that probably would demolish the chances of getting this merged, right @junegunn?
I am gonna try to get things rolling again in tcell/pull/749.
It's okay as long as we document the limitation. I assume you're a Linux user, so you might not be able to answer this, but I'm wondering how I can test this on macOS.
I assume you're a Linux user
Yes.
so you might not be able to answer this, but I'm wondering how I can test this on macOS.
Do you mean how to try out the keybindings? Build the PR, add some bindings a la
./target/fzf-linux_amd64
--bind 'ctrl-up:up+up+up+up+up'
--bind 'ctrl-down:down+down+down+down+down'
--bind 'ctrl-right:forward-word'
--bind 'ctrl-left:backward-word'
--bind 'alt-delete:delete-char'
--bind 'alt-backspace:backward-kill-word'
and see if they work.
Note that macOS might intercept certain keystrokes (like Windows does for Ctrl + Alt + Delete).
Besides that: Your ⌥ or ⌘ key (that depends on your terminal) is being treated as Alt.
Good news. tcell 2.9.0 fixed the windows issue with the broken shortcuts for Ctrl-Alt-* and Ctrl-Alt-Shift-*.
Since we are now also having full windows support, I added the remaining tests and updated tcell to 2.9.0.
This can be tested via the following "one-liner":
./target/fzf-linux_amd64 \
--bind 'up:change-prompt(up)' \
--bind 'down:change-prompt(down)' \
--bind 'right:change-prompt(right)' \
--bind 'left:change-prompt(left)' \
--bind 'home:change-prompt(home)' \
--bind 'end:change-prompt(end)' \
--bind 'delete:change-prompt(delete)' \
--bind 'page-up:change-prompt(pageup)' \
--bind 'page-down:change-prompt(pagedown)' \
\
--bind 'ctrl-up:change-prompt(ctrl-up)' \
--bind 'ctrl-down:change-prompt(ctrl-down)' \
--bind 'ctrl-right:change-prompt(ctrl-right)' \
--bind 'ctrl-left:change-prompt(ctrl-left)' \
--bind 'ctrl-home:change-prompt(ctrl-home)' \
--bind 'ctrl-end:change-prompt(ctrl-end)' \
--bind 'ctrl-delete:change-prompt(ctrl-delete)' \
--bind 'ctrl-page-up:change-prompt(ctrl-pageup)' \
--bind 'ctrl-page-down:change-prompt(ctrl-pagedown)' \
\
--bind 'shift-up:change-prompt(shift-up)' \
--bind 'shift-down:change-prompt(shift-down)' \
--bind 'shift-right:change-prompt(shift-right)' \
--bind 'shift-left:change-prompt(shift-left)' \
--bind 'shift-home:change-prompt(shift-home)' \
--bind 'shift-end:change-prompt(shift-end)' \
--bind 'shift-delete:change-prompt(shift-delete)' \
--bind 'shift-page-up:change-prompt(shift-pageup)' \
--bind 'shift-page-down:change-prompt(shift-pagedown)' \
\
--bind 'alt-up:change-prompt(alt-up)' \
--bind 'alt-down:change-prompt(alt-down)' \
--bind 'alt-right:change-prompt(alt-right)' \
--bind 'alt-left:change-prompt(alt-left)' \
--bind 'alt-home:change-prompt(alt-home)' \
--bind 'alt-end:change-prompt(alt-end)' \
--bind 'alt-delete:change-prompt(alt-delete)' \
--bind 'alt-page-up:change-prompt(alt-pageup)' \
--bind 'alt-page-down:change-prompt(alt-pagedown)' \
\
--bind 'ctrl-shift-up:change-prompt(ctrl-shift-up)' \
--bind 'ctrl-shift-down:change-prompt(ctrl-shift-down)' \
--bind 'ctrl-shift-right:change-prompt(ctrl-shift-right)' \
--bind 'ctrl-shift-left:change-prompt(ctrl-shift-left)' \
--bind 'ctrl-shift-home:change-prompt(ctrl-shift-home)' \
--bind 'ctrl-shift-end:change-prompt(ctrl-shift-end)' \
--bind 'ctrl-shift-delete:change-prompt(ctrl-shift-delete)' \
--bind 'ctrl-shift-page-up:change-prompt(ctrl-shift-pageup)' \
--bind 'ctrl-shift-page-down:change-prompt(ctrl-shift-pagedown)' \
\
--bind 'ctrl-alt-up:change-prompt(ctrl-alt-up)' \
--bind 'ctrl-alt-down:change-prompt(ctrl-alt-down)' \
--bind 'ctrl-alt-right:change-prompt(ctrl-alt-right)' \
--bind 'ctrl-alt-left:change-prompt(ctrl-alt-left)' \
--bind 'ctrl-alt-home:change-prompt(ctrl-alt-home)' \
--bind 'ctrl-alt-end:change-prompt(ctrl-alt-end)' \
--bind 'ctrl-alt-delete:change-prompt(ctrl-alt-delete)' \
--bind 'ctrl-alt-page-up:change-prompt(ctrl-alt-pageup)' \
--bind 'ctrl-alt-page-down:change-prompt(ctrl-alt-pagedown)' \
\
--bind 'shift-alt-up:change-prompt(shift-alt-up)' \
--bind 'shift-alt-down:change-prompt(shift-alt-down)' \
--bind 'shift-alt-right:change-prompt(shift-alt-right)' \
--bind 'shift-alt-left:change-prompt(shift-alt-left)' \
--bind 'shift-alt-home:change-prompt(shift-alt-home)' \
--bind 'shift-alt-end:change-prompt(shift-alt-end)' \
--bind 'shift-alt-delete:change-prompt(shift-alt-delete)' \
--bind 'shift-alt-page-up:change-prompt(shift-alt-pageup)' \
--bind 'shift-alt-page-down:change-prompt(shift-alt-pagedown)' \
\
--bind 'ctrl-alt-shift-up:change-prompt(ctrl-alt-shift-up)' \
--bind 'ctrl-alt-shift-down:change-prompt(ctrl-alt-shift-down)' \
--bind 'ctrl-alt-shift-right:change-prompt(ctrl-alt-shift-right)' \
--bind 'ctrl-alt-shift-left:change-prompt(ctrl-alt-shift-left)' \
--bind 'ctrl-alt-shift-home:change-prompt(ctrl-alt-shift-home)' \
--bind 'ctrl-alt-shift-end:change-prompt(ctrl-alt-shift-end)' \
--bind 'ctrl-alt-shift-delete:change-prompt(ctrl-alt-shift-delete)' \
--bind 'ctrl-alt-shift-page-up:change-prompt(ctrl-alt-shift-pageup)' \
--bind 'ctrl-alt-shift-page-down:change-prompt(ctrl-alt-shift-pagedown)' \
\
--bind 'backspace:change-prompt(backspace)' \
--bind 'ctrl-backspace:change-prompt(ctrl-backspace)' \
--bind 'alt-backspace:change-prompt(alt-backspace)' \
--bind 'ctrl-alt-backspace:change-prompt(ctrl-alt-backspace)' \
@junegunn This is now ready for review.
Thanks, I tested your patch using clear_all_shortcuts yes option in Kitty, and everything seems to work well, great job.
One thing I noticed after enabling clear_all_shortcuts in Kitty is that now Kitty recognizes CMD key as super, and sends an ANSI sequence that fzf doesn't recognize.
For example, pressing CMD-a leaves 97;9u on the prompt, CMD-b, 98;9u, etc.
Do we want to support this super key as well? (This was actually my first time hearing about it.) Maybe another time?
Thanks, I tested your patch using clear_all_shortcuts yes option in Kitty, and everything seems to work well, great job.
Thank you.
For example, pressing CMD-a leaves 97;9u on the prompt, CMD-b, 98;9u, etc.
Sounds like kittys own enhanced keyboard protocol.
Do we want to support this super key as well? (This was actually my first time hearing about it.) Maybe another time?
No, I'm afraid not. That would go far beyond the scope of this PR.
xterm is the de facto standard in terminal emulation. We now have everything what is possible with pure xterm to ensure that it works everywhere. So there should be no exceptions. As long as something implements xterm, it just works. And they all implement xterm first, other stuff maybe if at all.
Everything else is an extension, and even if it is available in large projects such as vim or emacs and supported by some terminals, you cannot rely on it. Sticking with pure xterm also has disadvantages.
Personally, I miss the Shift-*-Backspace key combinations.
Furthermore, the Super key in Linux (and the Windows key in Windows) is assigned to many GUI actions by default. This makes it a poor modifier, as the operating system would often intercept the key combination anyway before it could reach the terminal window. Not sure how the situation is like on Macs in this regard.
No, I'm afraid not. That would go far beyond the scope of this PR.
Agreed. Thanks for the detailed explanation.
Furthermore, the Super key in Linux (and the Windows key in Windows) is assigned to many GUI actions by default. This makes it a poor modifier.
On a related note, when I enable clear_all_shortcuts on Kitty, I'm unable to paste the clipboard content using CMD-V, which is annoying. So I don't know if I'm going to use the option even though it means I can't utilize these new keys in fzf.
On a related note, when I enable clear_all_shortcuts on Kitty, I'm unable to paste the clipboard content using CMD-V, which is annoying. So I don't know if I'm going to use the option even though it means I can't utilize these new keys in fzf.
Have you tried it without clear_all_shortcuts? My expectation is that most of them work and some don't because Kitty has bound internal actions to them.
Kitty has a set of default keyboard shortcuts to perform certain actions. For example, Ctrl+Shift+Page Down to scroll down the page. When you press this shortcut, it is not passed on to your shell, but Kitty intercepts it and executes its internal scroll function to visually scroll up the terminal window.
In general, it's good to have a sane set of default keyboard shortcuts, and Ctrl+Shift+Page Down is certainly one of them. However, this would have made testing this new feature difficult, as some would have been shadowed by Kitty's default keyboard shortcuts. Therefore, I suggested clearing all default keyboard shortcuts so that none of them would interfere with the test.
For actual use, you should certainly not use clear_all_shortcuts, but instead unbind the key combos in Kitty that you want to use in fzf.
Oops, tcell 2.9 requires Go 1.23, but fzf is still using 1.20 to support Windows 7.
$ go run -tags tcell main.go
src/tui/tcell.go:10:2: missing go.sum entry for module providing package github.com/gdamore/tcell/v2 (imported by github.com/junegunn/fzf/src/tui); to add:
go get github.com/junegunn/fzf/src/tui
$ go get github.com/junegunn/fzf/src/tui
go: upgraded golang.org/x/sys v0.30.0 => v0.35.0
go: upgraded golang.org/x/term v0.29.0 => v0.34.0
go: upgraded golang.org/x/text v0.21.0 => v0.28.0
$ go run -tags tcell main.go
golang.org/x/text/encoding/internal/identifier: cannot compile Go 1.23 code
golang.org/x/text/transform: cannot compile Go 1.23 code
github.com/gdamore/tcell/v2/terminfo: cannot compile Go 1.23 code
golang.org/x/sys/unix: cannot compile Go 1.23 code
We'll eventully drop support for Windows 7 and move on.
Microsoft ended security patches for Windows 7 after January 2020
It's been 5 years, so this might be the good time for that. But let me first release a patch version for the pending bug fixes with Go 1.20 for the Windows 7 users, then we'll move on.
We'll eventully drop support for Windows 7 and move on.
I would just like to briefly note that, theoretically, you can stay on tcell 2.8.x, which means you “only” lose the key combinations Ctrl-Alt-* and Ctrl-Alt-Shift-* in Windows. Everything else works as it does now. Linux and MacOSX would function completely.
But I also understand the reasons for discontinuing Windows 7 support. Maintaining different sets of functions for different platforms is very tedious, especially when there are no really good reasons for doing so.
junegunn added this to the 0.66.0 milestone yesterday
Cool. I am looking forward to 0.66.0 :smiley:
Is there anything left for me to do?
Everything's fine. I just want to wait a bit to see if the community has any feedback on that release.