make filling chars (and, thus, erase line/char) unset wrap (#2831)

EraseInLine calls `FillConsoleOutputCharacterW()`. In filling the row with
chars, we were setting the wrap flag. We need to specifically not do this on
ANY _FILL_ operation. Now a fill operation UNSETS the wrap flag if we fill to
the end of the line.

Originally, we had a boolean `setWrap` that would mean...
- **true**: if writing to the end of the row, SET the wrap value to true
- **false**: if writing to the end of the row, DON'T CHANGE the wrap value

Now we're making this bool a std::optional to allow for a ternary state. This
allows for us to handle the following cases completely. Refer to the table
below:

,- current wrap value
|     ,- are we filling the last cell in the row?
|     |     ,- new wrap value
|     |     |     ,- comments
|--   |--   |--   |
| 0   | 0   | 0   |
| 0   | 1   | 0   |
| 0   | 1   | 1   | THIS CASE WAS HANDLED CORRECTLY
| 1   | 0   | 0   | THIS CASE WAS UNHANDLED
| 1   | 0   | 1   |
| 1   | 1   | 1   |

To handle that special case (1-0-0), we need to UNSET the wrap. So now, we have
~setWrap~ `wrap` mean the following:
- **true**: if writing to the end of the row, SET the wrap value to TRUE
- **false**: if writing to the end of the row, SET the wrap value to FALSE
- **nullopt**: leave the wrap value as it is

Closes #1126

src/buffer/out/Row.cpp

@@ -147,11 +147,11 @@ const UnicodeStorage& ROW::GetUnicodeStorage() const noexcept
 // Arguments:
 // - it - custom console iterator to use for seeking input data. bool() false when it becomes invalid while seeking.
 // - index - column in row to start writing at
-// - setWrap - set the wrap flags if we hit the end of the row while writing and there's still more data in the iterator.
+// - wrap - change the wrap flag if we hit the end of the row while writing and there's still more data in the iterator.
 // - limitRight - right inclusive column ID for the last write in this row. (optional, will just write to the end of row if nullopt)
 // Return Value:
 // - iterator to first cell that was not written to this row.
-OutputCellIterator ROW::WriteCells(OutputCellIterator it, const size_t index, const bool setWrap, std::optional<size_t> limitRight)
+OutputCellIterator ROW::WriteCells(OutputCellIterator it, const size_t index, const std::optional<bool> wrap, std::optional<size_t> limitRight)
 {
     THROW_HR_IF(E_INVALIDARG, index >= _charRow.size());
     THROW_HR_IF(E_INVALIDARG, limitRight.value_or(0) >= _charRow.size());
@@ -202,10 +202,15 @@ OutputCellIterator ROW::WriteCells(OutputCellIterator it, const size_t index, co
                 ++it;
             }
 
-            // If we're asked to set the wrap status and we just filled the last column with some text, set wrap status on the row.
-            if (setWrap && fillingLastColumn)
+            // If we're asked to (un)set the wrap status and we just filled the last column with some text...
+            // NOTE:
+            //  - wrap = std::nullopt    --> don't change the wrap value
+            //  - wrap = true            --> we're filling cells as a steam, consider this a wrap
+            //  - wrap = false           --> we're filling cells as a block, unwrap
+            if (wrap.has_value() && fillingLastColumn)
             {
-                _charRow.SetWrapForced(true);
+                // set wrap status on the row to parameter's value.
+                _charRow.SetWrapForced(wrap.value());
             }
         }
         else

src/buffer/out/Row.hpp

@@ -57,7 +57,7 @@ class ROW final
     UnicodeStorage& GetUnicodeStorage() noexcept;
     const UnicodeStorage& GetUnicodeStorage() const noexcept;
 
-    OutputCellIterator WriteCells(OutputCellIterator it, const size_t index, const bool setWrap, std::optional<size_t> limitRight = std::nullopt);
+    OutputCellIterator WriteCells(OutputCellIterator it, const size_t index, const std::optional<bool> wrap = std::nullopt, std::optional<size_t> limitRight = std::nullopt);
 
     friend bool operator==(const ROW& a, const ROW& b) noexcept;
 

src/buffer/out/textBuffer.cpp

@@ -318,10 +318,12 @@ OutputCellIterator TextBuffer::Write(const OutputCellIterator givenIt)
 // Arguments:
 // - givenIt - Iterator representing output cell data to write
 // - target - the row/column to start writing the text to
+// - wrap - change the wrap flag if we hit the end of the row while writing and there's still more data
 // Return Value:
 // - The final position of the iterator
 OutputCellIterator TextBuffer::Write(const OutputCellIterator givenIt,
-                                     const COORD target)
+                                     const COORD target,
+                                     const std::optional<bool> wrap)
 {
     // Make mutable copy so we can walk.
     auto it = givenIt;
@@ -336,7 +338,8 @@ OutputCellIterator TextBuffer::Write(const OutputCellIterator givenIt,
     while (it && size.IsInBounds(lineTarget))
     {
         // Attempt to write as much data as possible onto this line.
-        it = WriteLine(it, lineTarget, true);
+        // NOTE: if wrap = true/false, we want to set the line's wrap to true/false (respectively) if we reach the end of the line
+        it = WriteLine(it, lineTarget, wrap);
 
         // Move to the next line down.
         lineTarget.X = 0;
@@ -351,13 +354,13 @@ OutputCellIterator TextBuffer::Write(const OutputCellIterator givenIt,
 // Arguments:
 // - givenIt - The iterator that will dereference into cell data to insert
 // - target - Coordinate targeted within output buffer
-// - setWrap - Whether we should try to set the wrap flag if we write up to the end of the line and have more data
+// - wrap - change the wrap flag if we hit the end of the row while writing and there's still more data in the iterator.
 // - limitRight - Optionally restrict the right boundary for writing (e.g. stop writing earlier than the end of line)
 // Return Value:
 // - The iterator, but advanced to where we stopped writing. Use to find input consumed length or cells written length.
 OutputCellIterator TextBuffer::WriteLine(const OutputCellIterator givenIt,
                                          const COORD target,
-                                         const bool setWrap,
+                                         const std::optional<bool> wrap,
                                          std::optional<size_t> limitRight)
 {
     // If we're not in bounds, exit early.
@@ -368,7 +371,7 @@ OutputCellIterator TextBuffer::WriteLine(const OutputCellIterator givenIt,
 
     //  Get the row and write the cells
     ROW& row = GetRowByOffset(target.Y);
-    const auto newIt = row.WriteCells(givenIt, target.X, setWrap, limitRight);
+    const auto newIt = row.WriteCells(givenIt, target.X, wrap, limitRight);
 
     // Take the cell distance written and notify that it needs to be repainted.
     const auto written = newIt.GetCellDistance(givenIt);

src/buffer/out/textBuffer.hpp

@@ -89,11 +89,12 @@ class TextBuffer final
     OutputCellIterator Write(const OutputCellIterator givenIt);
 
     OutputCellIterator Write(const OutputCellIterator givenIt,
-                             const COORD target);
+                             const COORD target,
+                             const std::optional<bool> wrap = true);
 
     OutputCellIterator WriteLine(const OutputCellIterator givenIt,
                                  const COORD target,
-                                 const bool setWrap = false,
+                                 const std::optional<bool> setWrap = std::nullopt,
                                  const std::optional<size_t> limitRight = std::nullopt);
 
     bool InsertCharacter(const wchar_t wch, const DbcsAttribute dbcsAttribute, const TextAttribute attr);

src/host/_output.cpp

@@ -290,7 +290,10 @@ void WriteToScreen(SCREEN_INFORMATION& screenInfo, const Viewport& region)
     try
     {
         const OutputCellIterator it(character, lengthToWrite);
-        const auto done = screenInfo.Write(it, startingCoordinate);
+
+        // when writing to the buffer, specifically unset wrap if we get to the last column.
+        // a fill operation should UNSET wrap in that scenario. See GH #1126 for more details.
+        const auto done = screenInfo.Write(it, startingCoordinate, false);
         cellsModified = done.GetInputDistance(it);
 
         // Notify accessibility

src/host/ft_host/API_FillOutputTests.cpp

@@ -58,4 +58,125 @@ class FillOutputTests
                                                                 &charsWritten));
         VERIFY_ARE_EQUAL(1u, charsWritten);
     }
+
+    TEST_METHOD(UnsetWrap)
+    {
+        // WARNING: If this test suddenly decides to start failing,
+        // this is because the wrap registry key is not set.
+        // TODO GH #2859: Get/Set Registry Key for Wrap
+
+        HANDLE hConsole = GetStdOutputHandle();
+        DWORD charsWritten = 0;
+
+        CONSOLE_SCREEN_BUFFER_INFOEX sbiex = { 0 };
+        sbiex.cbSize = sizeof(sbiex);
+        VERIFY_WIN32_BOOL_SUCCEEDED(GetConsoleScreenBufferInfoEx(hConsole, &sbiex));
+
+        const auto consoleWidth = sbiex.dwSize.X;
+
+        std::wstring input(consoleWidth + 2, L'a');
+        std::wstring filled(consoleWidth, L'b');
+
+        // Write until a wrap occurs
+        VERIFY_WIN32_BOOL_SUCCEEDED(WriteConsoleW(hConsole,
+                                                  input.data(),
+                                                  gsl::narrow_cast<DWORD>(input.size()),
+                                                  &charsWritten,
+                                                  nullptr));
+
+        // Verify wrap occurred
+        std::unique_ptr<wchar_t[]> bufferText = std::make_unique<wchar_t[]>(consoleWidth);
+        DWORD readSize = 0;
+        VERIFY_WIN32_BOOL_SUCCEEDED(ReadConsoleOutputCharacterW(hConsole,
+                                                                bufferText.get(),
+                                                                consoleWidth,
+                                                                { 0, 0 },
+                                                                &readSize));
+
+        WEX::Common::String expected(input.c_str(), readSize);
+        WEX::Common::String actual(bufferText.get(), readSize);
+        VERIFY_ARE_EQUAL(expected, actual);
+
+        bufferText = std::make_unique<wchar_t[]>(2);
+        VERIFY_WIN32_BOOL_SUCCEEDED(ReadConsoleOutputCharacterW(hConsole,
+                                                                bufferText.get(),
+                                                                2,
+                                                                { 0, 1 },
+                                                                &readSize));
+
+        VERIFY_ARE_EQUAL(2u, readSize);
+        expected = WEX::Common::String(input.c_str(), readSize);
+        actual = WEX::Common::String(bufferText.get(), readSize);
+        VERIFY_ARE_EQUAL(expected, actual);
+
+        // Fill Console Line with 'b's
+        VERIFY_WIN32_BOOL_SUCCEEDED(FillConsoleOutputCharacterW(hConsole,
+                                                                L'b',
+                                                                consoleWidth,
+                                                                { 2, 0 },
+                                                                &charsWritten));
+
+        // Verify first line is full of 'a's then 'b's
+        bufferText = std::make_unique<wchar_t[]>(consoleWidth);
+        VERIFY_WIN32_BOOL_SUCCEEDED(ReadConsoleOutputCharacterW(hConsole,
+                                                                bufferText.get(),
+                                                                consoleWidth,
+                                                                { 0, 0 },
+                                                                &readSize));
+
+        expected = WEX::Common::String(input.c_str(), 2);
+        actual = WEX::Common::String(bufferText.get(), 2);
+        VERIFY_ARE_EQUAL(expected, actual);
+
+        expected = WEX::Common::String(filled.c_str(), consoleWidth - 2);
+        actual = WEX::Common::String(&bufferText[2], readSize - 2);
+        VERIFY_ARE_EQUAL(expected, actual);
+
+        // Verify second line is still has 'a's that wrapped over
+        bufferText = std::make_unique<wchar_t[]>(2);
+        VERIFY_WIN32_BOOL_SUCCEEDED(ReadConsoleOutputCharacterW(hConsole,
+                                                                bufferText.get(),
+                                                                static_cast<SHORT>(2),
+                                                                { 0, 0 },
+                                                                &readSize));
+
+        VERIFY_ARE_EQUAL(2u, readSize);
+        expected = WEX::Common::String(input.c_str(), 2);
+        actual = WEX::Common::String(bufferText.get(), readSize);
+        VERIFY_ARE_EQUAL(expected, actual);
+
+        // Resize to be smaller by 2
+        sbiex.srWindow.Right -= 2;
+        sbiex.dwSize.X -= 2;
+        VERIFY_WIN32_BOOL_SUCCEEDED(SetConsoleScreenBufferInfoEx(hConsole, &sbiex));
+
+        // Verify first line is full of 'a's then 'b's
+        bufferText = std::make_unique<wchar_t[]>(consoleWidth - 2);
+        VERIFY_WIN32_BOOL_SUCCEEDED(ReadConsoleOutputCharacterW(hConsole,
+                                                                bufferText.get(),
+                                                                consoleWidth - static_cast<SHORT>(2),
+                                                                { 0, 0 },
+                                                                &readSize));
+
+        expected = WEX::Common::String(input.c_str(), 2);
+        actual = WEX::Common::String(bufferText.get(), 2);
+        VERIFY_ARE_EQUAL(expected, actual);
+
+        expected = WEX::Common::String(filled.c_str(), consoleWidth - 4);
+        actual = WEX::Common::String(&bufferText[2], readSize - 2);
+        VERIFY_ARE_EQUAL(expected, actual);
+
+        // Verify second line is still has 'a's ('b's didn't wrap over)
+        bufferText = std::make_unique<wchar_t[]>(static_cast<SHORT>(2));
+        VERIFY_WIN32_BOOL_SUCCEEDED(ReadConsoleOutputCharacterW(hConsole,
+                                                                bufferText.get(),
+                                                                static_cast<SHORT>(2),
+                                                                { 0, 0 },
+                                                                &readSize));
+
+        VERIFY_ARE_EQUAL(2u, readSize);
+        expected = WEX::Common::String(input.c_str(), 2);
+        actual = WEX::Common::String(bufferText.get(), readSize);
+        VERIFY_ARE_EQUAL(expected, actual);
+    }
 };

src/host/screenInfo.cpp

@@ -2599,14 +2599,17 @@ OutputCellIterator SCREEN_INFORMATION::Write(const OutputCellIterator it)
 // Arguments:
 // - it - Iterator representing output cell data to write.
 // - target - The position to start writing at
+// - wrap - change the wrap flag if we hit the end of the row while writing and there's still more data
 // Return Value:
 // - the iterator at its final position
 // Note:
 // - will throw exception on error.
 OutputCellIterator SCREEN_INFORMATION::Write(const OutputCellIterator it,
-                                             const COORD target)
+                                             const COORD target,
+                                             const std::optional<bool> wrap)
 {
-    return _textBuffer->Write(it, target);
+    // NOTE: if wrap = true/false, we want to set the line's wrap to true/false (respectively) if we reach the end of the line
+    return _textBuffer->Write(it, target, wrap);
 }
 
 // Routine Description:

src/host/screenInfo.hpp

@@ -131,7 +131,8 @@ class SCREEN_INFORMATION : public ConsoleObjectHeader, public Microsoft::Console
     OutputCellIterator Write(const OutputCellIterator it);
 
     OutputCellIterator Write(const OutputCellIterator it,
-                             const COORD target);
+                             const COORD target,
+                             const std::optional<bool> wrap = true);
 
     OutputCellIterator WriteRect(const OutputCellIterator it,
                                  const Microsoft::Console::Types::Viewport viewport);

