DRAFT - Fix Various Home / End key binding issues #462

kcculhwch · 2022-08-25T14:52:40Z

After playing around with various keybinding issues affecting MacOS, iTerm2, and other ncurses loading terminal emulators, I stumbled upon references to switching the terminal mode so that key bindings align with the output of terminfo (infocmp) for kend and khome. Basically, until the terminal emulator has been set to 'smkx' ("keypad-transmit" mode) home/end keys send normal mode bindings which use a different style escape sequences. This code slots in at the setup phase before we use the ncurses based libraries to gather the keybindings.

If you want to test the intended functionality here prior to pulling try running the following

tput smkx && irb

if you want to see what the raw chars look like in various mode

tput smkx && read will let you see the raw escape code of keypad-transmit mode

Some terminals and shells will reset after program exit, but some do not, so to return to receive mode:

tput rmkx

Note this is my first PR in this project and I didn't see any submission guidelines... so please let me know what changes would be required.

Note 2 ... MacOS's Terminal.app has a variation of home/end key bindings and prefers shift+home and shift+end but these map to kend and khome rather than kEND and kHOM

many terminals will only use terminfo string in application mode. smkx commands will transition the terminal to the correct mode.

Cleanup some line spacing, add comments in case people want to know what the code does add at_exit block so that we return the console back to rx mode upon exist

…vi key command list

kcculhwch · 2022-08-25T16:20:07Z

Added in delete key functionality for terminfo and added in to the vi list

kcculhwch · 2022-08-25T23:09:42Z

Removed key_delete from line_editor as it was not necessary.

sshock · 2022-09-11T23:38:02Z

lib/reline/ansi.rb

      config.add_default_key_binding_by_keymap(:emacs, key, func)
    end
  end
+


These 10 lines of code already exist below, so now it is in two places. Did you mean to remove it from below?

Ahh yes, I'll clean that up.

sshock · 2022-09-11T23:40:33Z

lib/reline/ansi.rb


  def self.set_default_key_bindings_terminfo(config)
+
+    # put terminal in tx mode so that


I think "Put terminal in keypad transmit mode" would be a little more accurate.

sshock · 2022-09-12T00:04:50Z

lib/reline/ansi.rb

+      # capname is undefined
+    end
+
+    # return terminal to rx mode


This comment needs to be fixed too. It's not about "transmit" vs "receive". Leaving keypad transmit mode doesn't mean the terminal goes into "receive" mode.

I think either of these would work fine:

"Leave keypad transmit mode"

"Return terminal to normal mode".

