Remove "bind -k" terminfo key names#11342
Conversation
| 'f' => opts.mode = BIND_FUNCTION_NAMES, | ||
| 'h' => opts.print_help = true, | ||
| 'k' => opts.use_terminfo = true, | ||
| 'K' => opts.mode = BIND_KEY_NAMES, |
There was a problem hiding this comment.
This will remove these options so it'll fall back to the "unknown option" error.
I think we should make these print a more specific error that tells you to rewrite your bindings.
I see this being triggered by old configuration a bunch.
| #[rustfmt::skip] | ||
| return Box::new([ | ||
| terminfo_add!(key_a1), terminfo_add!(key_a3), terminfo_add!(key_b2), | ||
| terminfo_add!(key_backspace), terminfo_add!(key_beg), terminfo_add!(key_btab), |
There was a problem hiding this comment.
Does that mean we can remove the keys from terminal.rs too?
|
On Tue, Apr 01, 2025 at 08:17:50AM -0700, Fabian Boehm wrote:
'h' => opts.print_help = true,
- 'k' => opts.use_terminfo = true,
- 'K' => opts.mode = BIND_KEY_NAMES,
This will remove these options so it'll fall back to the "unknown option" error.
I think we should make these print a more specific error that tells you to rewrite your bindings.
Yeah actually we should probably keep "bind --key-names" around (with all named keys... modifiers should probably get a separate flag)
I'm going with something like
bind -k nul 'echo foo'
# CHECKERR: bind: the -k/--key key name style is no longer supported. See `bind --key-names` for the new style
- return Box::new([
- terminfo_add!(key_a1), terminfo_add!(key_a3), terminfo_add!(key_b2),
- terminfo_add!(key_backspace), terminfo_add!(key_beg), terminfo_add!(key_btab),
Does that mean we can remove the keys from terminal.rs too?
yeah. I have started to look at removing all of terminfo.
I need to check again but it looks like we use terminfo for some workarounds
for terminals that are probably still not fixed, like
58347d4 (update PROMPT_SP heuristic, 2016-12-23) #789
3669805 (Improve compatibility with 0-16 color terminals., 2016-07-21) #3176
|
I don't think there's a relevant terminal where the "bind -k" notation is still needed. The remaining reason to keep it is backwards compatibility. But "bind -k" is already subtly broken on terminals that implement either of modifyOtherKeys, application keypad mode or the kitty keyboard protocol, since those alter the byte sequences (see fish-shell#11278). Having it randomly not work might do more harm than good. Remove it. This is meant go into 4.1, which means that users who switch back and forth between 4.1 and 4.0 can already use the new notation. If someone wants to use the bind config for a wider range of versions they could use "bind -k 2>/dev/null" etc. While at it, use the new key names in "bind --key-names".
5b38b0d to
2908c54
Compare
The biggest issue with terminfo is how often it is just plain wrong. I could not find a current terminal that has the xn capability unset correctly. You can find definitions that have it turned off, sure. You'll find an outdated "wezterm" definition (but wezterm sets $TERM to xterm-256color), you'll find "xterm+direct" (but xterm doesn't set that either). And you will find literally hundreds of museum pieces like the "wy350" from 1985. I can't find the terminfo definition for conemu (might be one called "msys"?), but if that's the only one we're better off hardcoding a hack for it or not supporting it. The only thing that is an even vaguely sensible use of terminfo is to check colors, and I don't know of an existing current terminal that does not understand the 256 color sequences (cool-retro-term ignores them because it doesn't do color, but I do not know of a terminal that is actually limited to 16 colors and barfs on these). We could just allow "dumb" and "xterm-mono" to turn off color, or support the "NO_COLOR" variable. |
that sounds good, because it's unlikely that these are set by accident.
that's good to know. I was also wondering what's the deal with |
(different things - that turns off syntax highlighting, but what if you can't have color escapes at all?)
Here's a full list of all terminfo definitions on my system that have setf but not setaf: Yes it's all museum pieces. Here's the script I used: for term in (path filter -rf /usr/share/terminfo/** | path basename)
tput -T $term setf &>/dev/null && not tput -T $term setaf &>/dev/null && echo $term
end |
This reverts commit 58347d4 (update PROMPT_SP heuristic, 2016-12-23) If we write the last column of a line: printf %0"$COLUMNS"d 0; sleep 3 most terminals will *not* move the cursor to the next line. This behavior is indicated by the terminfo's xenl (AKA xen or eat_newline_glitch) boolean capability. We originally added checks for this capability because ConEmu and Windows ConHost did not implement it, but they do now. Also, as mentioned in fish-shell#11342 (comment), we're not aware of any present-day terminal that does not have this behavior, but the value advertised by terminfo is sometimes wrong. Let's get rid of this for now. A following commit will document that we require this behavior.
This reverts commit 58347d4 (update PROMPT_SP heuristic, 2016-12-23) If we write the last column of a line: printf %0"$COLUMNS"d 0; sleep 3 most terminals will *not* move the cursor to the next line. This behavior is indicated by the terminfo's xenl (AKA xen or eat_newline_glitch) boolean capability. We originally added checks for this capability because ConEmu and Windows ConHost did not implement it, but they do now. Also, as mentioned in #11342 (comment), we're not aware of any present-day terminal that does not have this behavior, but the value advertised by terminfo is sometimes wrong. Let's get rid of this for now. A following commit will document that we require this behavior.
I don't think there's a relevant terminal where the "bind -k" notation is
still needed. The remaining reason to keep it is backwards compatibility.
But "bind -k" is already subtly broken on terminals that implement either
of modifyOtherKeys, application keypad mode or the kitty keyboard protocol,
since those alter the byte sequences (see #11278).
Having it randomly not work might do more harm than good. Remove it.
This is meant go into 4.1, which means that users who switch back and forth
between 4.1 and 4.0 can already use the new notation.
If someone wants to use a bind config for a wider range of versions they
could use "bind -k 2>/dev/null" etc.