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.

Sign up for GitHub

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

Jump to bottom

Alt-Backspace followed by Enter does not behave properly #2746

Closed
kylewlacy opened this issue Sep 13, 2019 · 4 comments · Fixed by #2823
Closed

Alt-Backspace followed by Enter does not behave properly #2746

kylewlacy opened this issue Sep 13, 2019 · 4 comments · Fixed by #2823
Assignees
@zadjii-msft
Labels
Area-Input Related to input processing (key presses, mouse, etc.) Issue-Bug It either shouldn't be doing this or needs an investigation. Priority-2 A description (P2) Product-Conpty For console issues specifically related to conpty Resolution-Fix-Committed Fix is checked in, but it might be 3-4 weeks until a release.
Milestone
20H1

Comments

@kylewlacy
Copy link

Pressing Alt+Backspace, then some other characters, followed by Enter will not trigger the right inputs for a shell through WSL. This issue is pretty subtle and it can be tricky to observe in my experience, so I have 2 test cases below to demonstrate this issue.

Environment

Windows build number: 10.0.18362.0
Windows Terminal version (if applicable): 0.4.2382.0 (latest available on the Windows Store)

Any other software? Ubuntu 18.04.3 LTS in WSL 1; Linux Mint with GNOME Terminal 3.28.1

Test case 1