For more context, consider xterm: with xterm smkx turns into \E[?1h and \E=, which sets the cursor keys and keypad to application mode, whereas rmkx turns into \E[?1l and \E>, which sets the cursor keys and keypad to normal mode.

sshock · 2022-09-12T00:15:55Z

@kcculhwch I noticed that your PR here has some overlap with my PR, which makes me wonder if we should combine them in some way...

For example, if you want to pull my changes into yours (mine is only 4 lines), that might be a good idea and then perhaps I'll delete mine (or if nothing else, at least there won't be a conflict when they both get merged).

Speaking of getting merged, my PR is only 4 lines long yet sadly nothing happened on it for 6 months. Then 11 days ago I was asked if I could add some tests, but I haven't quite figured out how to do that yet.

kcculhwch · 2022-09-13T15:40:43Z

@sshock I'll go ahead and pull your changes into mine when I get a moment.

sshock · 2022-09-18T23:27:10Z

lib/reline/ansi.rb

    'cud' => :ed_next_history,
    'cuf' => :ed_next_char,
    'cub' => :ed_prev_char,
+    'kdch1' => :key_delete 


Hi @kcculhwch, I have another suggestion that makes sense to include in your PR:

The four lines above this for 'cuu', 'cud', 'cuf', and 'cub' don't belong and should be removed (along with the special handling for them on lines 100-101). With your smkx fix in place, I don't think they'll be needed anymore.

I think what happened is @aycabta discovered that the cursor keys weren't working with some terminal emulators, including xterm, and he discovered that by mapping these it fixed it.

But that wasn't the right fix; the right fix is your PR, because as you and I both discovered independently, until the terminal has been set to 'smkx' mode, the home and end keys may send different escape sequences that don't align with the output of terminfo. Well, the same is true for the cursor keys.

In other words, this list already includes 'kcuu1', 'kcud1', 'kcuf1', and 'kcub1', which are the correct things to map for the arrow keys. They just weren't working (for some terminals) because reline wasn't putting the terminal in 'smkx' mode. Now with your PR they will work.

The escape sequences for 'cuu', 'cud', 'cuf', and 'cub' are not something the terminal sends when arrow keys are pressed! They are what you send to the terminal to make the cursor move.

It just so happens that in some terminals (well xterm at least), the escape sequences for 'cuu', 'cud', 'cuf', and 'cub' with the "%p1%d" removed from the middle happen to result in the escape sequences for the arrow keys in normal mode, but we don't need to rely on this happenstance anymore.

Maumagnaguagno · 2022-09-21T21:27:15Z

lib/reline/ansi.rb

+      begin
+        @@output.write Reline::Terminfo.tigetstr('rmkx')
+      rescue Reline::Terminfo::TerminfoError
+        # capname is undefined
+      end


Note that you can avoid the extra begin/end for the rescue to work inside the block since #12906.

sshock · 2023-02-04T17:03:31Z

@sshock I'll go ahead and pull your changes into mine when I get a moment.

@kcculhwch My PR got merged last month, so the key binding for delete is now there in master.

Do you want to update your branch against the latest and continue with this PR (including the cleanups we talked about)?

Home and End are still very much broken, at least with xterm, until we get this PR in. As we both know, an application must enter keypad-transmit mode to ensure key codes align with terminfo.

kcculhwch · 2023-02-06T12:09:03Z

Yeah. I’ve been a bit too busy lately to revisit. If I get a chance this week I’ll go ahead and do that.

…

On Sat, Feb 4, 2023 at 12:03 PM Phillip Hellewell ***@***.***> wrote: @sshock <https://github.com/sshock> I'll go ahead and pull your changes into mine when I get a moment. @kcculhwch <https://github.com/kcculhwch> My PR <#434> got merged last month, so the key binding for delete is now there in master. Do you want to update your branch against the latest and continue with this PR (including the cleanups we talked about)? Home and End are still very much broken, at least with xterm, until we get this PR in. As we both know, an application must enter keypad-transmit mode to ensure key codes align with terminfo. — Reply to this email directly, view it on GitHub <#462 (comment)>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/ACVCIGHYZLAR7AU6ULCKLMLWV2DW7ANCNFSM57TNXQ7Q> . You are receiving this because you were mentioned.Message ID: ***@***.***>

sshock · 2023-03-19T00:53:31Z

lib/reline/ansi.rb

+    end
+
+    # return terminal to rx mode
+    at_exit do


I think we should send the "smkx" and "rmkx" in prep and deprep rather than doing it here in set_default_key_bindings_terminfo and having to rely on an at_exit, which can be problematic.

sshock · 2023-03-19T01:36:41Z

@kcculhwch I created a new PR that is similar to this one, but incorporates all the feedback here.

The main difference is it sends "smkx" and "rmkx" in prep and deprep, which I think is more appropriate and avoids relying on at_exit.

Let me know what you think and if you want to close out this PR now you can.

sshock · 2023-07-11T02:52:33Z

@kcculhwch can you close this out? After talking with @tompng in #521, it is more clear to me that entering application mode has additional nuances we would need to handle. We are going to move forward with a different solution, probably #567.

st0012 · 2023-07-11T15:00:09Z

@sshock 👋 While I appreciate your passion and effort on solving the issue, can you please don't ask other contributors to close their PRs before we actually merge a solution?

kcculhwch · 2023-07-11T16:21:47Z

I don't mind either way, my work has taken me out of this convo for a while. Happy to close this out if it's no longer helping move the conversation forward, but if keeping it open is helping find a better solution, I don't mind keeping it open either.

…

On Tue, Jul 11, 2023 at 11:00 AM Stan Lo ***@***.***> wrote: @sshock <https://github.com/sshock> 👋 While I appreciate your passion and effort on solving the issue, can you please don't ask other contributors to close their PRs before we actually merge a solution? — Reply to this email directly, view it on GitHub <#462 (comment)>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/ACVCIGF6YXLANVD2MENRQKDXPVTAJANCNFSM57TNXQ7Q> . You are receiving this because you were mentioned.Message ID: ***@***.***>

sshock · 2023-07-12T03:11:43Z

@sshock wave While I appreciate your passion and effort on solving the issue, can you please don't ask other contributors to close their PRs before we actually merge a solution?

Sorry about that. I'll go re-open my version of this PR too (#521).

st0012 · 2023-07-12T21:26:14Z

@sshock No worries. Thanks for pushing the issue forward 👍

tompng · 2023-07-19T13:35:17Z

This is fixed by #569

kcculhwch added 3 commits August 25, 2022 01:08

use the smkx sequence to change terminal mode

4afe301

many terminals will only use terminfo string in application mode. smkx commands will transition the terminal to the correct mode.

Add spacing, comments, and return to rx mode

3e86762

Cleanup some line spacing, add comments in case people want to know what the code does add at_exit block so that we return the console back to rx mode upon exist

Add key_delete and kdch1 codes to ansi.rb and key_delete code to the …

50c43f6

…vi key command list

didn't need this here

537ee1e

sshock reviewed Sep 11, 2022

View reviewed changes

sshock reviewed Sep 12, 2022

View reviewed changes

sshock mentioned this pull request Sep 12, 2022

In macOS, home and end keys no longer work ruby/irb#330

Closed

sshock reviewed Sep 18, 2022

View reviewed changes

Maumagnaguagno reviewed Sep 21, 2022

View reviewed changes

This was referenced Oct 10, 2022

Always set default key bindings in lists #433

Closed

Home/End keys on numpad don't move cursor ruby/irb#408

Closed

sshock reviewed Mar 19, 2023

View reviewed changes

sshock mentioned this pull request Mar 19, 2023

Fix Home and End keys on macOS #521

Closed

tompng closed this Jul 19, 2023


		def self.set_default_key_bindings_terminfo(config)

		# put terminal in tx mode so that

DRAFT - Fix Various Home / End key binding issues #462

DRAFT - Fix Various Home / End key binding issues #462

Uh oh!

Conversation

kcculhwch commented Aug 25, 2022 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

kcculhwch commented Aug 25, 2022

Uh oh!

kcculhwch commented Aug 25, 2022

Uh oh!

sshock Sep 11, 2022

Choose a reason for hiding this comment

Uh oh!

kcculhwch Sep 13, 2022

Choose a reason for hiding this comment

Uh oh!

sshock Sep 11, 2022

Choose a reason for hiding this comment

Uh oh!

sshock Sep 12, 2022

Choose a reason for hiding this comment

Uh oh!

sshock commented Sep 12, 2022

Uh oh!

kcculhwch commented Sep 13, 2022 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

sshock Sep 18, 2022

Choose a reason for hiding this comment

Uh oh!

Maumagnaguagno Sep 21, 2022

Choose a reason for hiding this comment

Uh oh!

sshock commented Feb 4, 2023

Uh oh!

kcculhwch commented Feb 6, 2023 via email

Uh oh!

sshock Mar 19, 2023

Choose a reason for hiding this comment

Uh oh!

sshock commented Mar 19, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

sshock commented Jul 11, 2023

Uh oh!

st0012 commented Jul 11, 2023

Uh oh!

kcculhwch commented Jul 11, 2023 via email

Uh oh!

sshock commented Jul 12, 2023

Uh oh!

st0012 commented Jul 12, 2023

Uh oh!

tompng commented Jul 19, 2023

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants

kcculhwch commented Aug 25, 2022 •

edited

Loading

kcculhwch commented Sep 13, 2022 •

edited

Loading

sshock commented Mar 19, 2023 •

edited

Loading