Cleanup and format code for push.

src/cascadia/UnitTests_TerminalCore/ConptyRoundtripTests.cpp

@@ -2192,21 +2192,21 @@ void ConptyRoundtripTests::OutputWrappedLineWithSpaceAtBottomOfBuffer()
 void ConptyRoundtripTests::BreakLinesOnCursorMovement()
 {
     BEGIN_TEST_METHOD_PROPERTIES()
-        TEST_METHOD_PROPERTY(L"Data:cursorMovementMode", L"{0, 1, 2, 3, 4, 5}")
+        TEST_METHOD_PROPERTY(L"Data:cursorMovementMode", L"{0, 1, 2, 3, 4, 5, 6}")
     END_TEST_METHOD_PROPERTIES();
     constexpr int MoveCursorWithCUP = 0;
     constexpr int MoveCursorWithCR_LF = 1;
     constexpr int MoveCursorWithLF_CR = 2;
     constexpr int MoveCursorWithVPR_CR = 3;
     constexpr int MoveCursorWithCUB_NL = 4;
     constexpr int MoveCursorWithCUD_CR = 5;
+    constexpr int MoveCursorWithNothing = 6;
     INIT_TEST_PROPERTY(int, cursorMovementMode, L"Controls how we move the cursor, either with CUP, newline/carriage-return, or some other VT sequence");
 
     Log::Comment(L"This is a test for GH#5291. WSL vim uses spaces to clear the"
-                 L" ends of blank lines, not EL. This test ensures that conhost"
-                 L" properly treats those lines as _not actually wrapped_,"
-                 L" because if we leave them \"wrapped\", conpty won't newline"
-                 L" in between them.");
+                 L" ends of blank lines, not EL. This test ensures we emit text"
+                 L" from conpty such that the terminal re-creates the state of"
+                 L" the host, which includes wrapped lines of lots of spaces.");
     VERIFY_IS_NOT_NULL(_pVtRenderEngine.get());
 
     auto& g = ServiceLocator::LocateGlobals();
@@ -2219,6 +2219,8 @@ void ConptyRoundtripTests::BreakLinesOnCursorMovement()
 
     _flushFirstFrame();
 
+    // Any of the cursor movements that use a LF will actaully hard break the
+    // line - everything else will leave it marked as wrapped.
     const bool expectHardBreak = (cursorMovementMode == MoveCursorWithLF_CR) ||
                                  (cursorMovementMode == MoveCursorWithCR_LF) ||
                                  (cursorMovementMode == MoveCursorWithCUB_NL);
@@ -2232,12 +2234,9 @@ void ConptyRoundtripTests::BreakLinesOnCursorMovement()
 
         for (auto y = viewport.top<short>(); y < lastRow; y++)
         {
-            // VERIFY_IS_FALSE(tb.GetRowByOffset(y).GetCharRow().WasWrapForced());
+            // We're using CUP to move onto the status line _always_, so the
+            // second-last row will always be marked as wrapped.
             const auto rowWrapped = (!expectHardBreak) || (y == lastRow - 1);
-            Log::Comment(NoThrowString().Format(
-                L"y, lastRow, expectHardBreak, rowWrapped=%d, %d, %d, %d", y, lastRow, expectHardBreak, rowWrapped));
-
-            // VERIFY_ARE_EQUAL(!expectHardBreak, tb.GetRowByOffset(y).GetCharRow().WasWrapForced());
             VERIFY_ARE_EQUAL(rowWrapped, tb.GetRowByOffset(y).GetCharRow().WasWrapForced());
             TestUtils::VerifyExpectedString(tb, L"~    ", til::point{ 0, y });
         }
@@ -2330,6 +2329,12 @@ void ConptyRoundtripTests::BreakLinesOnCursorMovement()
                 hostSm.ProcessString(L"\r");
             }
         }
+        // Win32 vim.exe will simply do _nothing_ in this scenario. It'll just
+        // print the lines one after the other, without moving the cursor,
+        // relying on us auto moving to the following line.
+        else if (cursorMovementMode == MoveCursorWithNothing)
+        {
+        }
 
         // IMPORTANT! The way vim writes these blank lines is as '~' followed by
         // enough spaces to fill the line.

src/host/screenInfo.cpp

@@ -1663,23 +1663,18 @@ void SCREEN_INFORMATION::SetCursorDBMode(const bool DoubleCursor)
         return STATUS_INVALID_PARAMETER;
     }
 
