Skip to content
This repository has been archived by the owner on Apr 19, 2024. It is now read-only.

Arrow keys don't work when terminal is in keypad-transmit mode #228

Closed
romkatv opened this issue Jun 20, 2019 · 6 comments · Fixed by #367
Closed

Arrow keys don't work when terminal is in keypad-transmit mode #228

romkatv opened this issue Jun 20, 2019 · 6 comments · Fixed by #367
Labels

Comments

@romkatv
Copy link

romkatv commented Jun 20, 2019

To reproduce:

  1. Start a clean Bash session.
bash --norc
  1. Switch terminal to keypad-transmit mode (smkx).
echo $'\e[?1h\E='
  1. Run countrylist example.
go run examples/countrylist.go
  1. Press down arrow key.

Expected: Country selection moves down.

Actual: Country selection doesn't move and an error message is printed:

Unexpected Escape Sequence: ['\x1b' 'O']

The reason is that in keypad-transmit mode down arrow key sends ^[OB instead of ^[[B that survey expects.

More info:

@AlecAivazis
Copy link
Owner

Thanks for reporting this @romkatv! I am a bit swamped atm so I don't think I will have time to address this quickly but if you wanted to take a stab at fixing it, I'd be happy to review a PR!

@romkatv
Copy link
Author

romkatv commented Jun 20, 2019

Oh, this is not urgent at all. In fact, feel free to never fix this issue or close it. I've only opened it to let you know there is likely a bug. I should've mentioned this in the description.

@adonovan
Copy link
Contributor

adonovan commented Sep 8, 2021

My team is seeing this from time to time in our survey-based application. I'm not sure what I did to put my terminal in keypad-transmit mode, but thanks for the explanation and for the echo command, which allows me to reproduce the problem. The problem was quite persistent across runs until I reset the terminal, and I expect few of our users will understand the rather obscure cause and remedy, so please do regard this an open problem. I'd be happy to contribute a fix if you could advise on the right approach, e.g.

  • change ReadRune to skip over and discard the ESC O ... sequences?
  • interpret them as keys?
  • return an error that mentions the problematic terminal mode and remedy? or
  • change the state of the terminal out of this mode?

More useful explanation here: fish-shell/fish-shell#2139 (comment)

@AlecAivazis
Copy link
Owner

Hey @adonovan - sorry to hear its affecting your users. Do you mind giving #367 a test and seeing if it works? I'm struggling to find the time to pull it down myself

@mislav
Copy link
Collaborator

mislav commented Sep 9, 2021

@AlecAivazis At GitHub CLI we're also fairly interested in seeing this fixed. The proposed fix in the linked PR looks good to me code-wise, but I haven't yet tested in across different platforms. I could do that soon and report back.

If you are struggling to find time these days and if you are open to giving us maintainer bits so we could ship a fix for this and tag a patch release, feel free to reach out to me! 🙇

@adonovan
Copy link
Contributor

adonovan commented Sep 9, 2021

I confirm that the PR fixes the issue seen by me and @mislav; see my comment at https://github.com/AlecAivazis/survey/pull/367/files#r705362421. Thanks!

pgavlin added a commit to pulumi/pulumi that referenced this issue Nov 8, 2022
Pick up the bugfix to AlecAivazis/survey#228.

This also provides a cleaner API for setting survey icons.
bors bot added a commit to pulumi/pulumi that referenced this issue Nov 8, 2022
11118: ci: Update post-release PR to align with release process r=AaronFriel a=AaronFriel

As the release process now includes a "freeze PR" which updates `.version` _prior_ to release, the post-release PR shouldn't attempt to make the same change, as it causes a merge conflict.

This removes the notion of "next-version" from the CI scripts, as that post-release PR was the only place it was used.

The post-release PR can also be auto-merged, so `queue-merge` is set to true.

11249: ci: Reduce Windows test parallelism r=AaronFriel a=AaronFriel

Increases likelihood of CI passing on Windows due to longstanding CPU exhaustion bug on runners. (GitHub support ticket 1791026 on my account.)



11284: Make convert's pcl output yaml agnostic r=Frassle a=Frassle

<!--- 
Thanks so much for your contribution! If this is your first time contributing, please ensure that you have read the [CONTRIBUTING](https://github.com/pulumi/pulumi/blob/master/CONTRIBUTING.md) documentation.
-->

# Description

<!--- Please include a summary of the change and which issue is fixed. Please also include relevant motivation and context. -->

We're going to use convert for more data sources than just YAML (e.g. hcl, arm, etc). So we don't want the PCL output option to be YAML specific given it should be usable for any input source.


11288: [cli] Update the survey module r=pgavlin a=pgavlin

Pick up the bugfix to AlecAivazis/survey#228.

This also provides a cleaner API for setting survey icons.

11289: [cli] Add a newline in the refresh confirmation prompt r=pgavlin a=pgavlin

The lack of a newline causes the prompt to wrap in terminals that are fewer than 120 or so columns wide. The wrapping confuses the survey package's redraw, which causes the repeated prompts each time the survey is redrawn (i.e. on every keypress). Removing the wrapping by adding a newline avoids this issue.

Co-authored-by: Aaron Friel <mayreply@aaronfriel.com>
Co-authored-by: Fraser Waters <fraser@pulumi.com>
Co-authored-by: Pat Gavlin <pat@pulumi.com>
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants