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

Adds support for alternate HOME and END escape keys #87

Merged
merged 6 commits into from
Oct 3, 2023

Conversation

TWithers
Copy link
Contributor

@TWithers TWithers commented Sep 26, 2023

The HOME and END keys don't work on Mac in iTerm2. After searching around I found that they could be any of the following depending on the terminal type:

"\e[1~": beginning-of-line
"\e[4~": end-of-line
"\e[7~": beginning-of-line
"\e[8~": end-of-line
"\eOH": beginning-of-line
"\eOF": end-of-line
"\e[H": beginning-of-line
"\e[F": end-of-line

Rather than creating multiple Key::Home alternates (Home_1, Home_2, etc), I converted it to an array and added an oneOf() method for the Key class to help resolve it via the existing match functions.

@jessarcher jessarcher self-requested a review September 27, 2023 00:07
@jessarcher jessarcher marked this pull request as draft September 27, 2023 00:08
@jessarcher
Copy link
Member

Thanks, @TWithers!

I've found all these documented at https://en.wikipedia.org/wiki/ANSI_escape_code, except for \eOH and \eOF. Can you provide any more info on where these come from and where they are used?

@jessarcher
Copy link
Member

Also, do you think it's worth applying the same pattern to the arrow keys? We currently have two codes for each key (E.g. UP and UP_ARROW).

@TWithers
Copy link
Contributor Author

TWithers commented Oct 2, 2023

Thanks, @TWithers!

I've found all these documented at https://en.wikipedia.org/wiki/ANSI_escape_code, except for \eOH and \eOF. Can you provide any more info on where these come from and where they are used?

I found it here: https://wiki.archlinux.org/title/Home_and_End_keys_not_working#Readline
I am not sure which terminal version supports it, or if that terminal version is still around, so you are probably safe just to trust wiki.

@TWithers
Copy link
Contributor Author

TWithers commented Oct 2, 2023

Also, do you think it's worth applying the same pattern to the arrow keys? We currently have two codes for each key (E.g. UP and UP_ARROW).

It could be helpful and keep it simple, especially if there are more alternate keys that haven't been noticed or uncovered yet. It honestly wouldn't be a hard refactor either at this point if it is just the arrow keys that have the duplicates.

Let me know if you want me to make those 2 changes (0H and 0F removal, and the arrow key conversion) and commit them.

@jessarcher
Copy link
Member

Let me know if you want me to make those 2 changes (0H and 0F removal, and the arrow key conversion) and commit them.

My only concern with this PR is potentially breaking changes. I believe some folks have implemented their own prompts, which may depend on these constants being strings.

Ordinarily, it would be straightforward to just tag 0.2, but there are already a lot of packages depending on ^0.1 that would prevent 0.2 from being installed.

Maybe we leave the arrow keys for now and we can make that refactor when we have a stronger reason to tag a new major version.

@jessarcher jessarcher marked this pull request as ready for review October 2, 2023 23:38
@TWithers
Copy link
Contributor Author

TWithers commented Oct 2, 2023

Awesome! Sounds good to me.

@taylorotwell taylorotwell merged commit acb7add into laravel:main Oct 3, 2023
4 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants