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

conhost VK_F4 regression #5033

Closed
tamlin-mike opened this issue Mar 20, 2020 · 7 comments · Fixed by #15783
Closed

conhost VK_F4 regression #5033

tamlin-mike opened this issue Mar 20, 2020 · 7 comments · Fixed by #15783
Assignees
Labels
Area-CookedRead The cmd.exe COOKED_READ handling Area-Input Related to input processing (key presses, mouse, etc.) Impact-Compatibility Sure don't work like it used to. In-PR This issue has a related PR Issue-Bug It either shouldn't be doing this or needs an investigation. Needs-Tag-Fix Doesn't match tag requirements Priority-2 A description (P2) Product-Conhost For issues in the Console codebase
Milestone

Comments

@tamlin-mike
Copy link

Environment

Windows 10.0.18363.592 (bug also present in some earlier build(s))

Steps to reproduce

  • cmd.exe
  • C:\>aaa bbb ccc ddd
  • Home, Ctrl-Right. Cursor should now be before the first 'b'.
  • F4, Space. 'bbb' gets deleted.

Expected behavior

Visual cursor position remains (i.e. now just before ' ccc').

Actual behavior

  1. Visual cursor position moves to EOL.
  2. To make it worse, the insertion-point still is where the caret was, If you now type 'x' command line goes from 'aaa ccc ddd' to 'aaa x ccc ddd', while the cursor remains at EOL!
@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 Mar 20, 2020
@zadjii-msft
Copy link
Member

Yikes yep that's annoying. I bet that this regressed in one of our earlier COOKED_READ refactors, though it's hard to know when exactly.

@zadjii-msft zadjii-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. Priority-2 A description (P2) Product-Conhost For issues in the Console codebase labels Mar 20, 2020
@ghost ghost removed the Needs-Tag-Fix Doesn't match tag requirements label Mar 20, 2020
@zadjii-msft zadjii-msft added this to the 21H1 milestone Mar 20, 2020
@DHowett-MSFT
Copy link
Contributor

Regressed in 19H1 (10.0.18362)

@DHowett-MSFT DHowett-MSFT added Impact-Compatibility Sure don't work like it used to. and removed Needs-Triage It's a new issue that the core contributor team needs to triage at the next triage meeting labels Mar 20, 2020
@DHowett-MSFT
Copy link
Contributor

(Verified using CHOP)

@zadjii-msft
Copy link
Member

note to self: still reproing in 10.0.22518.1000

@zadjii-msft zadjii-msft modified the milestones: Windows vNext, 22H1 Jan 4, 2022
@zadjii-msft zadjii-msft modified the milestones: 22H1, Terminal v1.14 Feb 2, 2022
@zadjii-msft zadjii-msft self-assigned this Feb 3, 2022
@zadjii-msft
Copy link
Member

Sharing notes because it's too close to 5pm:

Important to remember that the cmdline functions that open a popup do not get handled in CommandLine::ProcessCommandLine. It has to wait for more input, so it gets processed later. So somewhere inside of CopyFromCharPopup::Process is where I need to be doing the final call back to AdjustCursorPosition, not at the bottom of CommandLine::ProcessCommandLine

@zadjii-msft zadjii-msft added the Area-CookedRead The cmd.exe COOKED_READ handling label Feb 23, 2022
@zadjii-msft zadjii-msft removed their assignment Mar 16, 2022
@zadjii-msft zadjii-msft modified the milestones: Terminal v1.14, 22H2 Apr 28, 2022
@tamlin-mike

This comment was marked as spam.

@lhecker lhecker self-assigned this Jun 27, 2023
@lhecker
Copy link
Member

lhecker commented Jun 27, 2023

FYI I'm currently working towards rewriting the command line implementation from scratch, because the original code was hard to maintain and the rewrite that brought this issue is even worse (and incorrect on top of it). Thanks for pinging this issue - now I know to remember to test it.
Contrary to what you said, you will be able to use it on Windows 10, even in conhost, and I'd be happy to show you how.

Also, while I understand your anger, don't write comments like that please.

@zadjii-msft zadjii-msft modified the milestones: 22H2, Backlog Jul 5, 2023
@microsoft-github-policy-service microsoft-github-policy-service bot added the In-PR This issue has a related PR label Aug 2, 2023
@microsoft-github-policy-service microsoft-github-policy-service bot added the Needs-Tag-Fix Doesn't match tag requirements label Aug 25, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Area-CookedRead The cmd.exe COOKED_READ handling Area-Input Related to input processing (key presses, mouse, etc.) Impact-Compatibility Sure don't work like it used to. In-PR This issue has a related PR Issue-Bug It either shouldn't be doing this or needs an investigation. Needs-Tag-Fix Doesn't match tag requirements Priority-2 A description (P2) Product-Conhost For issues in the Console codebase
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants