Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Ignore unhandled escape sequences #522

Merged
merged 2 commits into from Jul 8, 2023

Conversation

tompng
Copy link
Member

@tompng tompng commented Mar 20, 2023

Fixes #514

When unknown escape sequence is typed, reline prints the escape sequence.
In Mac Terminal.app, the keys are F1~F12 keys, option+A, option+up, etc.

This pull request will change reline to correctly ignore unmatched CSI sequence, SS3 sequence, ESC+char and single ESC input.

lib/reline/key_stroke.rb Outdated Show resolved Hide resolved
input[idx] ? [:matched, idx + 1] : [:matching, nil]
when 79 # == 'O'.ord
# SS3 sequence
input[idx + 1] ? [:matched, idx + 2] : [:matching, nil]
Copy link
Member Author

Choose a reason for hiding this comment

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

F1, F2, F3, F4 key in macOS ["\eOP", "\eOQ", "\eOR", "\eOS"]

There are many other type of escape sequence, but I think it's CSI and SS3 is enough for keyboard input sequence. some other sequence ends with "\a" or "\e\\" (example: "\e]0;NewWindowTitle\e\\") but if we implement it and user mistakenly input ESC+], irb looks hanging up.

@ima1zumi ima1zumi added the enhancement New feature or request label Mar 21, 2023
@tompng tompng force-pushed the ignore_unhandled_escape_sequences branch from ed9a5cc to bf643a1 Compare March 21, 2023 18:35
@sshock
Copy link
Contributor

sshock commented Mar 22, 2023

Not sure if y'all are looking for testers, but I can confirm that this fixes the F1, F2, F3, etc. keys to no longer spew out escape codes in an "xterm-256color" terminal on Debian Linux.

@tompng tompng force-pushed the ignore_unhandled_escape_sequences branch from bf643a1 to 732d3a9 Compare March 22, 2023 13:20
method_symbol = @config.editing_mode.get_method(key.char)
process_key(key.char, method_symbol)
if @config.editing_mode_is?(:vi_command) || @config.editing_mode_is?(:vi_insert)
# split ESC + key in vi mode
Copy link
Member Author

Choose a reason for hiding this comment

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

This is only used in vi mode.
In emacs mode, this is making ESC+char inserted into input buffer

@@ -3345,4 +3347,7 @@ def finish
@mark_pointer = new_pointer
end
alias_method :exchange_point_and_mark, :em_exchange_mark

private def em_meta_next(key)
end
Copy link
Member Author

@tompng tompng Mar 22, 2023

Choose a reason for hiding this comment

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

In method process_key, ed_insert is called if method_symbol is not defined.
I think adding this method is the simplest and easiest way to prevent ESC insertion.
Although, not all method_symbol written in lib/reline/key_actor/emacs.rb is defined in line_aditor.rb and I think it is not causing problem. Perhaps there is another way to fix without defining this method.

@tompng tompng force-pushed the ignore_unhandled_escape_sequences branch from 732d3a9 to 2c7188c Compare March 22, 2023 14:00
@tompng tompng marked this pull request as ready for review March 22, 2023 14:28
@tompng tompng force-pushed the ignore_unhandled_escape_sequences branch from 2c7188c to 09fcf0a Compare March 23, 2023 03:47
Copy link
Member

@ima1zumi ima1zumi left a comment

Choose a reason for hiding this comment

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

LGTM. Thank you so much!

@ima1zumi ima1zumi merged commit 0b63045 into ruby:master Jul 8, 2023
29 checks passed
matzbot pushed a commit to ruby/ruby that referenced this pull request Jul 8, 2023
(ruby/reline#522)

* Add unassigned escape sequence matcher to KeyStroke

* Do not insert ESC and unassigned ESC+key to input buffer
@tompng tompng deleted the ignore_unhandled_escape_sequences branch July 8, 2023 11:33
@tompng tompng mentioned this pull request Jul 19, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Development

Successfully merging this pull request may close these issues.

Ignore unknown or unimplemented escape sequence input
4 participants