Steps to reproduce

  1. Open a Bash terminal
  2. Type echo foo (don't press Enter)
  3. Press Alt+Backspace to delete the word (the command should now read echo)
  4. Type bar (the command should now read echo bar)
  5. Press Enter

Expected behavior

bar should be printed. This is the behavior when using WSL via vanilla Command Prompt, or using GNOME Terminal on a standalone Linux distro.

Actual behavior

Nothing happens. Pressing Alt+Backspace then pressing Enter inputs an escape sequence that would be expected from Alt+Enter, which Bash silently ignores by default.

To see this more visibly, run bind -x '"\e\r":"echo whoops"' then repeat the above steps. Now, whoops will be printed, since the above steps will input the escape sequence for Alt+Enter (\e\r).

Test case 2

Steps to reproduce

  1. Open a Bash terminal
  2. Run cat > sample.txt
  3. Type foo followed by Alt+Backspace followed by bar followed by Enter
  4. Press Ctrl+D to return to the prompt
  5. Run hexdump -C sample.txt

Expected behavior

hexdump should print the following output:

00000000  66 6f 6f 62 61 72 0a                              |foobar.|
00000007

This is the behavior when using WSL via vanilla Command Prompt, or using GNOME Terminal on a standalone Linux distro.

Actual behavior

hexdump prints the following output:

00000000  66 6f 6f 62 61 72 1b 0a                           |foobar..|
00000008

Just like before, we can see the sequence for Alt+Enter (\e\r, or 1b 0a in hex) is actually registered where we expected to see just \r (0a in hex).
@ghost ghost added Needs-Triage It's a new issue that the core contributor team needs to triage at the next triage meeting Needs-Tag-Fix Doesn't match tag requirements labels Sep 13, 2019
@DHowett-MSFT
Copy link
Contributor

Looks like we're carrying the Alt key state through to the next INPUT_RECORD. This reproduces with putty-wincon as well, so it's a conpty issue!

^      27 0033 0x1b
        127 0177 0x7f
^[^M     27 0033 0x1b
         13 0015 0x0d

@DHowett-MSFT DHowett-MSFT added Area-Input Related to input processing (key presses, mouse, etc.) Issue-Bug It either shouldn't be doing this or needs an investigation. Product-Conpty For console issues specifically related to conpty labels Sep 14, 2019
@ghost ghost removed the Needs-Tag-Fix Doesn't match tag requirements label Sep 14, 2019
@DHowett-MSFT DHowett-MSFT removed the Needs-Triage It's a new issue that the core contributor team needs to triage at the next triage meeting label Sep 16, 2019
@DHowett-MSFT DHowett-MSFT added this to the 20H1 milestone Sep 16, 2019
@DHowett-MSFT DHowett-MSFT added the Priority-2 A description (P2) label Sep 16, 2019
@DHowett-MSFT
Copy link
Contributor

Looks like recrudescence of #1209

@zadjii-msft zadjii-msft self-assigned this Sep 16, 2019
zadjii-msft added a commit that referenced this issue Sep 20, 2019
@zadjii-msft
Return to ground when we flush the last char
37d8840 
  The InputStateMachineEngine was incorrectly not returning to the ground state
  after flushing the last sequence. That means that something like alt+backspace
  would leave us in the Escape state, not the ground state. This makes sure we
  return to ground.

  Fixes #2746.

  Additionally removes the "Parser.UnitTests-common.vcxproj" file, which was
  originally used for a theoretical time when we only open-sourced the parser.
  It's unnecessary now, and we can get rid of it.

  Also includes a small patch to bcz.cmd, to make sure bx works with projects
  with a space in their name.
@zadjii-msft zadjii-msft mentioned this issue Sep 20, 2019
Merged
3 tasks
@ghost ghost added the In-PR This issue has a related PR label Sep 20, 2019
@ghost ghost added Needs-Tag-Fix Doesn't match tag requirements Resolution-Fix-Committed Fix is checked in, but it might be 3-4 weeks until a release. and removed In-PR This issue has a related PR labels Oct 4, 2019
zadjii-msft added a commit that referenced this issue Oct 4, 2019
@zadjii-msft
Return to ground when we flush the last char (#2823)
53c81a0 
## Summary of the Pull Request
The InputStateMachineEngine was incorrectly not returning to the ground state after flushing the last sequence. That means that something like alt+backspace would leave us in the Escape state, not the ground state. This makes sure we return to ground.

Additionally removes the "Parser.UnitTests-common.vcxproj" file, which was originally used for a theoretical time when we only open-sourced the parser. It's unnecessary now, and we can get rid of it.

Also includes a small patch to bcz.cmd, to make sure bx works with projects with a space in their name.

<!-- Please review the items on the PR checklist before submitting-->
## PR Checklist
* [x] Closes #2746
* [x] I work here
* [x] Tests added/passed
* [n/a] Requires documentation to be updated


<hr>

* Return to ground when we flush the last char

  The InputStateMachineEngine was incorrectly not returning to the ground state
  after flushing the last sequence. That means that something like alt+backspace
  would leave us in the Escape state, not the ground state. This makes sure we
  return to ground.

  Fixes #2746.

  Additionally removes the "Parser.UnitTests-common.vcxproj" file, which was
  originally used for a theoretical time when we only open-sourced the parser.
  It's unnecessary now, and we can get rid of it.

  Also includes a small patch to bcz.cmd, to make sure bx works with projects
  with a space in their name.

* Update src/terminal/parser/stateMachine.cpp

Co-Authored-By: Dustin L. Howett (MSFT) <duhowett@microsoft.com>

* add the comment @miniksa wanted
@ghost ghost removed the Needs-Tag-Fix Doesn't match tag requirements label Oct 4, 2019
@ghost
Copy link

ghost commented Oct 23, 2019

🎉This issue was addressed in #2823, which has now been successfully released as Windows Terminal Preview v0.6.2951.0.:tada:

Handy links:

@DHowett-MSFT
Copy link
Contributor

Hey! The fix for this went out in Windows in the Insider channel's Fast Ring with build 19013. It's worked in Windows Terminal since v0.6, and now it'll work in other terminals using ConPTY on Windows.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees

@zadjii-msft zadjii-msft

Labels
Area-Input Related to input processing (key presses, mouse, etc.) Issue-Bug It either shouldn't be doing this or needs an investigation. Priority-2 A description (P2) Product-Conpty For console issues specifically related to conpty Resolution-Fix-Committed Fix is checked in, but it might be 3-4 weeks until a release.
Projects
None yet
Milestone
20H1
Development

Successfully merging a pull request may close this issue.

Return to ground when we flush the last char microsoft/terminal
3 participants
@kylewlacy @DHowett-MSFT @zadjii-msft