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

strange behavior with tab[char] +line wrap + backspace in conhost/conpty #8839

Closed
Tracked by #8000
LuanVSO opened this issue Jan 21, 2021 · 3 comments · Fixed by #15567
Closed
Tracked by #8000

strange behavior with tab[char] +line wrap + backspace in conhost/conpty #8839

LuanVSO opened this issue Jan 21, 2021 · 3 comments · Fixed by #15567
Labels
Area-Output Related to output processing (inserting text into buffer, retrieving buffer text, etc.) Impact-Visual It look bad. In-PR This issue has a related PR 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
Milestone
Backlog

Comments

@LuanVSO
Copy link
Contributor

LuanVSO commented Jan 21, 2021

Environment

Windows build number: 10.0.19042.746
Windows Terminal version (if applicable):1.5.3242.0

Any other software?

Steps to reproduce

compile and run this simple code:

#include <stdio.h>
#include <stdlib.h>
int main(void)
{
	scanf("%*s",NULL);
	return 0;
}
  • then type [tab] + [any character]
  • hold tab again until the cursor goes to next line
  • hold backspace to delete all character's

Expected behavior

the character inserted gets removed

Actual behavior

it doesn't
image
@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 Jan 21, 2021
@j4james
Copy link
Collaborator

j4james commented Jan 21, 2021

It's worth mentioning that this also happens when you start conhost in the legacy v1 mode, so I don't think it's a new bug.

@zadjii-msft
Copy link
Member

Oh wow this happens all the way back to v1? That's crazy. I'm sure there some horrifying thing that's happening in WriteCharsLegacy to cause this.

@zadjii-msft zadjii-msft added Area-Output Related to output processing (inserting text into buffer, retrieving buffer text, 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 Jan 21, 2021
@zadjii-msft zadjii-msft added this to the Windows vNext milestone Jan 21, 2021
@ghost ghost removed the Needs-Tag-Fix Doesn't match tag requirements label Jan 21, 2021
@DHowett DHowett added Impact-Visual It look bad. and removed Needs-Triage It's a new issue that the core contributor team needs to triage at the next triage meeting labels Jan 22, 2021
@DHowett
Copy link
Member

DHowett commented Jan 22, 2021

Since it's not a regression, I moved it into the console backlog. Some buffer changes are coming that might make this more or less bad. We'll evaluate.

@zadjii-msft zadjii-msft mentioned this issue Aug 23, 2021
Closed
15 tasks
@zadjii-msft zadjii-msft modified the milestones: Console Backlog, Backlog Jan 4, 2022
@lhecker lhecker mentioned this issue Jun 23, 2023
Merged
@microsoft-github-policy-service microsoft-github-policy-service bot added the In-PR This issue has a related PR label Jun 25, 2023
microsoft-github-policy-service bot pushed a commit that referenced this issue Jun 30, 2023
@lhecker
We've been trying to reach you about your WriteCharsLegacy's extended…
94e6b91 
… Emoji support (#15567)

This is a complete rewrite of the old `WriteCharsLegacy` function
which is used when VT mode is disabled as well as for all interactive
console input handling on Windows. The previous code was almost
horrifying in some aspects as it first wrote the incoming text into a
local buffer, stripping/replacing any control characters. That's not
particular fast and never was. It's unknown why it was like that.

It also measured the width of each glyph to correctly determine the
cursor position and line wrapping. Presumably this used to work quite
well in the original console code, because it would then just copy
that local buffer into the destination text buffer, but with the
introduction of the broken and extremely slow `OutputCellIterator`
abstraction this would end up measuring all text twice and cause
disagreements between `WriteCharsLegacy`'s idea of the cursor position
and `OutputCellIterator`'s cursor position. Emoji input was basically
entirely broken. This PR fixes it by passing any incoming text
straight to the `TextBuffer` as well as by using its cursor positioning
facilities to correctly implement wrapping and backspace handling.

Backspacing over Emojis and an array of other aspects still don't work
correctly thanks to cmdline.cpp, but it works quite a lot better now.

Related to #8000
Closes #8839
Closes #10808

## Validation Steps Performed
* Printing various Unicode text ✅
* On an fgets() input line
  * Typing text works ✅
  * Inserting text works anywhere ✅
  * Ctrl+X is translated to ^X ✅
  * Null is translated to ^@ ✅
    This was tested by hardcoding the `OutputMode` to 3 instead of 7.
  * Backspace only advances to start of the input ✅
  * Backspace deletes the entire preceding tab ✅
  * Backspace doesn't delete whitespace preceding a tab ✅
  * Backspacing a force-wrapped wide glyph unwraps the line break ✅
  * Backspacing ^X deletes both glyphs ✅
  * Backspacing a force-wrapped tab deletes trailing whitespace ✅
* When executing
  ```cpp
  fputs("foo: ", stdout);
  fgets(buffer, stdin);
  ```
  pressing tab and then backspace does not delete the whitespace
  that follows after the "foo:" string (= `sOriginalXPosition`).
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
Area-Output Related to output processing (inserting text into buffer, retrieving buffer text, etc.) Impact-Visual It look bad. In-PR This issue has a related PR 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
Projects
None yet
Milestone
Backlog
4 participants
@DHowett @j4james @zadjii-msft @LuanVSO