Remove SetWrapForced from AdaptDispatch::_WriteToBuffer #20145

Closed
opened 2026-01-31 07:04:57 +00:00 by claunia · 2 comments
Owner

Originally created by @lhecker on GitHub (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.

Originally created by @lhecker on GitHub (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.
claunia added the Area-OutputIssue-BugResolution-DuplicateProduct-Conpty labels 2026-01-31 07:04:58 +00:00
Author
Owner

@carlos-zamora commented on GitHub (Jul 10, 2024):

/dup #17225

@carlos-zamora commented on GitHub (Jul 10, 2024): /dup #17225
Author
Owner

@microsoft-github-policy-service[bot] commented on GitHub (Jul 10, 2024):

Hi! We've identified this issue as a duplicate of another one that already exists on this Issue Tracker. This specific instance is being closed in favor of tracking the concern over on the referenced thread. Thanks for your report!

@microsoft-github-policy-service[bot] commented on GitHub (Jul 10, 2024): Hi! We've identified this issue as a duplicate of another one that already exists on this Issue Tracker. This specific instance is being closed in favor of tracking the concern over on the referenced thread. Thanks for your report! <!-- Policy app identification https://img.shields.io/static/v1?label=PullRequestIssueManagement. -->
Sign in to join this conversation.
1 Participants
Notifications
Due Date
No due date set.
Dependencies

No dependencies set.

Reference: starred/terminal#20145