-    // // GH#5291 - If we're moving the cursor, we're definitely manually breaking
-    // // the line. Check if the cursor is currently in the "delayed EOL wrap"
-    // // state. If it is, then the current cursor line is being treated as
-    // // wrapped, and the next char should go on the subsequent line.
-    // //
-    // // However, If we're moving the cursor manually, then we're certainly not
-    // // wrapping the current line. In that case, clear the wrap flag from the
-    // // current cursor position.
-    // const COORD oldCursorPos = cursor.GetPosition();
-    // if (cursor.IsDelayedEOLWrap())
-    // {
-    //     ROW& prevRow = _textBuffer->GetRowByOffset(oldCursorPos.Y);
-    //     if (prevRow.GetCharRow().WasWrapForced())
-    //     {
-    //         prevRow.GetCharRow().SetWrapForced(false);
-    //     }
-    // }
+    // In GH#5291, we experimented with manually breaking the line on all cursor
+    // movements here. As we print lines into the buffer, we mark lines as
+    // wrapped when we print the last cell of the row, not the first cell of the
+    // subsequent row (the row the first line wrapped onto).
+    //
+    // Logically, we thought that manaully breaking lines when we move the
+    // cursor was a good idea. We however, did not have the time to fully
+    // validate that this was the correct answer, and a simpler solution for the
+    // bug on hand was found. Furthermore, we thought it would be a more
+    // comprehensive solution to only mark lines as wrapped when we print the
+    // first cell of the second row, which would require some WriteCharsLegacy
+    // work.
 
     cursor.SetPosition(Position);
 

src/renderer/vt/paint.cpp

@@ -443,15 +443,23 @@ using namespace Microsoft::Console::Types;
     const bool useEraseChar = (optimalToUseECH) &&
                               (!_newBottomLine) &&
                               (!_clearedAllThisFrame);
+    const bool printingBottomLine = coord.Y == _lastViewport.BottomInclusive();
 
     // If we're not using erase char, but we did erase all at the start of the
     // frame, don't add spaces at the end.
     //
     // GH#5161: Only removeSpaces when we're in the _newBottomLine state and the
     // line we're trying to print right now _actually is the bottom line_
+    //
+    // GH#5291: DON'T remove spaces when the row wrapped. We might need those
+    // spaces to preserve the wrap state of this line, or the cursor position.
+    // For example, vim.exe uses "~    "... to clear the line, and then leaves
+    // the lines _wrapped_. It doesn't care to manually break the lines, but if
+    // we trimmed the spaces off here, we'd print all the "~"s one after another
+    // on the same line.
     const bool removeSpaces = !lineWrapped && (useEraseChar ||
                                                _clearedAllThisFrame ||
-                                               (_newBottomLine && coord.Y == _lastViewport.BottomInclusive()));
+                                               (_newBottomLine && printingBottomLine));
     const size_t cchActual = removeSpaces ?
                                  (cchLine - numSpaces) :
                                  cchLine;
@@ -547,7 +555,7 @@ using namespace Microsoft::Console::Types;
             RETURN_IF_FAILED(_EraseLine());
         }
     }
-    else if (_newBottomLine && coord.Y == _lastViewport.BottomInclusive())
+    else if (_newBottomLine && printingBottomLine)
     {
         // If we're on a new line, then we don't need to erase the line. The
         //      line is already empty.
@@ -566,7 +574,7 @@ using namespace Microsoft::Console::Types;
 
     // If we printed to the bottom line, and we previously thought that this was
     // a new bottom line, it certainly isn't new any longer.
-    if (coord.Y == _lastViewport.BottomInclusive())
+    if (printingBottomLine)
     {
         _newBottomLine = false;
     }

src/winconpty/winconpty.cpp

@@ -83,7 +83,7 @@ HRESULT _CreatePseudoConsole(const HANDLE hToken,
     RETURN_IF_WIN32_BOOL_FALSE(SetHandleInformation(signalPipeConhostSide.get(), HANDLE_FLAG_INHERIT, HANDLE_FLAG_INHERIT));
 
     // GH4061: Ensure that the path to executable in the format is escaped so C:\Program.exe cannot collide with C:\Program Files
-    const wchar_t* pwszFormat = L"\"%s\" %s%s--width %hu --height %hu --signal 0x%x --server 0x%x";
+    const wchar_t* pwszFormat = L"\"%s\" --headless %s%s--width %hu --height %hu --signal 0x%x --server 0x%x";
     // This is plenty of space to hold the formatted string
     wchar_t cmd[MAX_PATH]{};
     const BOOL bInheritCursor = (dwFlags & PSEUDOCONSOLE_INHERIT_CURSOR) == PSEUDOCONSOLE_INHERIT_CURSOR;