Skip to content

Commit e5182fb

Browse files
zadjii-msftzadjii-msft
authored
Make Conpty emit wrapped lines as actually wrapped lines (#4415)
## Summary of the Pull Request Changes how conpty emits text to preserve line-wrap state, and additionally adds rudimentary support to the Windows Terminal for wrapped lines. ## References * Does _not_ fix (!) #3088, but that might be lower down in conhost. This makes wt behave like conhost, so at least there's that * Still needs a proper deferred EOL wrap implementation in #780, which is left as a todo * #4200 is the mega bucket with all this work * MSFT:16485846 was the first attempt at this task, which caused the regression MSFT:18123777 so we backed it out. * #4403 - I made sure this worked with that PR before I even sent #4403 ## PR Checklist * [x] Closes #405 * [x] Closes #3367 * [x] I work here * [x] Tests added/passed * [n/a] Requires documentation to be updated ## Detailed Description of the Pull Request / Additional comments I started with the following implementation: When conpty is about to write the last column, note that we wrapped this line here. If the next character the vt renderer is told to paint get is supposed to be at the start of the following line, then we know that the previous line had wrapped, so we _won't_ emit the usual `\r\n` here, and we'll just continue emitting text. However, this isn't _exactly_ right - if someone fills the row _exactly_ with text, the information that's available to the vt renderer isn't enough to know for sure if this line broke or not. It is possible for the client to write a full line of text, with a `\n` at the end, to manually break the line. So, I had to also add the `lineWrapped` param to the `IRenderEngine` interface, which is about half the files in this changelist. ## Validation Steps Performed * Ran tests * Checked how the Windows Terminal behaves with these changes * Made sure that conhost/inception and gnome-terminal both act as you'd expect with wrapped lines from conpty
1 parent 1de07aa commit e5182fb

24 files changed

+512
-35
lines changed

‎src/cascadia/TerminalCore/Terminal.cpp

+16
Original file line numberDiff line numberDiff line change
@@ -454,6 +454,22 @@ void Terminal::_WriteBuffer(const std::wstring_view& stringView)
454454
// With well behaving shells during normal operation this safeguard should normally not be encountered.
455455
proposedCursorPosition.X = 0;
456456
proposedCursorPosition.Y++;
457+
458+
// Try the character again.
459+
i--;
460+
461+
// Mark the line we're currently on as wrapped
462+
463+
// TODO: GH#780 - This should really be a _deferred_ newline. If
464+
// the next character to come in is a newline or a cursor
465+
// movement or anything, then we should _not_ wrap this line
466+
// here.
467+
//
468+
// This is more WriteCharsLegacy2ElectricBoogaloo work. I'm
469+
// leaving it like this for now - it'll break for lines that
470+
// _exactly_ wrap, but we can't re-wrap lines now anyways, so it
471+
// doesn't matter.
472+
_buffer->GetRowByOffset(cursorPosBefore.Y).GetCharRow().SetWrapForced(true);
457473
}
458474

459475
_AdjustCursorPosition(proposedCursorPosition);

‎src/cascadia/UnitTests_TerminalCore/ConptyRoundtripTests.cpp

+263-2
Original file line numberDiff line numberDiff line change
@@ -44,7 +44,7 @@ using namespace Microsoft::Terminal::Core;
4444

4545
namespace TerminalCoreUnitTests
4646
{
47-
class TerminalBufferTests;
47+
class ConptyRoundtripTests;
4848
};
4949
using namespace TerminalCoreUnitTests;
5050

@@ -152,8 +152,14 @@ class TerminalCoreUnitTests::ConptyRoundtripTests final
152152
TEST_METHOD(SimpleWriteOutputTest);
153153
TEST_METHOD(WriteTwoLinesUsesNewline);
154154
TEST_METHOD(WriteAFewSimpleLines);
155+
155156
TEST_METHOD(PassthroughClearScrollback);
156157

158+
TEST_METHOD(TestWrappingALongString);
159+
TEST_METHOD(TestAdvancedWrapping);
160+
TEST_METHOD(TestExactWrappingWithoutSpaces);
161+
TEST_METHOD(TestExactWrappingWithSpaces);
162+
157163
TEST_METHOD(MoveCursorAtEOL);
158164

159165
private:
@@ -348,6 +354,262 @@ void ConptyRoundtripTests::WriteAFewSimpleLines()
348354
verifyData(termTb);
349355
}
350356

357+
void ConptyRoundtripTests::TestWrappingALongString()
358+
{
359+
auto& g = ServiceLocator::LocateGlobals();
360+
auto& renderer = *g.pRender;
361+
auto& gci = g.getConsoleInformation();
362+
auto& si = gci.GetActiveOutputBuffer();
363+
auto& hostSm = si.GetStateMachine();
364+
auto& hostTb = si.GetTextBuffer();
365+
auto& termTb = *term->_buffer;
366+
367+
_flushFirstFrame();
368+
_checkConptyOutput = false;
369+
370+
const auto initialTermView = term->GetViewport();
371+
372+
const auto charsToWrite = gsl::narrow_cast<short>(TestUtils::Test100CharsString.size());
373+
VERIFY_ARE_EQUAL(100, charsToWrite);
374+
375+
VERIFY_ARE_EQUAL(0, initialTermView.Top());
376+
VERIFY_ARE_EQUAL(32, initialTermView.BottomExclusive());
377+
378+
hostSm.ProcessString(TestUtils::Test100CharsString);
379+
380+
const auto secondView = term->GetViewport();
381+
382+
VERIFY_ARE_EQUAL(0, secondView.Top());
383+
VERIFY_ARE_EQUAL(32, secondView.BottomExclusive());
384+
385+
auto verifyBuffer = [&](const TextBuffer& tb) {
386+
auto& cursor = tb.GetCursor();
387+
// Verify the cursor wrapped to the second line
388+
VERIFY_ARE_EQUAL(charsToWrite % initialTermView.Width(), cursor.GetPosition().X);
389+
VERIFY_ARE_EQUAL(1, cursor.GetPosition().Y);
390+
391+
// Verify that we marked the 0th row as _wrapped_
392+
const auto& row0 = tb.GetRowByOffset(0);
393+
VERIFY_IS_TRUE(row0.GetCharRow().WasWrapForced());
394+
395+
const auto& row1 = tb.GetRowByOffset(1);
396+
VERIFY_IS_FALSE(row1.GetCharRow().WasWrapForced());
397+
398+
TestUtils::VerifyExpectedString(tb, TestUtils::Test100CharsString, { 0, 0 });
399+
};
400+
401+
verifyBuffer(hostTb);
402+
403+
VERIFY_SUCCEEDED(renderer.PaintFrame());
404+
405+
verifyBuffer(termTb);
406+
}
407+
408+
void ConptyRoundtripTests::TestAdvancedWrapping()
409+
{
410+
auto& g = ServiceLocator::LocateGlobals();
411+
auto& renderer = *g.pRender;
412+
auto& gci = g.getConsoleInformation();
413+
auto& si = gci.GetActiveOutputBuffer();
414+
auto& hostSm = si.GetStateMachine();
415+
auto& hostTb = si.GetTextBuffer();
416+
auto& termTb = *term->_buffer;
417+
const auto initialTermView = term->GetViewport();
418+
419+
_flushFirstFrame();
420+
421+
const auto charsToWrite = gsl::narrow_cast<short>(TestUtils::Test100CharsString.size());
422+
VERIFY_ARE_EQUAL(100, charsToWrite);
423+
424+
hostSm.ProcessString(TestUtils::Test100CharsString);
425+
hostSm.ProcessString(L"\n");
426+
hostSm.ProcessString(L" ");
427+
hostSm.ProcessString(L"1234567890");
428+
429+
auto verifyBuffer = [&](const TextBuffer& tb) {
430+
auto& cursor = tb.GetCursor();
431+
// Verify the cursor wrapped to the second line
432+
VERIFY_ARE_EQUAL(2, cursor.GetPosition().Y);
433+
VERIFY_ARE_EQUAL(20, cursor.GetPosition().X);
434+
435+
// Verify that we marked the 0th row as _wrapped_
436+
const auto& row0 = tb.GetRowByOffset(0);
437+
VERIFY_IS_TRUE(row0.GetCharRow().WasWrapForced());
438+
439+
const auto& row1 = tb.GetRowByOffset(1);
440+
VERIFY_IS_FALSE(row1.GetCharRow().WasWrapForced());
441+
442+
TestUtils::VerifyExpectedString(tb, TestUtils::Test100CharsString, { 0, 0 });
443+
TestUtils::VerifyExpectedString(tb, L" 1234567890", { 0, 2 });
444+
};
445+
446+
verifyBuffer(hostTb);
447+
448+
// First write the first 80 characters from the string
449+
expectedOutput.push_back(R"(!"#$%&'()*+,-./0123456789:;<=>?@ABCDEFGHIJKLMNOPQRSTUVWXYZ[\]^_`abcdefghijklmnop)");
450+
// Without line breaking, write the remaining 20 chars
451+
expectedOutput.push_back(R"(qrstuvwxyz{|}~!"#$%&)");
452+
// Clear the rest of row 1
453+
expectedOutput.push_back("\x1b[K");
454+
// This is the hard line break
455+
expectedOutput.push_back("\r\n");
456+
// Now write row 2 of the buffer
457+
expectedOutput.push_back(" 1234567890");
458+
// and clear everything after the text, because the buffer is empty.
459+
expectedOutput.push_back("\x1b[K");
460+
VERIFY_SUCCEEDED(renderer.PaintFrame());
461+
462+
verifyBuffer(termTb);
463+
}
464+
465+
void ConptyRoundtripTests::TestExactWrappingWithoutSpaces()
466+
{
467+
// This test (and TestExactWrappingWitSpaces) reveals a bug in the old
468+
// implementation.
469+
//
470+
// If a line _exactly_ wraps to the next line, we can't tell if the line
471+
// should really wrap, or manually break. The client app is writing a line
472+
// that's exactly the width of the buffer that manually linebreaked at the
473+
// end of the line, followed by another line.
474+
//
475+
// With the old PaintBufferLine interface, there's no way to know if this
476+
// case is because the line wrapped or not. Hence, the addition of the
477+
// `lineWrapped` parameter
478+
479+
auto& g = ServiceLocator::LocateGlobals();
480+
auto& renderer = *g.pRender;
481+
auto& gci = g.getConsoleInformation();
482+
auto& si = gci.GetActiveOutputBuffer();
483+
auto& hostSm = si.GetStateMachine();
484+
auto& hostTb = si.GetTextBuffer();
485+
auto& termTb = *term->_buffer;
486+
487+
const auto initialTermView = term->GetViewport();
488+
489+
_flushFirstFrame();
490+
491+
const auto charsToWrite = initialTermView.Width();
492+
VERIFY_ARE_EQUAL(80, charsToWrite);
493+
494+
for (auto i = 0; i < charsToWrite; i++)
495+
{
496+
// This is a handy way of just printing the printable characters that
497+
// _aren't_ the space character.
498+
const wchar_t wch = static_cast<wchar_t>(33 + (i % 94));
499+
hostSm.ProcessCharacter(wch);
500+
}
501+
502+
hostSm.ProcessString(L"\n");
503+
hostSm.ProcessString(L"1234567890");
504+
505+
auto verifyBuffer = [&](const TextBuffer& tb, const bool isTerminal) {
506+
auto& cursor = tb.GetCursor();
507+
// Verify the cursor wrapped to the second line
508+
VERIFY_ARE_EQUAL(1, cursor.GetPosition().Y);
509+
VERIFY_ARE_EQUAL(10, cursor.GetPosition().X);
510+
511+
// TODO: GH#780 - In the Terminal, neither line should be wrapped.
512+
// Unfortunately, until WriteCharsLegacy2ElectricBoogaloo is complete,
513+
// the Terminal will still treat the first line as wrapped. When #780 is
514+
// implemented, these tests will fail, and should again expect the first
515+
// line to not be wrapped.
516+
517+
// Verify that we marked the 0th row as _not wrapped_
518+
const auto& row0 = tb.GetRowByOffset(0);
519+
VERIFY_ARE_EQUAL(isTerminal, row0.GetCharRow().WasWrapForced());
520+
521+
const auto& row1 = tb.GetRowByOffset(1);
522+
VERIFY_IS_FALSE(row1.GetCharRow().WasWrapForced());
523+
524+
TestUtils::VerifyExpectedString(tb, LR"(!"#$%&'()*+,-./0123456789:;<=>?@ABCDEFGHIJKLMNOPQRSTUVWXYZ[\]^_`abcdefghijklmnop)", { 0, 0 });
525+
TestUtils::VerifyExpectedString(tb, L"1234567890", { 0, 1 });
526+
};
527+
528+
verifyBuffer(hostTb, false);
529+
530+
// First write the first 80 characters from the string
531+
expectedOutput.push_back(R"(!"#$%&'()*+,-./0123456789:;<=>?@ABCDEFGHIJKLMNOPQRSTUVWXYZ[\]^_`abcdefghijklmnop)");
532+
533+
// This is the hard line break
534+
expectedOutput.push_back("\r\n");
535+
// Now write row 2 of the buffer
536+
expectedOutput.push_back("1234567890");
537+
// and clear everything after the text, because the buffer is empty.
538+
expectedOutput.push_back("\x1b[K");
539+
VERIFY_SUCCEEDED(renderer.PaintFrame());
540+
541+
verifyBuffer(termTb, true);
542+
}
543+
544+
void ConptyRoundtripTests::TestExactWrappingWithSpaces()
545+
{
546+
// This test is also explained by the comment at the top of TestExactWrappingWithoutSpaces
547+
548+
auto& g = ServiceLocator::LocateGlobals();
549+
auto& renderer = *g.pRender;
550+
auto& gci = g.getConsoleInformation();
551+
auto& si = gci.GetActiveOutputBuffer();
552+
auto& hostSm = si.GetStateMachine();
553+
auto& hostTb = si.GetTextBuffer();
554+
auto& termTb = *term->_buffer;
555+
const auto initialTermView = term->GetViewport();
556+
557+
_flushFirstFrame();
558+
559+
const auto charsToWrite = initialTermView.Width();
560+
VERIFY_ARE_EQUAL(80, charsToWrite);
561+
562+
for (auto i = 0; i < charsToWrite; i++)
563+
{
564+
// This is a handy way of just printing the printable characters that
565+
// _aren't_ the space character.
566+
const wchar_t wch = static_cast<wchar_t>(33 + (i % 94));
567+
hostSm.ProcessCharacter(wch);
568+
}
569+
570+
hostSm.ProcessString(L"\n");
571+
hostSm.ProcessString(L" ");
572+
hostSm.ProcessString(L"1234567890");
573+
574+
auto verifyBuffer = [&](const TextBuffer& tb, const bool isTerminal) {
575+
auto& cursor = tb.GetCursor();
576+
// Verify the cursor wrapped to the second line
577+
VERIFY_ARE_EQUAL(1, cursor.GetPosition().Y);
578+
VERIFY_ARE_EQUAL(20, cursor.GetPosition().X);
579+
580+
// TODO: GH#780 - In the Terminal, neither line should be wrapped.
581+
// Unfortunately, until WriteCharsLegacy2ElectricBoogaloo is complete,
582+
// the Terminal will still treat the first line as wrapped. When #780 is
583+
// implemented, these tests will fail, and should again expect the first
584+
// line to not be wrapped.
585+
586+
// Verify that we marked the 0th row as _not wrapped_
587+
const auto& row0 = tb.GetRowByOffset(0);
588+
VERIFY_ARE_EQUAL(isTerminal, row0.GetCharRow().WasWrapForced());
589+
590+
const auto& row1 = tb.GetRowByOffset(1);
591+
VERIFY_IS_FALSE(row1.GetCharRow().WasWrapForced());
592+
593+
TestUtils::VerifyExpectedString(tb, LR"(!"#$%&'()*+,-./0123456789:;<=>?@ABCDEFGHIJKLMNOPQRSTUVWXYZ[\]^_`abcdefghijklmnop)", { 0, 0 });
594+
TestUtils::VerifyExpectedString(tb, L" 1234567890", { 0, 1 });
595+
};
596+
597+
verifyBuffer(hostTb, false);
598+
599+
// First write the first 80 characters from the string
600+
expectedOutput.push_back(R"(!"#$%&'()*+,-./0123456789:;<=>?@ABCDEFGHIJKLMNOPQRSTUVWXYZ[\]^_`abcdefghijklmnop)");
601+
602+
// This is the hard line break
603+
expectedOutput.push_back("\r\n");
604+
// Now write row 2 of the buffer
605+
expectedOutput.push_back(" 1234567890");
606+
// and clear everything after the text, because the buffer is empty.
607+
expectedOutput.push_back("\x1b[K");
608+
VERIFY_SUCCEEDED(renderer.PaintFrame());
609+
610+
verifyBuffer(termTb, true);
611+
}
612+
351613
void ConptyRoundtripTests::MoveCursorAtEOL()
352614
{
353615
// This is a test for GH#1245
@@ -360,7 +622,6 @@ void ConptyRoundtripTests::MoveCursorAtEOL()
360622
auto& hostSm = si.GetStateMachine();
361623
auto& hostTb = si.GetTextBuffer();
362624
auto& termTb = *term->_buffer;
363-
364625
_flushFirstFrame();
365626

366627
Log::Comment(NoThrowString().Format(

0 commit comments

Comments
 (0)