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

Remove SetWrapForced from AdaptDispatch::_WriteToBuffer #15602

Open
lhecker opened this issue Jun 26, 2023 · 0 comments
Open

Remove SetWrapForced from AdaptDispatch::_WriteToBuffer #15602

lhecker opened this issue Jun 26, 2023 · 0 comments
Labels
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. Product-Conpty For console issues specifically related to conpty
Milestone

Comments

@lhecker
Copy link
Member

lhecker commented Jun 26, 2023

From what I understand, a row should only be marked as force-wrapped if we actually wrote past the last column, which naturally occurs already in the AdaptDispatch::_DoLineFeed call.

Removing the SetWrapForced call however breaks these unit tests:

ConptyOutputTests::InvalidateUntilOneBeforeEnd
TerminalCoreUnitTests::ConptyRoundtripTests::BreakLinesOnCursorMovement#metadataSet0
TerminalCoreUnitTests::ConptyRoundtripTests::BreakLinesOnCursorMovement#metadataSet1
TerminalCoreUnitTests::ConptyRoundtripTests::BreakLinesOnCursorMovement#metadataSet2
TerminalCoreUnitTests::ConptyRoundtripTests::BreakLinesOnCursorMovement#metadataSet3
TerminalCoreUnitTests::ConptyRoundtripTests::BreakLinesOnCursorMovement#metadataSet4
TerminalCoreUnitTests::ConptyRoundtripTests::BreakLinesOnCursorMovement#metadataSet5
TerminalCoreUnitTests::ConptyRoundtripTests::BreakLinesOnCursorMovement#metadataSet6
TerminalCoreUnitTests::ConptyRoundtripTests::ResizeRepaintVimExeBuffer
TerminalCoreUnitTests::ConptyRoundtripTests::WrapNewLineAtBottom#metadataSet6
TerminalCoreUnitTests::ConptyRoundtripTests::WrapNewLineAtBottom#metadataSet7
TerminalCoreUnitTests::ConptyRoundtripTests::WrapNewLineAtBottom#metadataSet8
TerminalCoreUnitTests::ConptyRoundtripTests::WrapNewLineAtBottomLikeMSYS#metadataSet6
TerminalCoreUnitTests::ConptyRoundtripTests::WrapNewLineAtBottomLikeMSYS#metadataSet7
TerminalCoreUnitTests::ConptyRoundtripTests::WrapNewLineAtBottomLikeMSYS#metadataSet8

They break because when a ROW is not marked as force-wrapped the output behavior of the VtEngine and its output changes. We need to investigate why they break, what the correct behavior should be and how we can fix it. Presumably most unit tests simply assert the wrong behavior, but it's also possible that VtEngine::_PaintUtf8BufferLine and its removeSpaces variable may have to be updated.

@lhecker lhecker added Product-Conpty For console issues specifically related to conpty 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. labels Jun 26, 2023
@lhecker lhecker added this to the Terminal v1.19 milestone Jun 26, 2023
@microsoft-github-policy-service microsoft-github-policy-service bot added the Needs-Triage It's a new issue that the core contributor team needs to triage at the next triage meeting label Jun 26, 2023
@carlos-zamora carlos-zamora removed the Needs-Triage It's a new issue that the core contributor team needs to triage at the next triage meeting label Jun 28, 2023
@lhecker lhecker modified the milestones: Terminal v1.19, Backlog Sep 5, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
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. Product-Conpty For console issues specifically related to conpty
Projects
None yet
Development

No branches or pull requests

2 participants