src/host/ut_host/TextBufferTests.cpp

@@ -77,6 +77,8 @@ class TextBufferTests
 
     TEST_METHOD(TestWrapFlag);
 
+    TEST_METHOD(TestWrapThroughWriteLine);
+
     TEST_METHOD(TestDoubleBytePadFlag);
 
     void DoBoundaryTest(PWCHAR const pwszInputString,
@@ -197,6 +199,78 @@ void TextBufferTests::TestWrapFlag()
     VERIFY_IS_FALSE(Row.GetCharRow().WasWrapForced());
 }
 
+void TextBufferTests::TestWrapThroughWriteLine()
+{
+    TextBuffer& textBuffer = GetTbi();
+
+    auto VerifyWrap = [&](bool expected) {
+        ROW& Row = textBuffer._GetFirstRow();
+
+        if (expected)
+        {
+            VERIFY_IS_TRUE(Row.GetCharRow().WasWrapForced());
+        }
+        else
+        {
+            VERIFY_IS_FALSE(Row.GetCharRow().WasWrapForced());
+        }
+    };
+
+    // Construct string for testing
+    const auto width = textBuffer.GetSize().Width();
+    std::wstring chars = L"";
+    for (auto i = 0; i < width; i++)
+    {
+        chars.append(L"a");
+    }
+    const auto lineOfText = std::move(chars);
+
+    Log::Comment(L"Case 1 : Implicit wrap (false)");
+    {
+        TextAttribute expectedAttr(FOREGROUND_RED);
+        OutputCellIterator it(lineOfText, expectedAttr);
+
+        textBuffer.WriteLine(it, { 0, 0 });
+        VerifyWrap(false);
+    }
+
+    Log::Comment(L"Case 2 : wrap = true");
+    {
+        TextAttribute expectedAttr(FOREGROUND_RED);
+        OutputCellIterator it(lineOfText, expectedAttr);
+
+        textBuffer.WriteLine(it, { 0, 0 }, true);
+        VerifyWrap(true);
+    }
+
+    Log::Comment(L"Case 3: wrap = nullopt (remain as TRUE)");
+    {
+        TextAttribute expectedAttr(FOREGROUND_RED);
+        OutputCellIterator it(lineOfText, expectedAttr);
+
+        textBuffer.WriteLine(it, { 0, 0 }, std::nullopt);
+        VerifyWrap(true);
+    }
+
+    Log::Comment(L"Case 4: wrap = false");
+    {
+        TextAttribute expectedAttr(FOREGROUND_RED);
+        OutputCellIterator it(lineOfText, expectedAttr);
+
+        textBuffer.WriteLine(it, { 0, 0 }, false);
+        VerifyWrap(false);
+    }
+
+    Log::Comment(L"Case 5: wrap = nullopt (remain as false)");
+    {
+        TextAttribute expectedAttr(FOREGROUND_RED);
+        OutputCellIterator it(lineOfText, expectedAttr);
+
+        textBuffer.WriteLine(it, { 0, 0 }, std::nullopt);
+        VerifyWrap(false);
+    }
+}
+
 void TextBufferTests::TestDoubleBytePadFlag()
 {
     TextBuffer& textBuffer = GetTbi();