Skip to content

changed escape sequence for alt-arrow to work on bash on os x#417

Merged
parisk merged 2 commits intoxtermjs:masterfrom
BobReid:os_x_bash_arrow_fix
Dec 19, 2016
Merged

changed escape sequence for alt-arrow to work on bash on os x#417
parisk merged 2 commits intoxtermjs:masterfrom
BobReid:os_x_bash_arrow_fix

Conversation

@BobReid
Copy link
Copy Markdown

@BobReid BobReid commented Dec 16, 2016

These are the escape sequences that make alt-arrow work on os x bash.

I have just started poking around the code base and am not super familiar with it. Any guidance on how to make this cross platform / cross shell would be appreciated.

@BobReid
Copy link
Copy Markdown
Author

BobReid commented Dec 16, 2016

Fix for #225

src/xterm.js Outdated
// http://unix.stackexchange.com/a/108106
if (result.key == '\x1b[1;3D') {
result.key = '\x1b[1;5D';
result.key = '\x1bb';
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

You can wrap this in a platform check by using if (browser.isMac). This change also fails a test which will need its own if statement.

@parisk do you expect this to break anything? I'm concerned about how #225 (comment) could not repro and other it may not behave in other shells like zsh.

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

I just tested with zsh.
I actually reprod the issue with zsh as well with the old escape codes. The new escape codes actually works with both bash and zsh on my mac which actually caught me by surprise.

I will wrap the changes in the isMac check and update the tests

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

@Tyriar as for the test I could tackle it in two ways.

  1. Add a simple if check to the tests to check the platform. downside being the test is now dependent on the platform it runs on
  2. Add additional tests and mock out browser check so all cases are tested regardless of platform.

What is preferable to you?

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

I see that there are already some test that mock out the browser check so I will go with that for now.

@BobReid
Copy link
Copy Markdown
Author

BobReid commented Dec 16, 2016

Updated the PR.
Added the browser check and fixed tests

@parisk
Copy link
Copy Markdown
Contributor

parisk commented Dec 19, 2016

Thanks @BobReid!

@parisk parisk merged commit 593a4ec into xtermjs:master Dec 19, 2016
@BobReid BobReid deleted the os_x_bash_arrow_fix branch December 19, 2016 15:23
@aliatsis
Copy link
Copy Markdown

this still doesn't fix alt+Backspace to delete words on mac.
should that be a separate issue?

@BobReid
Copy link
Copy Markdown
Author

BobReid commented Dec 19, 2016

alt+Backspace does not have a special function on my mac (bash & zsh on stock terminal and iTerm)

@parisk
Copy link
Copy Markdown
Contributor

parisk commented Dec 20, 2016

On my tests Alt + Backspace just deletes a character in bash.

@aliatsis what macOS version are you running currently? Also are you using the stock terminal with bash?

@aliatsis
Copy link
Copy Markdown

@parisk I'm on Sierra (10.12).. it was the same in Yosemite before I upgraded.
Yes, I am using the stock terminal with bash

Tyriar added a commit to microsoft/vscode that referenced this pull request Dec 28, 2016
The terminal should be a lot faster at processing output now thanks to
xtermjs/xterm.js#422

Also of note: xtermjs/xterm.js#417: changed escape sequence for alt-arrow to work on bash on os x
@Tyriar Tyriar modified the milestone: 2.3.0 Feb 2, 2017
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.

4 participants