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

Refactor input key reading #712

Merged
merged 5 commits into from
Jun 5, 2024
Merged

Refactor input key reading #712

merged 5 commits into from
Jun 5, 2024

Conversation

tompng
Copy link
Member

@tompng tompng commented May 29, 2024

Simplify input key reading

Depends on #709
Second step of #708

We don't need complex read_2nd_character_of_key_sequence and read_escaped_key.

This source code commend is partially right (only for ESC keys), but not correct.

# GNU Readline waits for "keyseq-timeout" milliseconds to see if the ESC
# is followed by a character, and times out and treats it as a standalone
# ESC if the second character does not arrive. If the second character
# comes before timed out, it is treated as a modifier key with the
# meta-property of meta-key, so that it can be distinguished from
# multibyte characters with the 8th bit turned on.

Background

With this inputrc file

# .inputrc
"abcd": "[ABCD]"
"ab": "[AB]"

Input ab is :matched and also :matching (part of abcd). In this case, Readline waits for keyseq-timeout milliseconds.
Input abc is :matching (part of abcd). In this case, Readline waits forever.
It's not ESC specific nor 2nd-char specific.

Changes

  1. Introduce another state :matching_matched for KeyStroke#match_status.
  2. Refactor key reading. timeout will be matching_matched ? keyseq_timeout : Infinity.

@tompng tompng force-pushed the read_key_refactor branch 3 times, most recently from df1cac5 to 2d80784 Compare May 29, 2024 17:24
@tompng tompng force-pushed the read_key_refactor branch from 2d80784 to 2fe13a1 Compare June 2, 2024 07:39
@tompng tompng force-pushed the read_key_refactor branch from 2fe13a1 to e66bc21 Compare June 2, 2024 17:36
lib/reline.rb Show resolved Hide resolved
@tompng tompng force-pushed the read_key_refactor branch from e66bc21 to eec33b4 Compare June 3, 2024 13:22
@tompng tompng marked this pull request as ready for review June 3, 2024 13:24
lib/reline.rb Outdated Show resolved Hide resolved
@tompng tompng force-pushed the read_key_refactor branch from eec33b4 to 61d8698 Compare June 3, 2024 14:57
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 a great simplification!

end
input[idx + 1] ? :unmatched : input[idx] ? :matched : :matching
input[idx + 1] ? UNMATCHED : input[idx] ? MATCHED : MATCHING
Copy link
Member

Choose a reason for hiding this comment

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

nit: can we please expand this, my brain isn't good enough to process nested ternary operators 😅

@tompng
Copy link
Member Author

tompng commented Jun 5, 2024

One test is failing (fails building native extension while installing io/console). I think it is not related to this change.

Passed job
https://github.com/tompng/reline/actions/runs/9377629652/job/25819506712

Set up job
  Runner Image
    Image: windows-2022
    Version: 20240514.3.0
Set up Ruby
  Print Ruby version
    ruby 3.4.0dev (2024-06-05T00:45:53Z master 63e9eaa5be) [x64-mswin64_140]

Failed job
https://github.com/ruby/reline/actions/runs/9377629917/job/25819752045

Set up job
  Runner Image
    Image: windows-2022
    Version: 20240603.1.0
Set up Ruby
  Print Ruby version
    ruby 3.4.0dev (2024-06-05T00:45:53Z master 63e9eaa5be) [x64-mswin64_140]

Same commit, same ruby version, different runner image verision.
I think ci on master branch will start failing soon. I ran ci in master https://github.com/ruby/reline/actions/runs/9377894574/job/25820174240 it passed maybe because it still uses windows-2022 ver: 20240514.3.0

@tompng tompng merged commit 64deec1 into ruby:master Jun 5, 2024
39 of 40 checks passed
@tompng tompng deleted the read_key_refactor branch June 5, 2024 04:04
matzbot pushed a commit to ruby/ruby that referenced this pull request Jun 5, 2024
(ruby/reline#712)

* Add key binding matching status :matching_matched

* Simplify read_2nd_character

* Add a comment of matching status and EOF

* Matching status to a constant

* Expand complicated ternary operators to case-when

ruby/reline@64deec100b
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging this pull request may close these issues.

2 participants