Skip to content

Conversation

@tompng
Copy link
Member

@tompng tompng commented Jul 11, 2023

Fixes HOME key, END key, Meta + Arrow key problem in some terminal emulator.

ANSI cursor sequence

Most terminals are vt100 compatible. In vt100 normal mode, ANSI cursor sequence "\e[A", "\e[B", "\e[C", "\e[D" will be sent on arrow key press.
("Cursor Control" section of https://vt100.net/docs/vt100-ug/chapter3.html, "Minimum requirements for VT100 emulation" section of https://www.real-world-systems.com/docs/ANSIcode.html)
These arrow key sequence are normally not written in terminfo. This pull request make Reline bind these cursor sequences as default.

HOME \e[H and END \e[F keys are also categorized as cursor sequence. You can check this by setting Cursor Key Mode (DECCKM). HOME/END key will change the behavior just like arrow key does.

# This hack is not needed anymore.
when 'cuu', 'cud', 'cuf', 'cub'
  [ key_code.sub(/%p1%d/, '').bytes, key_binding ]

Modifiers

In https://mdfs.net/Docs/Comp/Comms/ANSIKeys, it says that terminal emulator will add a modifier value like "\e[1;3A" for Meta+Up, "\e[1;5A" for Ctrl+Up. Some terminal emulator uses "\e\e[A" for Meta+Up instead of modifier value.

Up Left Meta + Up Meta + Left Ctrl + Up Ctrl + Left
Terminal.app \e[A \e[D \e\e[A \eb \e[A \e[1;5D
iTerm2 \e[A \e[D \e\e[A \e\e[D \e[1;5A \e[1;5D
Alacritty \e[A \e[D \e[1;3A \e[1;3D \e[1;5A \e[1;5D
VSCode Terminal \e[A \e[D \e[1;3A \eb \e[1;5A \e[1;5D

(\eb is handled by Reline::KeyStroke#compress_meta_key)

This pull request also bind these arrow + modifier key sequences.

Reduce comprehensive list

Some bytes in set_default_key_bindings_comprehensive_list will be registered as default. I've removed them from comprehensive_list.

[27, 91, 49, 59, 53, 67] => :em_next_word, # Ctrl+→, extended entry
[27, 91, 49, 59, 53, 68] => :ed_prev_word, # Ctrl+←, extended entry
[27, 91, 49, 59, 51, 67] => :em_next_word, # Meta+→, extended entry
[27, 91, 49, 59, 51, 68] => :ed_prev_word, # Meta+←, extended entry
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

"\e[1;5C", "\e[1;5D", "\e[1;3C", "\e[1;3D" will be bound in set_default_key_bindings_ansi_cursor

[27, 27, 91, 67] => :em_next_word, # Option+→, extended entry
[27, 27, 91, 68] => :ed_prev_word, # Option+←, extended entry
[195, 166] => :em_next_word, # Option+f
[195, 162] => :ed_prev_word, # Option+b
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[195, 166] and [195,162] are unicode bytes for æ and â. It's not an escape sequence.

In mac, option+f option+b will insert ƒ in any textarea and also in iTerm2, but we don't want to add 'ƒ'.bytes '∫'.bytes to this list. Same for æ and â.

# Escape sequences that omit the move distance and are set to defaults
# value 1 may be sometimes sent by pressing the arrow-key.
when 'cuu', 'cud', 'cuf', 'cub'
[ key_code.sub(/%p1%d/, '').bytes, key_binding ]
Copy link
Member Author

@tompng tompng Jul 11, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

# searched cuu from terminfo
cuu=\E[%p1%dA
cuu=\233%p1%dA
cuu=\Ep-%p1%d;
cuu=\E&a-%p1%dR
cuu=\E[%p1%dA$<5>
cuu=\E[%p1%dA$<30>
cuu=\037up %p1%d\r
cuu=\035up %p1%d;

these are not for input sequence. It's for output sequence like \e[42A = move cursor up 42 rows.
This pattern does not match to \e[A (UP) nor \e[1;2A (UP+SHIFT).
We need to explicitly bind "\e[A", not irrelevant_value.gsub(replace_it, to_match_CSI_A)

bindings = [["\e[#{char}", default_func]] # CSI + char
if modifiers[:ctrl]
# CSI + ctrl_key_modifier + char
bindings << ["\e[1;5#{char}", modifiers[:ctrl]]
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

In https://mdfs.net/Docs/Comp/Comms/ANSIKeys, it says "\e[1;5D" and also "\e[5D" is valid as Ctrl+Left.
Although, I think we only need 1;5 style because set_default_key_bindings_comprehensive_list only had \e[1;5D, emacs and readline only supports \e[1;5D style.

@tompng tompng requested a review from a team July 18, 2023 12:16
Copy link
Member

@st0012 st0012 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Generally looks good and the description/comments are easy to follow 👍

2 questions:

  • Since this PR sets new default bindings, I think it's an enhancement?
  • Can you provide a few manual test steps to check the behaviour changes? I can perform them on Terminal.app, iTerm2 and VS Code Terminal.

@tompng tompng added the enhancement New feature or request label Jul 19, 2023
@tompng
Copy link
Member Author

tompng commented Jul 19, 2023

enhancement?

I think it is also an enhancement. I added a label.

Manual test steps are

  1. Disable keyboard shortcut for mission control that uses ctrl + arrows
    Screenshot 2023-07-19 at 14 52 01

  2. Type foo bar baz to IRB

  3. Press (move cursor), Ctrl+← Ctrl+→ Option+← Option+→ (move to prev/next word), Home(fn+←) End(fn+→) (move to beginning/end of line), (move history)

  4. Don't to forget reenable keyboard shortcuts 😄

I think the change will be like this

Keys Terminal iTerm VSCode
left/right --- --- ---
up/down --- --- ---
Ctrl left/right --- --- ---
Option left/right --- improved ---
Home End no input improved improved

(Terminal.app does not input sequence for Home and End keys)

@tompng
Copy link
Member Author

tompng commented Jul 19, 2023

To check the input sequence, use STDIN.raw{STDIN.wait_readable; sleep 0.1; STDIN.readpartial(10)}

@sshock
Copy link
Contributor

sshock commented Jul 19, 2023

LGTM

@st0012
Copy link
Member

st0012 commented Jul 19, 2023

I think the change will be like this

I can confirm these changes with my testing 👍

To check the input sequence, use STDIN.raw{STDIN.wait_readable; sleep 0.1; STDIN.readpartial(10)}

This is a very useful snippet. Can you add it to the Contributing session of README too?

Copy link
Member

@st0012 st0012 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is awesome, thank you!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

enhancement New feature or request

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants