From ff51f535f124167f856917cba400ae7ae9c2ef17 Mon Sep 17 00:00:00 2001 From: Leonard Hecker Date: Fri, 16 Jun 2023 23:56:24 +0200 Subject: [PATCH 1/9] Remove boolean success return values from TextBuffer --- src/buffer/out/Row.cpp | 3 +- src/buffer/out/Row.hpp | 2 +- src/buffer/out/textBuffer.cpp | 317 ++++++++++++++-------------------- src/buffer/out/textBuffer.hpp | 12 +- src/host/_stream.cpp | 58 +++---- src/host/_stream.h | 5 +- src/host/cmdline.cpp | 6 +- src/host/output.cpp | 35 ++-- src/host/output.h | 2 +- src/host/readDataCooked.cpp | 7 +- 10 files changed, 178 insertions(+), 269 deletions(-) diff --git a/src/buffer/out/Row.cpp b/src/buffer/out/Row.cpp index 097fc10dc11..1084b2126c9 100644 --- a/src/buffer/out/Row.cpp +++ b/src/buffer/out/Row.cpp @@ -435,10 +435,9 @@ OutputCellIterator ROW::WriteCells(OutputCellIterator it, const til::CoordType c return it; } -bool ROW::SetAttrToEnd(const til::CoordType columnBegin, const TextAttribute attr) +void ROW::SetAttrToEnd(const til::CoordType columnBegin, const TextAttribute attr) { _attr.replace(_clampedColumnInclusive(columnBegin), _attr.size(), attr); - return true; } void ROW::ReplaceAttributes(const til::CoordType beginIndex, const til::CoordType endIndex, const TextAttribute& newAttr) diff --git a/src/buffer/out/Row.hpp b/src/buffer/out/Row.hpp index 2054086f95a..858c4590099 100644 --- a/src/buffer/out/Row.hpp +++ b/src/buffer/out/Row.hpp @@ -133,7 +133,7 @@ class ROW final void ClearCell(til::CoordType column); OutputCellIterator WriteCells(OutputCellIterator it, til::CoordType columnBegin, std::optional wrap = std::nullopt, std::optional limitRight = std::nullopt); - bool SetAttrToEnd(til::CoordType columnBegin, TextAttribute attr); + void SetAttrToEnd(til::CoordType columnBegin, TextAttribute attr); void ReplaceAttributes(til::CoordType beginIndex, til::CoordType endIndex, const TextAttribute& newAttr); void ReplaceCharacters(til::CoordType columnBegin, til::CoordType width, const std::wstring_view& chars); void ReplaceText(RowWriteState& state); diff --git a/src/buffer/out/textBuffer.cpp b/src/buffer/out/textBuffer.cpp index d4d62facb8c..47b7d71541e 100644 --- a/src/buffer/out/textBuffer.cpp +++ b/src/buffer/out/textBuffer.cpp @@ -406,9 +406,8 @@ bool TextBuffer::_AssertValidDoubleByteSequence(const DbcsAttribute dbcsAttribut //Return Value: // - true if we successfully prepared the buffer and moved the cursor // - false otherwise (out of memory) -bool TextBuffer::_PrepareForDoubleByteSequence(const DbcsAttribute dbcsAttribute) +void TextBuffer::_PrepareForDoubleByteSequence(const DbcsAttribute dbcsAttribute) { - auto fSuccess = true; // Now compensate if we don't have enough space for the upcoming double byte sequence // We only need to compensate for leading bytes if (dbcsAttribute == DbcsAttribute::Leading) @@ -424,10 +423,9 @@ bool TextBuffer::_PrepareForDoubleByteSequence(const DbcsAttribute dbcsAttribute row.SetDoubleBytePadded(true); // then move the cursor forward and onto the next row - fSuccess = IncrementCursor(); + IncrementCursor(); } } - return fSuccess; } void TextBuffer::ConsumeGrapheme(std::wstring_view& chars) noexcept @@ -602,53 +600,37 @@ OutputCellIterator TextBuffer::WriteLine(const OutputCellIterator givenIt, //Return Value: // - true if we successfully inserted the character // - false otherwise (out of memory) -bool TextBuffer::InsertCharacter(const std::wstring_view chars, +void TextBuffer::InsertCharacter(const std::wstring_view chars, const DbcsAttribute dbcsAttribute, const TextAttribute attr) { // Ensure consistent buffer state for double byte characters based on the character type we're about to insert - auto fSuccess = _PrepareForDoubleByteSequence(dbcsAttribute); + _PrepareForDoubleByteSequence(dbcsAttribute); - if (fSuccess) - { - // Get the current cursor position - const auto iRow = GetCursor().GetPosition().y; // row stored as logical position, not array position - const auto iCol = GetCursor().GetPosition().x; // column logical and array positions are equal. + // Get the current cursor position + const auto iRow = GetCursor().GetPosition().y; // row stored as logical position, not array position + const auto iCol = GetCursor().GetPosition().x; // column logical and array positions are equal. - // Get the row associated with the given logical position - auto& Row = GetRowByOffset(iRow); + // Get the row associated with the given logical position + auto& Row = GetRowByOffset(iRow); - // Store character and double byte data - try - { - switch (dbcsAttribute) - { - case DbcsAttribute::Leading: - Row.ReplaceCharacters(iCol, 2, chars); - break; - case DbcsAttribute::Trailing: - Row.ReplaceCharacters(iCol - 1, 2, chars); - break; - default: - Row.ReplaceCharacters(iCol, 1, chars); - break; - } - } - catch (...) - { - LOG_HR(wil::ResultFromCaughtException()); - return false; - } - - // Store color data - fSuccess = Row.SetAttrToEnd(iCol, attr); - if (fSuccess) - { - // Advance the cursor - fSuccess = IncrementCursor(); - } + // Store character and double byte data + switch (dbcsAttribute) + { + case DbcsAttribute::Leading: + Row.ReplaceCharacters(iCol, 2, chars); + break; + case DbcsAttribute::Trailing: + Row.ReplaceCharacters(iCol - 1, 2, chars); + break; + default: + Row.ReplaceCharacters(iCol, 1, chars); + break; } - return fSuccess; + + // Store color data + Row.SetAttrToEnd(iCol, attr); + IncrementCursor(); } //Routine Description: @@ -660,9 +642,9 @@ bool TextBuffer::InsertCharacter(const std::wstring_view chars, //Return Value: // - true if we successfully inserted the character // - false otherwise (out of memory) -bool TextBuffer::InsertCharacter(const wchar_t wch, const DbcsAttribute dbcsAttribute, const TextAttribute attr) +void TextBuffer::InsertCharacter(const wchar_t wch, const DbcsAttribute dbcsAttribute, const TextAttribute attr) { - return InsertCharacter({ &wch, 1 }, dbcsAttribute, attr); + InsertCharacter({ &wch, 1 }, dbcsAttribute, attr); } //Routine Description: @@ -701,7 +683,7 @@ void TextBuffer::_AdjustWrapOnCurrentRow(const bool fSet) //Return Value: // - true if we successfully moved the cursor. // - false otherwise (out of memory) -bool TextBuffer::IncrementCursor() +void TextBuffer::IncrementCursor() { // Cursor position is stored as logical array indices (starts at 0) for the window // Buffer Size is specified as the "length" of the array. It would say 80 for valid values of 0-79. @@ -711,7 +693,6 @@ bool TextBuffer::IncrementCursor() // Move the cursor one position to the right GetCursor().IncrementXPosition(1); - auto fSuccess = true; // If we've passed the final valid column... if (GetCursor().GetPosition().x > iFinalColumnIndex) { @@ -719,9 +700,8 @@ bool TextBuffer::IncrementCursor() _SetWrapOnCurrentRow(); // Then move the cursor to a new line - fSuccess = NewlineCursor(); + NewlineCursor(); } - return fSuccess; } //Routine Description: @@ -730,9 +710,8 @@ bool TextBuffer::IncrementCursor() // - //Return Value: // - true if we successfully moved the cursor. -bool TextBuffer::NewlineCursor() +void TextBuffer::NewlineCursor() { - auto fSuccess = false; const auto iFinalRowIndex = GetSize().BottomInclusive(); // Reset the cursor position to 0 and move down one line @@ -746,13 +725,8 @@ bool TextBuffer::NewlineCursor() GetCursor().SetYPosition(iFinalRowIndex); // Instead increment the circular buffer to move us into the "oldest" row of the backing buffer - fSuccess = IncrementCircularBuffer(); + IncrementCircularBuffer(); } - else - { - fSuccess = true; - } - return fSuccess; } //Routine Description: @@ -761,7 +735,7 @@ bool TextBuffer::NewlineCursor() // - fillAttributes - the attributes with which the recycled row will be initialized. //Return Value: // - true if we successfully incremented the buffer. -bool TextBuffer::IncrementCircularBuffer(const TextAttribute& fillAttributes) +void TextBuffer::IncrementCircularBuffer(const TextAttribute& fillAttributes) { // FirstRow is at any given point in time the array index in the circular buffer that corresponds // to the logical position 0 in the window (cursor coordinates and all other coordinates). @@ -786,7 +760,6 @@ bool TextBuffer::IncrementCircularBuffer(const TextAttribute& fillAttributes) _firstRow = 0; } } - return true; } //Routine Description: @@ -2401,6 +2374,7 @@ HRESULT TextBuffer::Reflow(TextBuffer& oldBuffer, TextBuffer& newBuffer, const std::optional lastCharacterViewport, std::optional> positionInfo) +try { const auto& oldCursor = oldBuffer.GetCursor(); auto& newCursor = newBuffer.GetCursor(); @@ -2417,7 +2391,6 @@ HRESULT TextBuffer::Reflow(TextBuffer& oldBuffer, auto fFoundCursorPos = false; auto foundOldMutable = false; auto foundOldVisible = false; - auto hr = S_OK; // Loop through all the rows of the old buffer and reprint them into the new buffer til::CoordType iOldRow = 0; for (; iOldRow < cOldRowsTotal; iOldRow++) @@ -2473,20 +2446,12 @@ HRESULT TextBuffer::Reflow(TextBuffer& oldBuffer, fFoundCursorPos = true; } - try - { - // TODO: MSFT: 19446208 - this should just use an iterator and the inserter... - const auto glyph = row.GlyphAt(iOldCol); - const auto dbcsAttr = row.DbcsAttrAt(iOldCol); - const auto textAttr = row.GetAttrByColumn(iOldCol); + // TODO: MSFT: 19446208 - this should just use an iterator and the inserter... + const auto glyph = row.GlyphAt(iOldCol); + const auto dbcsAttr = row.DbcsAttrAt(iOldCol); + const auto textAttr = row.GetAttrByColumn(iOldCol); - if (!newBuffer.InsertCharacter(glyph, dbcsAttr, textAttr)) - { - hr = E_OUTOFMEMORY; - break; - } - } - CATCH_RETURN(); + newBuffer.InsertCharacter(glyph, dbcsAttr, textAttr); } // GH#32: Copy the attributes from the rest of the row into this new buffer. @@ -2519,16 +2484,9 @@ HRESULT TextBuffer::Reflow(TextBuffer& oldBuffer, copyAttrCol < cOldColsTotal && newAttrColumn < newWidth; copyAttrCol++, newAttrColumn++) { - try - { - // TODO: MSFT: 19446208 - this should just use an iterator and the inserter... - const auto textAttr = row.GetAttrByColumn(copyAttrCol); - if (!newRow.SetAttrToEnd(newAttrColumn, textAttr)) - { - break; - } - } - CATCH_LOG(); // Not worth dying over. + // TODO: MSFT: 19446208 - this should just use an iterator and the inserter... + const auto textAttr = row.GetAttrByColumn(copyAttrCol); + newRow.SetAttrToEnd(newAttrColumn, textAttr); } // If we found the old row that the caller was interested in, set the @@ -2555,61 +2513,58 @@ HRESULT TextBuffer::Reflow(TextBuffer& oldBuffer, } } - if (SUCCEEDED(hr)) + // If we didn't have a full row to copy, insert a new + // line into the new buffer. + // Only do so if we were not forced to wrap. If we did + // force a word wrap, then the existing line break was + // only because we ran out of space. + if (iRight < cOldColsTotal && !row.WasWrapForced()) { - // If we didn't have a full row to copy, insert a new - // line into the new buffer. - // Only do so if we were not forced to wrap. If we did - // force a word wrap, then the existing line break was - // only because we ran out of space. - if (iRight < cOldColsTotal && !row.WasWrapForced()) + if (!fFoundCursorPos && (iRight == cOldCursorPos.x && iOldRow == cOldCursorPos.y)) { - if (!fFoundCursorPos && (iRight == cOldCursorPos.x && iOldRow == cOldCursorPos.y)) - { - cNewCursorPos = newCursor.GetPosition(); - fFoundCursorPos = true; - } - // Only do this if it's not the final line in the buffer. - // On the final line, we want the cursor to sit - // where it is done printing for the cursor - // adjustment to follow. - if (iOldRow < cOldRowsTotal - 1) - { - hr = newBuffer.NewlineCursor() ? hr : E_OUTOFMEMORY; - } - else + cNewCursorPos = newCursor.GetPosition(); + fFoundCursorPos = true; + } + // Only do this if it's not the final line in the buffer. + // On the final line, we want the cursor to sit + // where it is done printing for the cursor + // adjustment to follow. + if (iOldRow < cOldRowsTotal - 1) + { + newBuffer.NewlineCursor(); + } + else + { + // If we are on the final line of the buffer, we have one more check. + // We got into this code path because we are at the right most column of a row in the old buffer + // that had a hard return (no wrap was forced). + // However, as we're inserting, the old row might have just barely fit into the new buffer and + // caused a new soft return (wrap was forced) putting the cursor at x=0 on the line just below. + // We need to preserve the memory of the hard return at this point by inserting one additional + // hard newline, otherwise we've lost that information. + // We only do this when the cursor has just barely poured over onto the next line so the hard return + // isn't covered by the soft one. + // e.g. + // The old line was: + // |aaaaaaaaaaaaaaaaaaa | with no wrap which means there was a newline after that final a. + // The cursor was here ^ + // And the new line will be: + // |aaaaaaaaaaaaaaaaaaa| and show a wrap at the end + // | | + // ^ and the cursor is now there. + // If we leave it like this, we've lost the newline information. + // So we insert one more newline so a continued reflow of this buffer by resizing larger will + // continue to look as the original output intended with the newline data. + // After this fix, it looks like this: + // |aaaaaaaaaaaaaaaaaaa| no wrap at the end (preserved hard newline) + // | | + // ^ and the cursor is now here. + const auto coordNewCursor = newCursor.GetPosition(); + if (coordNewCursor.x == 0 && coordNewCursor.y > 0) { - // If we are on the final line of the buffer, we have one more check. - // We got into this code path because we are at the right most column of a row in the old buffer - // that had a hard return (no wrap was forced). - // However, as we're inserting, the old row might have just barely fit into the new buffer and - // caused a new soft return (wrap was forced) putting the cursor at x=0 on the line just below. - // We need to preserve the memory of the hard return at this point by inserting one additional - // hard newline, otherwise we've lost that information. - // We only do this when the cursor has just barely poured over onto the next line so the hard return - // isn't covered by the soft one. - // e.g. - // The old line was: - // |aaaaaaaaaaaaaaaaaaa | with no wrap which means there was a newline after that final a. - // The cursor was here ^ - // And the new line will be: - // |aaaaaaaaaaaaaaaaaaa| and show a wrap at the end - // | | - // ^ and the cursor is now there. - // If we leave it like this, we've lost the newline information. - // So we insert one more newline so a continued reflow of this buffer by resizing larger will - // continue to look as the original output intended with the newline data. - // After this fix, it looks like this: - // |aaaaaaaaaaaaaaaaaaa| no wrap at the end (preserved hard newline) - // | | - // ^ and the cursor is now here. - const auto coordNewCursor = newCursor.GetPosition(); - if (coordNewCursor.x == 0 && coordNewCursor.y > 0) + if (newBuffer.GetRowByOffset(coordNewCursor.y - 1).WasWrapForced()) { - if (newBuffer.GetRowByOffset(coordNewCursor.y - 1).WasWrapForced()) - { - hr = newBuffer.NewlineCursor() ? hr : E_OUTOFMEMORY; - } + newBuffer.NewlineCursor(); } } } @@ -2642,77 +2597,61 @@ HRESULT TextBuffer::Reflow(TextBuffer& oldBuffer, newRowY++; } - if (SUCCEEDED(hr)) + // Finish copying remaining parameters from the old text buffer to the new one + newBuffer.CopyProperties(oldBuffer); + newBuffer.CopyHyperlinkMaps(oldBuffer); + newBuffer.CopyPatterns(oldBuffer); + + // If we found where to put the cursor while placing characters into the buffer, + // just put the cursor there. Otherwise we have to advance manually. + if (fFoundCursorPos) { - // Finish copying remaining parameters from the old text buffer to the new one - newBuffer.CopyProperties(oldBuffer); - newBuffer.CopyHyperlinkMaps(oldBuffer); - newBuffer.CopyPatterns(oldBuffer); + newCursor.SetPosition(cNewCursorPos); + } + else + { + // Advance the cursor to the same offset as before + // get the number of newlines and spaces between the old end of text and the old cursor, + // then advance that many newlines and chars + auto iNewlines = cOldCursorPos.y - cOldLastChar.y; + const auto iIncrements = cOldCursorPos.x - cOldLastChar.x; + const auto cNewLastChar = newBuffer.GetLastNonSpaceCharacter(); - // If we found where to put the cursor while placing characters into the buffer, - // just put the cursor there. Otherwise we have to advance manually. - if (fFoundCursorPos) + // If the last row of the new buffer wrapped, there's going to be one less newline needed, + // because the cursor is already on the next line + if (newBuffer.GetRowByOffset(cNewLastChar.y).WasWrapForced()) { - newCursor.SetPosition(cNewCursorPos); + iNewlines = std::max(iNewlines - 1, 0); } else { - // Advance the cursor to the same offset as before - // get the number of newlines and spaces between the old end of text and the old cursor, - // then advance that many newlines and chars - auto iNewlines = cOldCursorPos.y - cOldLastChar.y; - const auto iIncrements = cOldCursorPos.x - cOldLastChar.x; - const auto cNewLastChar = newBuffer.GetLastNonSpaceCharacter(); - - // If the last row of the new buffer wrapped, there's going to be one less newline needed, - // because the cursor is already on the next line - if (newBuffer.GetRowByOffset(cNewLastChar.y).WasWrapForced()) + // if this buffer didn't wrap, but the old one DID, then the d(columns) of the + // old buffer will be one more than in this buffer, so new need one LESS. + if (oldBuffer.GetRowByOffset(cOldLastChar.y).WasWrapForced()) { iNewlines = std::max(iNewlines - 1, 0); } - else - { - // if this buffer didn't wrap, but the old one DID, then the d(columns) of the - // old buffer will be one more than in this buffer, so new need one LESS. - if (oldBuffer.GetRowByOffset(cOldLastChar.y).WasWrapForced()) - { - iNewlines = std::max(iNewlines - 1, 0); - } - } + } - for (auto r = 0; r < iNewlines; r++) - { - if (!newBuffer.NewlineCursor()) - { - hr = E_OUTOFMEMORY; - break; - } - } - if (SUCCEEDED(hr)) - { - for (auto c = 0; c < iIncrements - 1; c++) - { - if (!newBuffer.IncrementCursor()) - { - hr = E_OUTOFMEMORY; - break; - } - } - } + for (auto r = 0; r < iNewlines; r++) + { + newBuffer.NewlineCursor(); + } + for (auto c = 0; c < iIncrements - 1; c++) + { + newBuffer.IncrementCursor(); } } - if (SUCCEEDED(hr)) - { - // Save old cursor size before we delete it - const auto ulSize = oldCursor.GetSize(); + // Save old cursor size before we delete it + const auto ulSize = oldCursor.GetSize(); - // Set size back to real size as it will be taking over the rendering duties. - newCursor.SetSize(ulSize); - } + // Set size back to real size as it will be taking over the rendering duties. + newCursor.SetSize(ulSize); - return hr; + return S_OK; } +CATCH_RETURN() // Method Description: // - Adds or updates a hyperlink in our hyperlink table diff --git a/src/buffer/out/textBuffer.hpp b/src/buffer/out/textBuffer.hpp index 2843a2407e9..ff7d274da5e 100644 --- a/src/buffer/out/textBuffer.hpp +++ b/src/buffer/out/textBuffer.hpp @@ -112,13 +112,13 @@ class TextBuffer final const std::optional setWrap = std::nullopt, const std::optional limitRight = std::nullopt); - bool InsertCharacter(const wchar_t wch, const DbcsAttribute dbcsAttribute, const TextAttribute attr); - bool InsertCharacter(const std::wstring_view chars, const DbcsAttribute dbcsAttribute, const TextAttribute attr); - bool IncrementCursor(); - bool NewlineCursor(); + void InsertCharacter(const wchar_t wch, const DbcsAttribute dbcsAttribute, const TextAttribute attr); + void InsertCharacter(const std::wstring_view chars, const DbcsAttribute dbcsAttribute, const TextAttribute attr); + void IncrementCursor(); + void NewlineCursor(); // Scroll needs access to this to quickly rotate around the buffer. - bool IncrementCircularBuffer(const TextAttribute& fillAttributes = {}); + void IncrementCircularBuffer(const TextAttribute& fillAttributes = {}); til::point GetLastNonSpaceCharacter(std::optional viewOptional = std::nullopt) const; @@ -240,7 +240,7 @@ class TextBuffer final void _SetWrapOnCurrentRow(); void _AdjustWrapOnCurrentRow(const bool fSet); // Assist with maintaining proper buffer state for Double Byte character sequences - bool _PrepareForDoubleByteSequence(const DbcsAttribute dbcsAttribute); + void _PrepareForDoubleByteSequence(const DbcsAttribute dbcsAttribute); bool _AssertValidDoubleByteSequence(const DbcsAttribute dbcsAttribute); void _ExpandTextRow(til::inclusive_rect& selectionRow) const; DelimiterClass _GetDelimiterClassAt(const til::point pos, const std::wstring_view wordDelimiters) const; diff --git a/src/host/_stream.cpp b/src/host/_stream.cpp index d6f1e87e80e..5bd3b675e44 100644 --- a/src/host/_stream.cpp +++ b/src/host/_stream.cpp @@ -39,10 +39,7 @@ using Microsoft::Console::VirtualTerminal::StateMachine; // - coordCursor - New location of cursor. // - fKeepCursorVisible - TRUE if changing window origin desirable when hit right edge // Return Value: -[[nodiscard]] NTSTATUS AdjustCursorPosition(SCREEN_INFORMATION& screenInfo, - _In_ til::point coordCursor, - const BOOL fKeepCursorVisible, - _Inout_opt_ til::CoordType* psScrollY) +void AdjustCursorPosition(SCREEN_INFORMATION& screenInfo, _In_ til::point coordCursor, const BOOL fKeepCursorVisible, _Inout_opt_ til::CoordType* psScrollY) { const auto bufferSize = screenInfo.GetBufferSize().Dimensions(); if (coordCursor.x < 0) @@ -71,16 +68,10 @@ using Microsoft::Console::VirtualTerminal::StateMachine; } } - auto Status = STATUS_SUCCESS; - if (coordCursor.y >= bufferSize.height) { // At the end of the buffer. Scroll contents of screen buffer so new position is visible. - FAIL_FAST_IF(!(coordCursor.y == bufferSize.height)); - if (!StreamScrollRegion(screenInfo)) - { - Status = STATUS_NO_MEMORY; - } + StreamScrollRegion(screenInfo); if (nullptr != psScrollY) { @@ -90,28 +81,21 @@ using Microsoft::Console::VirtualTerminal::StateMachine; } const auto cursorMovedPastViewport = coordCursor.y > screenInfo.GetViewport().BottomInclusive(); - if (SUCCEEDED_NTSTATUS(Status)) + + // if at right or bottom edge of window, scroll right or down one char. + if (cursorMovedPastViewport) { - // if at right or bottom edge of window, scroll right or down one char. - if (cursorMovedPastViewport) - { - til::point WindowOrigin; - WindowOrigin.x = 0; - WindowOrigin.y = coordCursor.y - screenInfo.GetViewport().BottomInclusive(); - Status = screenInfo.SetViewportOrigin(false, WindowOrigin, true); - } + til::point WindowOrigin; + WindowOrigin.x = 0; + WindowOrigin.y = coordCursor.y - screenInfo.GetViewport().BottomInclusive(); + LOG_IF_FAILED(screenInfo.SetViewportOrigin(false, WindowOrigin, true)); } - if (SUCCEEDED_NTSTATUS(Status)) + if (fKeepCursorVisible) { - if (fKeepCursorVisible) - { - screenInfo.MakeCursorVisible(coordCursor); - } - Status = screenInfo.SetCursorPosition(coordCursor, !!fKeepCursorVisible); + screenInfo.MakeCursorVisible(coordCursor); } - - return Status; + LOG_IF_FAILED(screenInfo.SetCursorPosition(coordCursor, !!fKeepCursorVisible)); } // Routine Description: @@ -180,7 +164,7 @@ using Microsoft::Console::VirtualTerminal::StateMachine; CursorPosition.x = 0; CursorPosition.y++; - Status = AdjustCursorPosition(screenInfo, CursorPosition, WI_IsFlagSet(dwFlags, WC_KEEP_CURSOR_VISIBLE), psScrollY); + AdjustCursorPosition(screenInfo, CursorPosition, WI_IsFlagSet(dwFlags, WC_KEEP_CURSOR_VISIBLE), psScrollY); CursorPosition = cursor.GetPosition(); } @@ -372,7 +356,7 @@ using Microsoft::Console::VirtualTerminal::StateMachine; // WCL-NOTE: wrong place (typically inside another character). CursorPosition.x = XPosition; - Status = AdjustCursorPosition(screenInfo, CursorPosition, WI_IsFlagSet(dwFlags, WC_KEEP_CURSOR_VISIBLE), psScrollY); + AdjustCursorPosition(screenInfo, CursorPosition, WI_IsFlagSet(dwFlags, WC_KEEP_CURSOR_VISIBLE), psScrollY); // WCL-NOTE: If we have processed the entire input string during our "fast one-line print" handler, // WCL-NOTE: we are done as there is nothing more to do. Neat! @@ -501,7 +485,7 @@ using Microsoft::Console::VirtualTerminal::StateMachine; CursorPosition.x -= 1; TempNumSpaces -= 1; - Status = AdjustCursorPosition(screenInfo, CursorPosition, dwFlags & WC_KEEP_CURSOR_VISIBLE, psScrollY); + AdjustCursorPosition(screenInfo, CursorPosition, dwFlags & WC_KEEP_CURSOR_VISIBLE, psScrollY); if (dwFlags & WC_DESTRUCTIVE_BACKSPACE) { try @@ -523,7 +507,7 @@ using Microsoft::Console::VirtualTerminal::StateMachine; CursorPosition.x = 0; OutputDebugStringA(("CONSRV: Ignoring backspace to previous line\n")); } - Status = AdjustCursorPosition(screenInfo, CursorPosition, (dwFlags & WC_KEEP_CURSOR_VISIBLE) != 0, psScrollY); + AdjustCursorPosition(screenInfo, CursorPosition, (dwFlags & WC_KEEP_CURSOR_VISIBLE) != 0, psScrollY); if (dwFlags & WC_DESTRUCTIVE_BACKSPACE) { try @@ -548,7 +532,7 @@ using Microsoft::Console::VirtualTerminal::StateMachine; // on the prev row if it was set textBuffer.GetRowByOffset(CursorPosition.y).SetWrapForced(false); - Status = AdjustCursorPosition(screenInfo, CursorPosition, dwFlags & WC_KEEP_CURSOR_VISIBLE, psScrollY); + AdjustCursorPosition(screenInfo, CursorPosition, dwFlags & WC_KEEP_CURSOR_VISIBLE, psScrollY); } } // Notify accessibility to read the backspaced character. @@ -598,7 +582,7 @@ using Microsoft::Console::VirtualTerminal::StateMachine; } CATCH_LOG(); - Status = AdjustCursorPosition(screenInfo, CursorPosition, (dwFlags & WC_KEEP_CURSOR_VISIBLE) != 0, psScrollY); + AdjustCursorPosition(screenInfo, CursorPosition, (dwFlags & WC_KEEP_CURSOR_VISIBLE) != 0, psScrollY); break; } case UNICODE_CARRIAGERETURN: @@ -609,7 +593,7 @@ using Microsoft::Console::VirtualTerminal::StateMachine; pwchBuffer++; CursorPosition.x = 0; CursorPosition.y = cursor.GetPosition().y; - Status = AdjustCursorPosition(screenInfo, CursorPosition, (dwFlags & WC_KEEP_CURSOR_VISIBLE) != 0, psScrollY); + AdjustCursorPosition(screenInfo, CursorPosition, (dwFlags & WC_KEEP_CURSOR_VISIBLE) != 0, psScrollY); break; } case UNICODE_LINEFEED: @@ -632,7 +616,7 @@ using Microsoft::Console::VirtualTerminal::StateMachine; textBuffer.GetRowByOffset(cursor.GetPosition().y).SetWrapForced(false); } - Status = AdjustCursorPosition(screenInfo, CursorPosition, (dwFlags & WC_KEEP_CURSOR_VISIBLE) != 0, psScrollY); + AdjustCursorPosition(screenInfo, CursorPosition, (dwFlags & WC_KEEP_CURSOR_VISIBLE) != 0, psScrollY); break; } default: @@ -678,7 +662,7 @@ using Microsoft::Console::VirtualTerminal::StateMachine; // is too wide to fit on the current line). Row.SetDoubleBytePadded(true); - Status = AdjustCursorPosition(screenInfo, CursorPosition, dwFlags & WC_KEEP_CURSOR_VISIBLE, psScrollY); + AdjustCursorPosition(screenInfo, CursorPosition, dwFlags & WC_KEEP_CURSOR_VISIBLE, psScrollY); continue; } break; diff --git a/src/host/_stream.h b/src/host/_stream.h index 149482f1ff1..d12e6622e72 100644 --- a/src/host/_stream.h +++ b/src/host/_stream.h @@ -35,10 +35,7 @@ Routine Description: Return Value: --*/ -[[nodiscard]] NTSTATUS AdjustCursorPosition(SCREEN_INFORMATION& screenInfo, - _In_ til::point coordCursor, - const BOOL fKeepCursorVisible, - _Inout_opt_ til::CoordType* psScrollY); +void AdjustCursorPosition(SCREEN_INFORMATION& screenInfo, _In_ til::point coordCursor, const BOOL fKeepCursorVisible, _Inout_opt_ til::CoordType* psScrollY); /*++ Routine Description: diff --git a/src/host/cmdline.cpp b/src/host/cmdline.cpp index 1675962144b..094ba9ba2dc 100644 --- a/src/host/cmdline.cpp +++ b/src/host/cmdline.cpp @@ -268,8 +268,7 @@ void RedrawCommandLine(COOKED_READ_DATA& cookedReadData) { CursorPosition.x++; } - Status = AdjustCursorPosition(cookedReadData.ScreenInfo(), CursorPosition, TRUE, nullptr); - FAIL_FAST_IF_NTSTATUS_FAILED(Status); + AdjustCursorPosition(cookedReadData.ScreenInfo(), CursorPosition, TRUE, nullptr); } } @@ -1298,8 +1297,7 @@ til::point CommandLine::DeleteFromRightOfCursor(COOKED_READ_DATA& cookedReadData if (UpdateCursorPosition && cookedReadData.IsEchoInput()) { - Status = AdjustCursorPosition(cookedReadData.ScreenInfo(), cursorPosition, true, nullptr); - FAIL_FAST_IF_NTSTATUS_FAILED(Status); + AdjustCursorPosition(cookedReadData.ScreenInfo(), cursorPosition, true, nullptr); } return STATUS_SUCCESS; diff --git a/src/host/output.cpp b/src/host/output.cpp index fbdb9babf84..75f522de314 100644 --- a/src/host/output.cpp +++ b/src/host/output.cpp @@ -299,33 +299,30 @@ static void _ScrollScreen(SCREEN_INFORMATION& screenInfo, const Viewport& source // - screenInfo - reference to screen buffer info. // Return Value: // - true if we succeeded in scrolling the buffer, otherwise false (if we're out of memory) -bool StreamScrollRegion(SCREEN_INFORMATION& screenInfo) +void StreamScrollRegion(SCREEN_INFORMATION& screenInfo) { // Rotate the circular buffer around and wipe out the previous final line. auto& buffer = screenInfo.GetTextBuffer(); - auto fSuccess = buffer.IncrementCircularBuffer(buffer.GetCurrentAttributes()); - if (fSuccess) + buffer.IncrementCircularBuffer(buffer.GetCurrentAttributes()); + + // Trigger a graphical update if we're active. + if (screenInfo.IsActiveScreenBuffer()) { - // Trigger a graphical update if we're active. - if (screenInfo.IsActiveScreenBuffer()) - { - til::point coordDelta; - coordDelta.y = -1; + til::point coordDelta; + coordDelta.y = -1; - auto pNotifier = ServiceLocator::LocateAccessibilityNotifier(); - if (pNotifier) - { - // Notify accessibility that a scroll has occurred. - pNotifier->NotifyConsoleUpdateScrollEvent(coordDelta.x, coordDelta.y); - } + auto pNotifier = ServiceLocator::LocateAccessibilityNotifier(); + if (pNotifier) + { + // Notify accessibility that a scroll has occurred. + pNotifier->NotifyConsoleUpdateScrollEvent(coordDelta.x, coordDelta.y); + } - if (ServiceLocator::LocateGlobals().pRender != nullptr) - { - ServiceLocator::LocateGlobals().pRender->TriggerScroll(&coordDelta); - } + if (ServiceLocator::LocateGlobals().pRender != nullptr) + { + ServiceLocator::LocateGlobals().pRender->TriggerScroll(&coordDelta); } } - return fSuccess; } // Routine Description: diff --git a/src/host/output.h b/src/host/output.h index 849ba505596..fff6c001c25 100644 --- a/src/host/output.h +++ b/src/host/output.h @@ -47,7 +47,7 @@ void ScrollRegion(SCREEN_INFORMATION& screenInfo, VOID SetConsoleWindowOwner(const HWND hwnd, _Inout_opt_ ConsoleProcessHandle* pProcessData); -bool StreamScrollRegion(SCREEN_INFORMATION& screenInfo); +void StreamScrollRegion(SCREEN_INFORMATION& screenInfo); // For handling process handle state, not the window state itself. void CloseConsoleProcessState(); diff --git a/src/host/readDataCooked.cpp b/src/host/readDataCooked.cpp index 6ea80796f7e..eac6e5fb074 100644 --- a/src/host/readDataCooked.cpp +++ b/src/host/readDataCooked.cpp @@ -757,12 +757,7 @@ bool COOKED_READ_DATA::ProcessInput(const wchar_t wchOrig, // adjust cursor position for WriteChars _originalCursorPosition.y += ScrollY; CursorPosition.y += ScrollY; - status = AdjustCursorPosition(_screenInfo, CursorPosition, TRUE, nullptr); - if (FAILED_NTSTATUS(status)) - { - _bytesRead = 0; - return true; - } + AdjustCursorPosition(_screenInfo, CursorPosition, TRUE, nullptr); } } } From 1ea84d3eac4603b0c7c667e8bbce663fc38add53 Mon Sep 17 00:00:00 2001 From: Leonard Hecker Date: Sat, 17 Jun 2023 00:03:28 +0200 Subject: [PATCH 2/9] Rewrite and modernize WriteCharsLegacy --- src/buffer/out/Row.cpp | 11 + src/buffer/out/Row.hpp | 1 + .../ConptyRoundtripTests.cpp | 4 +- src/host/CommandNumberPopup.cpp | 4 +- src/host/_stream.cpp | 740 ++++++------------ src/host/_stream.h | 3 +- src/host/cmdline.cpp | 24 +- src/host/cmdline.h | 16 +- src/host/ft_fuzzer/fuzzmain.cpp | 2 +- src/host/readDataCooked.cpp | 10 +- src/host/ut_host/ScreenBufferTests.cpp | 2 +- 11 files changed, 273 insertions(+), 544 deletions(-) diff --git a/src/buffer/out/Row.cpp b/src/buffer/out/Row.cpp index 1084b2126c9..b497d35fc7d 100644 --- a/src/buffer/out/Row.cpp +++ b/src/buffer/out/Row.cpp @@ -872,6 +872,17 @@ std::wstring_view ROW::GetText() const noexcept return { _chars.data(), _charSize() }; } +std::wstring_view ROW::GetText(til::CoordType columnBegin, til::CoordType columnEnd) const noexcept +{ + const til::CoordType columns = _columnCount; + const auto colBeg = std::max(0, std::min(columns, columnBegin)); + const auto colEnd = std::max(colBeg, std::min(columns, columnEnd)); + const size_t chBeg = _uncheckedCharOffset(gsl::narrow_cast(colBeg)); + const size_t chEnd = _uncheckedCharOffset(gsl::narrow_cast(colEnd)); +#pragma warning(suppress : 26481) // Don't use pointer arithmetic. Use span instead (bounds.1). + return { _chars.data() + chBeg, chEnd - chBeg }; +} + DelimiterClass ROW::DelimiterClassAt(til::CoordType column, const std::wstring_view& wordDelimiters) const noexcept { const auto col = _clampedColumn(column); diff --git a/src/buffer/out/Row.hpp b/src/buffer/out/Row.hpp index 858c4590099..8310898b1ca 100644 --- a/src/buffer/out/Row.hpp +++ b/src/buffer/out/Row.hpp @@ -151,6 +151,7 @@ class ROW final std::wstring_view GlyphAt(til::CoordType column) const noexcept; DbcsAttribute DbcsAttrAt(til::CoordType column) const noexcept; std::wstring_view GetText() const noexcept; + std::wstring_view GetText(til::CoordType columnBegin, til::CoordType columnEnd) const noexcept; DelimiterClass DelimiterClassAt(til::CoordType column, const std::wstring_view& wordDelimiters) const noexcept; auto AttrBegin() const noexcept { return _attr.begin(); } diff --git a/src/cascadia/UnitTests_TerminalCore/ConptyRoundtripTests.cpp b/src/cascadia/UnitTests_TerminalCore/ConptyRoundtripTests.cpp index cd9ac092898..9f267746511 100644 --- a/src/cascadia/UnitTests_TerminalCore/ConptyRoundtripTests.cpp +++ b/src/cascadia/UnitTests_TerminalCore/ConptyRoundtripTests.cpp @@ -21,7 +21,7 @@ #include "../host/readDataCooked.hpp" #include "../host/output.h" #include "../host/_stream.h" // For WriteCharsLegacy -#include "../host/cmdline.h" // For WC_LIMIT_BACKSPACE +#include "../host/cmdline.h" // For WC_INTERACTIVE #include "test/CommonState.hpp" #include "../cascadia/TerminalCore/Terminal.hpp" @@ -3422,7 +3422,7 @@ void ConptyRoundtripTests::WrapNewLineAtBottomLikeMSYS() } else if (writingMethod == PrintWithWriteCharsLegacy) { - doWriteCharsLegacy(si, str, WC_LIMIT_BACKSPACE); + doWriteCharsLegacy(si, str, WC_INTERACTIVE); } }; diff --git a/src/host/CommandNumberPopup.cpp b/src/host/CommandNumberPopup.cpp index bf77a39b37f..4ccce4d6d75 100644 --- a/src/host/CommandNumberPopup.cpp +++ b/src/host/CommandNumberPopup.cpp @@ -42,7 +42,7 @@ void CommandNumberPopup::_handleNumber(COOKED_READ_DATA& cookedReadData, const w &CharsToWrite, &NumSpaces, cookedReadData.OriginalCursorPosition().x, - WC_DESTRUCTIVE_BACKSPACE | WC_KEEP_CURSOR_VISIBLE | WC_PRINTABLE_CONTROL_CHARS, + WC_INTERACTIVE | WC_KEEP_CURSOR_VISIBLE, nullptr)); cookedReadData.ScreenInfo().SetAttributes(realAttributes); try @@ -73,7 +73,7 @@ void CommandNumberPopup::_handleBackspace(COOKED_READ_DATA& cookedReadData) noex &CharsToWrite, &NumSpaces, cookedReadData.OriginalCursorPosition().x, - WC_DESTRUCTIVE_BACKSPACE | WC_KEEP_CURSOR_VISIBLE | WC_PRINTABLE_CONTROL_CHARS, + WC_INTERACTIVE | WC_KEEP_CURSOR_VISIBLE, nullptr)); cookedReadData.ScreenInfo().SetAttributes(realAttributes); _pop(); diff --git a/src/host/_stream.cpp b/src/host/_stream.cpp index 5bd3b675e44..bb58bf3e4e2 100644 --- a/src/host/_stream.cpp +++ b/src/host/_stream.cpp @@ -98,6 +98,42 @@ void AdjustCursorPosition(SCREEN_INFORMATION& screenInfo, _In_ til::point coordC LOG_IF_FAILED(screenInfo.SetCursorPosition(coordCursor, !!fKeepCursorVisible)); } +// As the name implies, this writes text without processing its control characters. +static size_t _writeCharsLegacyUnprocessed(SCREEN_INFORMATION& screenInfo, const DWORD dwFlags, til::CoordType* const psScrollY, const std::wstring_view& text) +{ + const auto fKeepCursorVisible = WI_IsFlagSet(dwFlags, WC_KEEP_CURSOR_VISIBLE); + const auto fWrapAtEOL = WI_IsFlagSet(screenInfo.OutputMode, ENABLE_WRAP_AT_EOL_OUTPUT); + const auto hasAccessibilityEventing = screenInfo.HasAccessibilityEventing(); + auto& textBuffer = screenInfo.GetTextBuffer(); + auto CursorPosition = textBuffer.GetCursor().GetPosition(); + size_t TempNumSpaces = 0; + + RowWriteState state{ + .text = text, + .columnLimit = til::CoordTypeMax, + }; + + while (!state.text.empty()) + { + state.columnBegin = CursorPosition.x; + textBuffer.WriteLine(CursorPosition.y, fWrapAtEOL, textBuffer.GetCurrentAttributes(), state); + CursorPosition.x = state.columnEnd; + + const auto spaces = gsl::narrow_cast(state.columnEnd - state.columnBegin); + TempNumSpaces += spaces; + + if (hasAccessibilityEventing && spaces != 0) + { + screenInfo.NotifyAccessibilityEventing(state.columnBegin, CursorPosition.y, state.columnEnd - 1, CursorPosition.y); + } + + AdjustCursorPosition(screenInfo, CursorPosition, fKeepCursorVisible, psScrollY); + CursorPosition = textBuffer.GetCursor().GetPosition(); + } + + return TempNumSpaces; +} + // Routine Description: // - This routine writes a string to the screen, processing any embedded // unicode characters. The string is also copied to the input buffer, if @@ -111,9 +147,8 @@ void AdjustCursorPosition(SCREEN_INFORMATION& screenInfo, _In_ til::point coordC // - pcb - On input, number of bytes to write. On output, number of bytes written. // - pcSpaces - On output, the number of spaces consumed by the written characters. // - dwFlags - -// WC_DESTRUCTIVE_BACKSPACE backspace overwrites characters. +// WC_INTERACTIVE backspace overwrites characters, control characters are expanded (as in, to "^X") // WC_KEEP_CURSOR_VISIBLE change window origin desirable when hit rt. edge -// WC_PRINTABLE_CONTROL_CHARS if control characters should be expanded (as in, to "^X") // Return Value: // Note: // - This routine does not process tabs and backspace properly. That code will be implemented as part of the line editing services. @@ -126,565 +161,270 @@ void AdjustCursorPosition(SCREEN_INFORMATION& screenInfo, _In_ til::point coordC const til::CoordType sOriginalXPosition, const DWORD dwFlags, _Inout_opt_ til::CoordType* const psScrollY) +try { - const auto& gci = ServiceLocator::LocateGlobals().getConsoleInformation(); + static constexpr wchar_t tabSpaces[8]{ L' ', L' ', L' ', L' ', L' ', L' ', L' ', L' ' }; + + UNREFERENCED_PARAMETER(sOriginalXPosition); + UNREFERENCED_PARAMETER(pwchBuffer); + UNREFERENCED_PARAMETER(pwchBufferBackupLimit); + auto& textBuffer = screenInfo.GetTextBuffer(); auto& cursor = textBuffer.GetCursor(); - auto CursorPosition = cursor.GetPosition(); - auto Status = STATUS_SUCCESS; - til::CoordType XPosition; - size_t TempNumSpaces = 0; + const auto width = textBuffer.GetSize().Width(); + const auto Attributes = textBuffer.GetCurrentAttributes(); + const auto fKeepCursorVisible = WI_IsFlagSet(dwFlags, WC_KEEP_CURSOR_VISIBLE); const auto fUnprocessed = WI_IsFlagClear(screenInfo.OutputMode, ENABLE_PROCESSED_OUTPUT); const auto fWrapAtEOL = WI_IsFlagSet(screenInfo.OutputMode, ENABLE_WRAP_AT_EOL_OUTPUT); + auto it = pwchRealUnicode; + const auto end = it + *pcb / sizeof(wchar_t); + size_t TempNumSpaces = 0; - // Must not adjust cursor here. It has to stay on for many write scenarios. Consumers should call for the - // cursor to be turned off if they want that. - - const auto Attributes = screenInfo.GetAttributes(); - const auto BufferSize = *pcb; - *pcb = 0; - - auto lpString = pwchRealUnicode; - - const auto coordScreenBufferSize = screenInfo.GetBufferSize().Dimensions(); - - static constexpr til::CoordType LOCAL_BUFFER_SIZE = 1024; - WCHAR LocalBuffer[LOCAL_BUFFER_SIZE]; - - while (*pcb < BufferSize) + if (cursor.IsDelayedEOLWrap() && fWrapAtEOL) { - // correct for delayed EOL - if (cursor.IsDelayedEOLWrap() && fWrapAtEOL) + auto CursorPosition = cursor.GetPosition(); + const auto delayedCursorPosition = cursor.GetDelayedAtPosition(); + cursor.ResetDelayEOLWrap(); + if (delayedCursorPosition == CursorPosition) { - const auto coordDelayedAt = cursor.GetDelayedAtPosition(); - cursor.ResetDelayEOLWrap(); - // Only act on a delayed EOL if we didn't move the cursor to a different position from where the EOL was marked. - if (coordDelayedAt.x == CursorPosition.x && coordDelayedAt.y == CursorPosition.y) - { - CursorPosition.x = 0; - CursorPosition.y++; + CursorPosition.x = 0; + CursorPosition.y++; + AdjustCursorPosition(screenInfo, CursorPosition, fKeepCursorVisible, psScrollY); + } + } - AdjustCursorPosition(screenInfo, CursorPosition, WI_IsFlagSet(dwFlags, WC_KEEP_CURSOR_VISIBLE), psScrollY); + if (fUnprocessed) + { + TempNumSpaces += _writeCharsLegacyUnprocessed(screenInfo, dwFlags, psScrollY, { it, end }); + it = end; + } - CursorPosition = cursor.GetPosition(); - } + while (it != end) + { + const auto nextControlChar = std::find_if(it, end, [](const auto& wch) { return !IS_GLYPH_CHAR(wch); }); + if (nextControlChar != it) + { + TempNumSpaces += _writeCharsLegacyUnprocessed(screenInfo, dwFlags, psScrollY, { it, nextControlChar }); + it = nextControlChar; } - // As an optimization, collect characters in buffer and print out all at once. - XPosition = cursor.GetPosition().x; - til::CoordType i = 0; - auto LocalBufPtr = LocalBuffer; - while (*pcb < BufferSize && i < LOCAL_BUFFER_SIZE && XPosition < coordScreenBufferSize.width) + for (; it != end && !IS_GLYPH_CHAR(*it); ++it) { -#pragma prefast(suppress : 26019, "Buffer is taken in multiples of 2. Validation is ok.") - const auto Char = *lpString; - // WCL-NOTE: We believe RealUnicodeChar to be identical to Char, because we believe pwchRealUnicode - // WCL-NOTE: to be identical to lpString. They are incremented in lockstep, never separately, and lpString - // WCL-NOTE: is initialized from pwchRealUnicode. - const auto RealUnicodeChar = *pwchRealUnicode; - if (IS_GLYPH_CHAR(RealUnicodeChar) || fUnprocessed) + switch (*it) { - // WCL-NOTE: This operates on a single code unit instead of a whole codepoint. It will mis-measure surrogate pairs. - if (IsGlyphFullWidth(Char)) + case UNICODE_NULL: + if (WI_IsFlagSet(dwFlags, WC_INTERACTIVE)) { - if (i < (LOCAL_BUFFER_SIZE - 1) && XPosition < (coordScreenBufferSize.width - 1)) - { - *LocalBufPtr++ = Char; - - // cursor adjusted by 2 because the char is double width - XPosition += 2; - i += 1; - pwchBuffer++; - } - else - { - goto EndWhile; - } + break; } - else + TempNumSpaces += _writeCharsLegacyUnprocessed(screenInfo, dwFlags, psScrollY, { &tabSpaces[0], 1 }); + continue; + case UNICODE_BELL: + if (WI_IsFlagSet(dwFlags, WC_INTERACTIVE)) { - *LocalBufPtr = Char; - LocalBufPtr++; - XPosition++; - i++; - pwchBuffer++; + break; } - } - else + std::ignore = screenInfo.SendNotifyBeep(); + continue; + case UNICODE_BACKSPACE: { - FAIL_FAST_IF(!(WI_IsFlagSet(screenInfo.OutputMode, ENABLE_PROCESSED_OUTPUT))); - switch (RealUnicodeChar) - { - case UNICODE_BELL: - if (dwFlags & WC_PRINTABLE_CONTROL_CHARS) - { - goto CtrlChar; - } - else - { - screenInfo.SendNotifyBeep(); - } - break; - case UNICODE_BACKSPACE: + auto CursorPosition = cursor.GetPosition(); - // automatically go to EndWhile. this is because - // backspace is not destructive, so "aBkSp" prints - // a with the cursor on the "a". we could achieve - // this behavior staying in this loop and figuring out - // the string that needs to be printed, but it would - // be expensive and it's the exceptional case. - - goto EndWhile; - break; - case UNICODE_TAB: + if (WI_IsFlagClear(dwFlags, WC_INTERACTIVE)) { - const auto TabSize = NUMBER_OF_SPACES_IN_TAB(XPosition); - XPosition = XPosition + TabSize; - if (XPosition >= coordScreenBufferSize.width) - { - goto EndWhile; - } - - for (til::CoordType j = 0; j < TabSize && i < LOCAL_BUFFER_SIZE; j++, i++) - { - *LocalBufPtr = UNICODE_SPACE; - LocalBufPtr++; - } - - pwchBuffer++; - break; + CursorPosition.x = textBuffer.GetRowByOffset(CursorPosition.y).NavigateToPrevious(CursorPosition.x); + AdjustCursorPosition(screenInfo, CursorPosition, fKeepCursorVisible, psScrollY); + continue; } - case UNICODE_LINEFEED: - case UNICODE_CARRIAGERETURN: - goto EndWhile; - default: - // if char is ctrl char, write ^char. - if ((dwFlags & WC_PRINTABLE_CONTROL_CHARS) && (IS_CONTROL_CHAR(RealUnicodeChar))) - { - CtrlChar: - if (i < (LOCAL_BUFFER_SIZE - 1)) - { - // WCL-NOTE: We do not properly measure that there is space for two characters - // WCL-NOTE: left on the screen. - *LocalBufPtr = (WCHAR)'^'; - LocalBufPtr++; - XPosition++; - i++; - - *LocalBufPtr = (WCHAR)(RealUnicodeChar + (WCHAR)'@'); - LocalBufPtr++; - XPosition++; - i++; - - pwchBuffer++; - } - else - { - goto EndWhile; - } - } - else - { - if (Char == UNICODE_NULL) - { - *LocalBufPtr = UNICODE_SPACE; - } - else - { - // As a special favor to incompetent apps that attempt to display control chars, - // convert to corresponding OEM Glyph Chars - WORD CharType; + const auto moveUp = [&]() { + CursorPosition.x = -1; + AdjustCursorPosition(screenInfo, CursorPosition, fKeepCursorVisible, psScrollY); - GetStringTypeW(CT_CTYPE1, &RealUnicodeChar, 1, &CharType); - if (WI_IsFlagSet(CharType, C1_CNTRL)) - { - ConvertOutputToUnicode(gci.OutputCP, - (LPSTR)&RealUnicodeChar, - 1, - LocalBufPtr, - 1); - } - else - { - // WCL-NOTE: We should never hit this. - // WCL-NOTE: 1. Normal characters are handled via the early check for IS_GLYPH_CHAR - // WCL-NOTE: 2. Control characters are handled via the CtrlChar label (if WC_PRINTABLE_CONTROL_CHARS is on) - // WCL-NOTE: And if they are control characters they will trigger the C1_CNTRL check above. - *LocalBufPtr = Char; - } - } + const auto y = cursor.GetPosition().y; + auto& row = textBuffer.GetRowByOffset(y); - LocalBufPtr++; - XPosition++; - i++; - pwchBuffer++; - } - } - } - lpString++; - pwchRealUnicode++; - *pcb += sizeof(WCHAR); - } - EndWhile: - if (i != 0) - { - CursorPosition = cursor.GetPosition(); - - // Make sure we don't write past the end of the buffer. - // WCL-NOTE: This check uses a code unit count instead of a column count. That is incorrect. - if (i > coordScreenBufferSize.width - CursorPosition.x) - { - i = coordScreenBufferSize.width - CursorPosition.x; - } + CursorPosition.x = width; + CursorPosition.y = y; - // line was wrapped if we're writing up to the end of the current row - OutputCellIterator it(std::wstring_view(LocalBuffer, i), Attributes); - const auto itEnd = screenInfo.Write(it); - - // Notify accessibility - if (screenInfo.HasAccessibilityEventing()) - { - screenInfo.NotifyAccessibilityEventing(CursorPosition.x, CursorPosition.y, CursorPosition.x + i - 1, CursorPosition.y); - } - - // The number of "spaces" or "cells" we have consumed needs to be reported and stored for later - // when/if we need to erase the command line. - TempNumSpaces += itEnd.GetCellDistance(it); - // WCL-NOTE: We are using the "estimated" X position delta instead of the actual delta from - // WCL-NOTE: the iterator. It is not clear why. If they differ, the cursor ends up in the - // WCL-NOTE: wrong place (typically inside another character). - CursorPosition.x = XPosition; + if (row.WasDoubleBytePadded()) + { + CursorPosition.x--; + TempNumSpaces--; + } - AdjustCursorPosition(screenInfo, CursorPosition, WI_IsFlagSet(dwFlags, WC_KEEP_CURSOR_VISIBLE), psScrollY); + row.SetWrapForced(false); + row.SetDoubleBytePadded(false); + }; - // WCL-NOTE: If we have processed the entire input string during our "fast one-line print" handler, - // WCL-NOTE: we are done as there is nothing more to do. Neat! - if (*pcb == BufferSize) - { - if (nullptr != pcSpaces) + // We have to move up early because the tab handling code below needs + // to be on the row of the tab already, so that we can call GetText(). + if (CursorPosition.x == 0 && CursorPosition.y != 0) { - *pcSpaces = TempNumSpaces; + moveUp(); } - return STATUS_SUCCESS; - } - continue; - } - else if (*pcb >= BufferSize) - { - // WCL-NOTE: This case looks like it is never encountered, but it *is* if WC_PRINTABLE_CONTROL_CHARS is off. - // WCL-NOTE: If the string is entirely nonprinting control characters, there will be - // WCL-NOTE: no output in the buffer (LocalBuffer; i == 0) but we will have processed - // WCL-NOTE: "every" character. We can just bail out and report the number of spaces consumed. - FAIL_FAST_IF(!(WI_IsFlagSet(screenInfo.OutputMode, ENABLE_PROCESSED_OUTPUT))); - - // this catches the case where the number of backspaces == the number of characters. - if (nullptr != pcSpaces) - { - *pcSpaces = TempNumSpaces; - } - return STATUS_SUCCESS; - } - switch (*lpString) - { - case UNICODE_BACKSPACE: - { - // move cursor backwards one space. overwrite current char with blank. - // we get here because we have to backspace from the beginning of the line - TempNumSpaces -= 1; - if (pwchBuffer == pwchBufferBackupLimit) - { - CursorPosition.x -= 1; - } - else - { - const wchar_t* Tmp; - wchar_t* Tmp2 = nullptr; - WCHAR LastChar; + til::CoordType glyphCount = 1; - const auto bufferSize = pwchBuffer - pwchBufferBackupLimit; - std::unique_ptr buffer; - try - { - buffer = std::make_unique(bufferSize); - std::fill_n(buffer.get(), bufferSize, UNICODE_NULL); - } - catch (...) + if (pwchBuffer != pwchBufferBackupLimit) { - return NTSTATUS_FROM_HRESULT(wil::ResultFromCaughtException()); - } + const auto LastChar = pwchBuffer[-1]; - for (i = 0, Tmp2 = buffer.get(), Tmp = pwchBufferBackupLimit; - i < bufferSize; - i++, Tmp++) - { - // see 18120085, these two need to be separate if statements - if (*Tmp == UNICODE_BACKSPACE) + // Deleting tabs is a bit tricky, because they have a variable width between 1 and 8 spaces, + // are stored as whitespace but are technically distinct from whitespace. + if (LastChar == UNICODE_TAB) { - //it is important we do nothing in the else case for - // this one instead of falling through to the below else. - if (Tmp2 > buffer.get()) + const auto precedingText = textBuffer.GetRowByOffset(CursorPosition.y).GetText(CursorPosition.x - 8, CursorPosition.x); + + // First, we measure the amount of spaces that precede the cursor in the text buffer, + // which is generally the amount of spaces that we end up deleting. We do it this way, + // because we don't know what kind of complex mix of wide/narrow glyphs precede the tab. + // Basically, by asking the text buffer we get the size information of the preceding text. + if (precedingText.size() >= 2 && precedingText.back() == L' ') { - Tmp2--; + auto textIt = precedingText.rbegin() + 1; + const auto textEnd = precedingText.rend(); + + for (; textIt != textEnd && *textIt == L' '; ++textIt) + { + glyphCount++; + } } + + // But there's a problem: When you print " \t" it should delete 6 spaces and not 8. + // In other words, we shouldn't delete any actual preceding whitespaces. We can ask + // the "backup" buffer (= preceding text in the commandline) for this information. + const auto backupEnd = pwchBuffer - 1; + const auto backupLimit = backupEnd - std::min(7, backupEnd - pwchBufferBackupLimit); + auto backupBeg = backupEnd; + for (; backupBeg != backupLimit && backupBeg[-1] == L' '; --backupBeg) + { + } + glyphCount -= std::min(glyphCount - 1, gsl::narrow_cast(backupEnd - backupBeg)); } - else + // Control chars in interactive mode where previously written out + // as ^X for instance, so now we also need to delete 2 glyphs. + else if (IS_CONTROL_CHAR(LastChar)) { - FAIL_FAST_IF(!(Tmp2 >= buffer.get())); - *Tmp2++ = *Tmp; + glyphCount = 2; } } - if (Tmp2 == buffer.get()) - { - LastChar = UNICODE_SPACE; - } - else - { -#pragma prefast(suppress : 26001, "This is fine. Tmp2 has to have advanced or it would equal pBuffer.") - LastChar = *(Tmp2 - 1); - } - if (LastChar == UNICODE_TAB) + for (;;) { - CursorPosition.x -= RetrieveNumberOfSpaces(sOriginalXPosition, - pwchBufferBackupLimit, - pwchBuffer - pwchBufferBackupLimit - 1); - if (CursorPosition.x < 0) + // We've already moved up if the cursor was in the first column so + // we need to start off with overwriting the text with whitespace. + // It wouldn't make sense to check the cursor position again already. { - CursorPosition.x = (coordScreenBufferSize.width - 1) / TAB_SIZE; - CursorPosition.x *= TAB_SIZE; - CursorPosition.x += 1; - CursorPosition.y -= 1; - - // since you just backspaced yourself back up into the previous row, unset the wrap - // flag on the prev row if it was set - textBuffer.GetRowByOffset(CursorPosition.y).SetWrapForced(false); + const auto previousColumn = CursorPosition.x; + CursorPosition.x = textBuffer.GetRowByOffset(CursorPosition.y).NavigateToPrevious(previousColumn); + + RowWriteState state{ + .text = { &tabSpaces[0], 8 }, + .columnBegin = CursorPosition.x, + .columnLimit = previousColumn, + }; + textBuffer.WriteLine(CursorPosition.y, fWrapAtEOL, Attributes, state); + TempNumSpaces -= previousColumn - CursorPosition.x; } - } - else if (IS_CONTROL_CHAR(LastChar)) - { - CursorPosition.x -= 1; - TempNumSpaces -= 1; - // overwrite second character of ^x sequence. - if (dwFlags & WC_DESTRUCTIVE_BACKSPACE) + // The cursor movement logic is a little different for the last iteration, so we exit early here. + glyphCount--; + if (glyphCount <= 0) { - try - { - screenInfo.Write(OutputCellIterator(UNICODE_SPACE, Attributes, 1), CursorPosition); - Status = STATUS_SUCCESS; - } - CATCH_LOG(); + break; } - CursorPosition.x -= 1; - } - else if (IsGlyphFullWidth(LastChar)) - { - CursorPosition.x -= 1; - TempNumSpaces -= 1; - - AdjustCursorPosition(screenInfo, CursorPosition, dwFlags & WC_KEEP_CURSOR_VISIBLE, psScrollY); - if (dwFlags & WC_DESTRUCTIVE_BACKSPACE) + // Otherwise, in case we need to delete 2 or more glyphs, we need to check it again. + if (CursorPosition.x == 0 && CursorPosition.y != 0) { - try - { - screenInfo.Write(OutputCellIterator(UNICODE_SPACE, Attributes, 1), CursorPosition); - Status = STATUS_SUCCESS; - } - CATCH_LOG(); + moveUp(); } - CursorPosition.x -= 1; } - else + + // After the last iteration the cursor might now be in the first column after a line + // that was previously padded with a whitespace in the last column due to a wide glyph. + // Now that the wide glyph is presumably gone, we can move up a line. + if (CursorPosition.x == 0 && CursorPosition.y != 0 && textBuffer.GetRowByOffset(CursorPosition.y - 1).WasDoubleBytePadded()) { - CursorPosition.x--; + moveUp(); } - } - if ((dwFlags & WC_LIMIT_BACKSPACE) && (CursorPosition.x < 0)) - { - CursorPosition.x = 0; - OutputDebugStringA(("CONSRV: Ignoring backspace to previous line\n")); - } - AdjustCursorPosition(screenInfo, CursorPosition, (dwFlags & WC_KEEP_CURSOR_VISIBLE) != 0, psScrollY); - if (dwFlags & WC_DESTRUCTIVE_BACKSPACE) - { - try + else { - screenInfo.Write(OutputCellIterator(UNICODE_SPACE, Attributes, 1), cursor.GetPosition()); + AdjustCursorPosition(screenInfo, CursorPosition, fKeepCursorVisible, psScrollY); } - CATCH_LOG(); - } - if (cursor.GetPosition().x == 0 && fWrapAtEOL && pwchBuffer > pwchBufferBackupLimit) - { - if (CheckBisectProcessW(screenInfo, - pwchBufferBackupLimit, - pwchBuffer + 1 - pwchBufferBackupLimit, - gsl::narrow_cast(coordScreenBufferSize.width) - sOriginalXPosition, - sOriginalXPosition, - dwFlags & WC_PRINTABLE_CONTROL_CHARS)) - { - CursorPosition.x = coordScreenBufferSize.width - 1; - CursorPosition.y = cursor.GetPosition().y - 1; - // since you just backspaced yourself back up into the previous row, unset the wrap flag - // on the prev row if it was set - textBuffer.GetRowByOffset(CursorPosition.y).SetWrapForced(false); - - AdjustCursorPosition(screenInfo, CursorPosition, dwFlags & WC_KEEP_CURSOR_VISIBLE, psScrollY); - } - } - // Notify accessibility to read the backspaced character. - // See GH:12735, MSFT:31748387 - if (screenInfo.HasAccessibilityEventing()) - { - if (auto pConsoleWindow = ServiceLocator::LocateConsoleWindow()) + // Notify accessibility to read the backspaced character. + // See GH:12735, MSFT:31748387 + if (screenInfo.HasAccessibilityEventing()) { - LOG_IF_FAILED(pConsoleWindow->SignalUia(UIA_Text_TextChangedEventId)); + if (auto pConsoleWindow = ServiceLocator::LocateConsoleWindow()) + { + LOG_IF_FAILED(pConsoleWindow->SignalUia(UIA_Text_TextChangedEventId)); + } } + continue; } - break; - } - case UNICODE_TAB: - { - const auto TabSize = NUMBER_OF_SPACES_IN_TAB(cursor.GetPosition().x); - CursorPosition.x = cursor.GetPosition().x + TabSize; - - // move cursor forward to next tab stop. fill space with blanks. - // we get here when the tab extends beyond the right edge of the - // window. if the tab goes wraps the line, set the cursor to the first - // position in the next line. - pwchBuffer++; - - TempNumSpaces += TabSize; - size_t NumChars = 0; - if (CursorPosition.x >= coordScreenBufferSize.width) + case UNICODE_TAB: { - NumChars = gsl::narrow(coordScreenBufferSize.width - cursor.GetPosition().x); - CursorPosition.x = 0; - CursorPosition.y = cursor.GetPosition().y + 1; - - // since you just tabbed yourself past the end of the row, set the wrap - textBuffer.GetRowByOffset(cursor.GetPosition().y).SetWrapForced(true); + auto CursorPosition = cursor.GetPosition(); + const auto tabCount = gsl::narrow_cast(NUMBER_OF_SPACES_IN_TAB(CursorPosition.x)); + TempNumSpaces += _writeCharsLegacyUnprocessed(screenInfo, dwFlags, psScrollY, { &tabSpaces[0], tabCount }); + continue; } - else + case UNICODE_LINEFEED: { - NumChars = gsl::narrow(CursorPosition.x - cursor.GetPosition().x); - CursorPosition.y = cursor.GetPosition().y; - } + auto CursorPosition = cursor.GetPosition(); + if (WI_IsFlagClear(screenInfo.OutputMode, DISABLE_NEWLINE_AUTO_RETURN)) + { + CursorPosition.x = 0; + } - try - { - const OutputCellIterator it(UNICODE_SPACE, Attributes, NumChars); - const auto done = screenInfo.Write(it, cursor.GetPosition()); - NumChars = done.GetCellDistance(it); + textBuffer.GetRowByOffset(CursorPosition.y).SetWrapForced(false); + CursorPosition.y = CursorPosition.y + 1; + AdjustCursorPosition(screenInfo, CursorPosition, fKeepCursorVisible, psScrollY); + continue; } - CATCH_LOG(); - - AdjustCursorPosition(screenInfo, CursorPosition, (dwFlags & WC_KEEP_CURSOR_VISIBLE) != 0, psScrollY); - break; - } - case UNICODE_CARRIAGERETURN: - { - // Carriage return moves the cursor to the beginning of the line. - // We don't need to worry about handling cr or lf for - // backspace because input is sent to the user on cr or lf. - pwchBuffer++; - CursorPosition.x = 0; - CursorPosition.y = cursor.GetPosition().y; - AdjustCursorPosition(screenInfo, CursorPosition, (dwFlags & WC_KEEP_CURSOR_VISIBLE) != 0, psScrollY); - break; - } - case UNICODE_LINEFEED: - { - // move cursor to the next line. - pwchBuffer++; - - if (WI_IsFlagClear(screenInfo.OutputMode, DISABLE_NEWLINE_AUTO_RETURN)) + case UNICODE_CARRIAGERETURN: { - // Traditionally, we reset the X position to 0 with a newline automatically. - // Some things might not want this automatic "ONLCR line discipline" (for example, things that are expecting a *NIX behavior.) - // They will turn it off with an output mode flag. + auto CursorPosition = cursor.GetPosition(); CursorPosition.x = 0; + AdjustCursorPosition(screenInfo, CursorPosition, fKeepCursorVisible, psScrollY); + continue; + } + default: + break; } - CursorPosition.y = cursor.GetPosition().y + 1; - + if (WI_IsFlagSet(dwFlags, WC_INTERACTIVE) && IS_CONTROL_CHAR(*it)) { - // since we explicitly just moved down a row, clear the wrap status on the row we just came from - textBuffer.GetRowByOffset(cursor.GetPosition().y).SetWrapForced(false); + const wchar_t wchs[2]{ L'^', static_cast(*it + L'@') }; + TempNumSpaces += _writeCharsLegacyUnprocessed(screenInfo, dwFlags, psScrollY, { &wchs[0], 2 }); } - - AdjustCursorPosition(screenInfo, CursorPosition, (dwFlags & WC_KEEP_CURSOR_VISIBLE) != 0, psScrollY); - break; - } - default: - { - const auto Char = *lpString; - if (Char >= UNICODE_SPACE && - IsGlyphFullWidth(Char) && - XPosition >= (coordScreenBufferSize.width - 1) && - fWrapAtEOL) + else { - const auto TargetPoint = cursor.GetPosition(); - auto& Row = textBuffer.GetRowByOffset(TargetPoint.y); - - try + // TODO test + // As a special favor to incompetent apps that attempt to display control chars, + // convert to corresponding OEM Glyph Chars + const auto cp = ServiceLocator::LocateGlobals().getConsoleInformation().OutputCP; + wchar_t buffer; + const auto result = MultiByteToWideChar(cp, MB_USEGLYPHCHARS, reinterpret_cast(it), 1, &buffer, 1); + if (result == 1) { - // If we're on top of a trailing cell, clear it and the previous cell. - if (Row.DbcsAttrAt(TargetPoint.x) == DbcsAttribute::Trailing) - { - // Space to clear for 2 cells. - OutputCellIterator it(UNICODE_SPACE, 2); - - // Back target point up one. - auto writeTarget = TargetPoint; - writeTarget.x--; - - // Write 2 clear cells. - screenInfo.Write(it, writeTarget); - } + TempNumSpaces += _writeCharsLegacyUnprocessed(screenInfo, dwFlags, psScrollY, { &buffer, 1 }); } - catch (...) - { - return NTSTATUS_FROM_HRESULT(wil::ResultFromCaughtException()); - } - - CursorPosition.x = 0; - CursorPosition.y = TargetPoint.y + 1; - - // since you just moved yourself down onto the next row with 1 character, that sounds like a - // forced wrap so set the flag - Row.SetWrapForced(true); - - // Additionally, this padding is only called for IsConsoleFullWidth (a.k.a. when a character - // is too wide to fit on the current line). - Row.SetDoubleBytePadded(true); - - AdjustCursorPosition(screenInfo, CursorPosition, dwFlags & WC_KEEP_CURSOR_VISIBLE, psScrollY); - continue; } - break; } - } - if (FAILED_NTSTATUS(Status)) - { - return Status; - } - - *pcb += sizeof(WCHAR); - lpString++; - pwchRealUnicode++; } - if (nullptr != pcSpaces) + if (pcSpaces) { *pcSpaces = TempNumSpaces; } - return STATUS_SUCCESS; + return S_OK; } +NT_CATCH_RETURN() // Routine Description: // - This routine writes a string to the screen, processing any embedded @@ -699,9 +439,8 @@ void AdjustCursorPosition(SCREEN_INFORMATION& screenInfo, _In_ til::point coordC // - pcb - On input, number of bytes to write. On output, number of bytes written. // - pcSpaces - On output, the number of spaces consumed by the written characters. // - dwFlags - -// WC_DESTRUCTIVE_BACKSPACE backspace overwrites characters. +// WC_INTERACTIVE backspace overwrites characters, control characters are expanded (as in, to "^X") // WC_KEEP_CURSOR_VISIBLE change window origin (viewport) desirable when hit rt. edge -// WC_PRINTABLE_CONTROL_CHARS if control characters should be expanded (as in, to "^X") // Return Value: // Note: // - This routine does not process tabs and backspace properly. That code will be implemented as part of the line editing services. @@ -714,9 +453,9 @@ void AdjustCursorPosition(SCREEN_INFORMATION& screenInfo, _In_ til::point coordC const til::CoordType sOriginalXPosition, const DWORD dwFlags, _Inout_opt_ til::CoordType* const psScrollY) +try { - if (!WI_IsFlagSet(screenInfo.OutputMode, ENABLE_VIRTUAL_TERMINAL_PROCESSING) || - !WI_IsFlagSet(screenInfo.OutputMode, ENABLE_PROCESSED_OUTPUT)) + if (WI_IsAnyFlagClear(screenInfo.OutputMode, ENABLE_VIRTUAL_TERMINAL_PROCESSING | ENABLE_PROCESSED_OUTPUT)) { return WriteCharsLegacy(screenInfo, pwchBufferBackupLimit, @@ -729,40 +468,19 @@ void AdjustCursorPosition(SCREEN_INFORMATION& screenInfo, _In_ til::point coordC psScrollY); } - auto Status = STATUS_SUCCESS; + auto& machine = screenInfo.GetStateMachine(); + const auto cch = *pcb / sizeof(WCHAR); - const auto BufferSize = *pcb; - *pcb = 0; + machine.ProcessString({ pwchRealUnicode, cch }); + if (nullptr != pcSpaces) { - size_t TempNumSpaces = 0; - - { - if (SUCCEEDED_NTSTATUS(Status)) - { - FAIL_FAST_IF(!(WI_IsFlagSet(screenInfo.OutputMode, ENABLE_PROCESSED_OUTPUT))); - FAIL_FAST_IF(!(WI_IsFlagSet(screenInfo.OutputMode, ENABLE_VIRTUAL_TERMINAL_PROCESSING))); - - // defined down in the WriteBuffer default case hiding on the other end of the state machine. See outputStream.cpp - // This is the only mode used by DoWriteConsole. - FAIL_FAST_IF(!(WI_IsFlagSet(dwFlags, WC_LIMIT_BACKSPACE))); - - auto& machine = screenInfo.GetStateMachine(); - const auto cch = BufferSize / sizeof(WCHAR); - - machine.ProcessString({ pwchRealUnicode, cch }); - *pcb += BufferSize; - } - } - - if (nullptr != pcSpaces) - { - *pcSpaces = TempNumSpaces; - } + *pcSpaces = 0; } - return Status; + return STATUS_SUCCESS; } +NT_CATCH_RETURN() // Routine Description: // - Takes the given text and inserts it into the given screen buffer. @@ -825,7 +543,7 @@ void AdjustCursorPosition(SCREEN_INFORMATION& screenInfo, _In_ til::point coordC pcbBuffer, nullptr, textBuffer.GetCursor().GetPosition().x, - WC_LIMIT_BACKSPACE, + 0, nullptr); } diff --git a/src/host/_stream.h b/src/host/_stream.h index d12e6622e72..58351c86cc1 100644 --- a/src/host/_stream.h +++ b/src/host/_stream.h @@ -54,9 +54,8 @@ Routine Description: bytes written. NumSpaces - On output, the number of spaces consumed by the written characters. dwFlags - - WC_DESTRUCTIVE_BACKSPACE backspace overwrites characters. + WC_INTERACTIVE backspace overwrites characters, control characters are expanded (as in, to "^X") WC_KEEP_CURSOR_VISIBLE change window origin desirable when hit rt. edge - WC_PRINTABLE_CONTROL_CHARS if control characters should be expanded (as in, to "^X") Return Value: diff --git a/src/host/cmdline.cpp b/src/host/cmdline.cpp index 094ba9ba2dc..4b48f4bd24b 100644 --- a/src/host/cmdline.cpp +++ b/src/host/cmdline.cpp @@ -251,7 +251,7 @@ void RedrawCommandLine(COOKED_READ_DATA& cookedReadData) &cookedReadData.BytesRead(), &cookedReadData.VisibleCharCount(), cookedReadData.OriginalCursorPosition().x, - WC_DESTRUCTIVE_BACKSPACE | WC_KEEP_CURSOR_VISIBLE | WC_PRINTABLE_CONTROL_CHARS, + WC_INTERACTIVE | WC_KEEP_CURSOR_VISIBLE, &ScrollY); FAIL_FAST_IF_NTSTATUS_FAILED(Status); @@ -291,7 +291,7 @@ void SetCurrentCommandLine(COOKED_READ_DATA& cookedReadData, _In_ SHORT Index) / &cookedReadData.BytesRead(), &cookedReadData.VisibleCharCount(), cookedReadData.OriginalCursorPosition().x, - WC_DESTRUCTIVE_BACKSPACE | WC_KEEP_CURSOR_VISIBLE | WC_PRINTABLE_CONTROL_CHARS, + WC_INTERACTIVE | WC_KEEP_CURSOR_VISIBLE, &ScrollY)); cookedReadData.OriginalCursorPosition().y += ScrollY; } @@ -455,7 +455,7 @@ void CommandLine::_processHistoryCycling(COOKED_READ_DATA& cookedReadData, &cookedReadData.BytesRead(), &cookedReadData.VisibleCharCount(), cookedReadData.OriginalCursorPosition().x, - WC_DESTRUCTIVE_BACKSPACE | WC_KEEP_CURSOR_VISIBLE | WC_PRINTABLE_CONTROL_CHARS, + WC_INTERACTIVE | WC_KEEP_CURSOR_VISIBLE, &ScrollY)); cookedReadData.OriginalCursorPosition().y += ScrollY; } @@ -490,7 +490,7 @@ void CommandLine::_setPromptToOldestCommand(COOKED_READ_DATA& cookedReadData) &cookedReadData.BytesRead(), &cookedReadData.VisibleCharCount(), cookedReadData.OriginalCursorPosition().x, - WC_DESTRUCTIVE_BACKSPACE | WC_KEEP_CURSOR_VISIBLE | WC_PRINTABLE_CONTROL_CHARS, + WC_INTERACTIVE | WC_KEEP_CURSOR_VISIBLE, &ScrollY)); cookedReadData.OriginalCursorPosition().y += ScrollY; } @@ -526,7 +526,7 @@ void CommandLine::_setPromptToNewestCommand(COOKED_READ_DATA& cookedReadData) &cookedReadData.BytesRead(), &cookedReadData.VisibleCharCount(), cookedReadData.OriginalCursorPosition().x, - WC_DESTRUCTIVE_BACKSPACE | WC_KEEP_CURSOR_VISIBLE | WC_PRINTABLE_CONTROL_CHARS, + WC_INTERACTIVE | WC_KEEP_CURSOR_VISIBLE, &ScrollY)); cookedReadData.OriginalCursorPosition().y += ScrollY; } @@ -553,7 +553,7 @@ void CommandLine::DeletePromptAfterCursor(COOKED_READ_DATA& cookedReadData) noex &cookedReadData.BytesRead(), &cookedReadData.VisibleCharCount(), cookedReadData.OriginalCursorPosition().x, - WC_DESTRUCTIVE_BACKSPACE | WC_KEEP_CURSOR_VISIBLE | WC_PRINTABLE_CONTROL_CHARS, + WC_INTERACTIVE | WC_KEEP_CURSOR_VISIBLE, nullptr)); } } @@ -580,7 +580,7 @@ til::point CommandLine::_deletePromptBeforeCursor(COOKED_READ_DATA& cookedReadDa &cookedReadData.BytesRead(), &cookedReadData.VisibleCharCount(), cookedReadData.OriginalCursorPosition().x, - WC_DESTRUCTIVE_BACKSPACE | WC_KEEP_CURSOR_VISIBLE | WC_PRINTABLE_CONTROL_CHARS, + WC_INTERACTIVE | WC_KEEP_CURSOR_VISIBLE, nullptr)); } return cookedReadData.OriginalCursorPosition(); @@ -869,7 +869,7 @@ til::point CommandLine::_moveCursorRight(COOKED_READ_DATA& cookedReadData) noexc &CharsToWrite, &NumSpaces, cookedReadData.OriginalCursorPosition().x, - WC_DESTRUCTIVE_BACKSPACE | WC_KEEP_CURSOR_VISIBLE | WC_PRINTABLE_CONTROL_CHARS, + WC_INTERACTIVE | WC_KEEP_CURSOR_VISIBLE, &ScrollY)); cookedReadData.OriginalCursorPosition().y += ScrollY; cookedReadData.VisibleCharCount() += NumSpaces; @@ -912,7 +912,7 @@ void CommandLine::_insertCtrlZ(COOKED_READ_DATA& cookedReadData) noexcept &CharsToWrite, &NumSpaces, cookedReadData.OriginalCursorPosition().x, - WC_DESTRUCTIVE_BACKSPACE | WC_KEEP_CURSOR_VISIBLE | WC_PRINTABLE_CONTROL_CHARS, + WC_INTERACTIVE | WC_KEEP_CURSOR_VISIBLE, &ScrollY)); cookedReadData.OriginalCursorPosition().y += ScrollY; cookedReadData.VisibleCharCount() += NumSpaces; @@ -962,7 +962,7 @@ void CommandLine::_fillPromptWithPreviousCommandFragment(COOKED_READ_DATA& cooke &cchCount, &NumSpaces, cookedReadData.OriginalCursorPosition().x, - WC_DESTRUCTIVE_BACKSPACE | WC_KEEP_CURSOR_VISIBLE | WC_PRINTABLE_CONTROL_CHARS, + WC_INTERACTIVE | WC_KEEP_CURSOR_VISIBLE, &ScrollY)); cookedReadData.OriginalCursorPosition().y += ScrollY; cookedReadData.VisibleCharCount() += NumSpaces; @@ -1007,7 +1007,7 @@ til::point CommandLine::_cycleMatchingCommandHistoryToPrompt(COOKED_READ_DATA& c &cookedReadData.BytesRead(), &cookedReadData.VisibleCharCount(), cookedReadData.OriginalCursorPosition().x, - WC_DESTRUCTIVE_BACKSPACE | WC_KEEP_CURSOR_VISIBLE | WC_PRINTABLE_CONTROL_CHARS, + WC_INTERACTIVE | WC_KEEP_CURSOR_VISIBLE, &ScrollY)); cookedReadData.OriginalCursorPosition().y += ScrollY; cursorPosition.y += ScrollY; @@ -1062,7 +1062,7 @@ til::point CommandLine::DeleteFromRightOfCursor(COOKED_READ_DATA& cookedReadData &cookedReadData.BytesRead(), &cookedReadData.VisibleCharCount(), cookedReadData.OriginalCursorPosition().x, - WC_DESTRUCTIVE_BACKSPACE | WC_KEEP_CURSOR_VISIBLE | WC_PRINTABLE_CONTROL_CHARS, + WC_INTERACTIVE | WC_KEEP_CURSOR_VISIBLE, nullptr)); } diff --git a/src/host/cmdline.h b/src/host/cmdline.h index 32f3ee4e7b4..0614f8143f3 100644 --- a/src/host/cmdline.h +++ b/src/host/cmdline.h @@ -131,18 +131,18 @@ void DeleteCommandLine(COOKED_READ_DATA& cookedReadData, const bool fUpdateField void RedrawCommandLine(COOKED_READ_DATA& cookedReadData); // Values for WriteChars(), WriteCharsLegacy() dwFlags -#define WC_DESTRUCTIVE_BACKSPACE 0x01 +#define WC_INTERACTIVE 0x01 #define WC_KEEP_CURSOR_VISIBLE 0x02 -#define WC_PRINTABLE_CONTROL_CHARS 0x04 // This is no longer necessary. The buffer will always be Unicode. We don't need to perform special work to check if we're in a raster font // and convert the entire buffer to match (and all insertions). -//#define WC_FALSIFY_UNICODE 0x08 - -#define WC_LIMIT_BACKSPACE 0x10 -//#define WC_NONDESTRUCTIVE_TAB 0x20 - This is not needed anymore, because the VT code handles tabs internally now. -//#define WC_NEWLINE_SAVE_X 0x40 - This has been replaced with an output mode flag instead as it's line discipline behavior that may not necessarily be coupled with VT. -//#define WC_DELAY_EOL_WRAP 0x80 - This is not needed anymore, because the AdaptDispatch class handles all VT output. +//#define WC_DESTRUCTIVE_BACKSPACE 0x01 - Replaced with WC_INTERACTIVE +//#define WC_PRINTABLE_CONTROL_CHARS 0x04 - Replaced with WC_INTERACTIVE +//#define WC_FALSIFY_UNICODE 0x08 +//#define WC_LIMIT_BACKSPACE 0x10 - Replaced with !WC_INTERACTIVE +//#define WC_NONDESTRUCTIVE_TAB 0x20 - This is not needed anymore, because the VT code handles tabs internally now. +//#define WC_NEWLINE_SAVE_X 0x40 - This has been replaced with an output mode flag instead as it's line discipline behavior that may not necessarily be coupled with VT. +//#define WC_DELAY_EOL_WRAP 0x80 - This is not needed anymore, because the AdaptDispatch class handles all VT output. // Word delimiters bool IsWordDelim(const wchar_t wch); diff --git a/src/host/ft_fuzzer/fuzzmain.cpp b/src/host/ft_fuzzer/fuzzmain.cpp index 87f2ee78897..4b8f04dea6e 100644 --- a/src/host/ft_fuzzer/fuzzmain.cpp +++ b/src/host/ft_fuzzer/fuzzmain.cpp @@ -142,7 +142,7 @@ extern "C" __declspec(dllexport) int LLVMFuzzerTestOneInput(const uint8_t* data, &sizeInBytes, nullptr, 0, - WC_PRINTABLE_CONTROL_CHARS | WC_DESTRUCTIVE_BACKSPACE | WC_KEEP_CURSOR_VISIBLE, + WC_INTERACTIVE | WC_KEEP_CURSOR_VISIBLE, &scrollY); return 0; } diff --git a/src/host/readDataCooked.cpp b/src/host/readDataCooked.cpp index eac6e5fb074..6da7ed899ab 100644 --- a/src/host/readDataCooked.cpp +++ b/src/host/readDataCooked.cpp @@ -534,7 +534,7 @@ bool COOKED_READ_DATA::ProcessInput(const wchar_t wchOrig, &NumToWrite, &NumSpaces, _originalCursorPosition.x, - WC_DESTRUCTIVE_BACKSPACE | WC_KEEP_CURSOR_VISIBLE | WC_PRINTABLE_CONTROL_CHARS, + WC_INTERACTIVE | WC_KEEP_CURSOR_VISIBLE, &ScrollY); if (SUCCEEDED_NTSTATUS(status)) { @@ -616,7 +616,7 @@ bool COOKED_READ_DATA::ProcessInput(const wchar_t wchOrig, &NumToWrite, nullptr, _originalCursorPosition.x, - WC_DESTRUCTIVE_BACKSPACE | WC_KEEP_CURSOR_VISIBLE | WC_PRINTABLE_CONTROL_CHARS, + WC_INTERACTIVE | WC_KEEP_CURSOR_VISIBLE, nullptr); if (FAILED_NTSTATUS(status)) { @@ -717,7 +717,7 @@ bool COOKED_READ_DATA::ProcessInput(const wchar_t wchOrig, // write the new command line to the screen NumToWrite = _bytesRead; - DWORD dwFlags = WC_DESTRUCTIVE_BACKSPACE | WC_PRINTABLE_CONTROL_CHARS; + DWORD dwFlags = WC_INTERACTIVE; if (wch == UNICODE_CARRIAGERETURN) { dwFlags |= WC_KEEP_CURSOR_VISIBLE; @@ -782,7 +782,7 @@ bool COOKED_READ_DATA::ProcessInput(const wchar_t wchOrig, &NumToWrite, nullptr, _originalCursorPosition.x, - WC_DESTRUCTIVE_BACKSPACE | WC_KEEP_CURSOR_VISIBLE | WC_PRINTABLE_CONTROL_CHARS, + WC_INTERACTIVE | WC_KEEP_CURSOR_VISIBLE, nullptr); if (FAILED_NTSTATUS(status)) { @@ -844,7 +844,7 @@ size_t COOKED_READ_DATA::Write(const std::wstring_view wstr) &bytesInserted, &NumSpaces, OriginalCursorPosition().x, - WC_DESTRUCTIVE_BACKSPACE | WC_KEEP_CURSOR_VISIBLE | WC_PRINTABLE_CONTROL_CHARS, + WC_INTERACTIVE | WC_KEEP_CURSOR_VISIBLE, &ScrollY)); OriginalCursorPosition().y += ScrollY; VisibleCharCount() += NumSpaces; diff --git a/src/host/ut_host/ScreenBufferTests.cpp b/src/host/ut_host/ScreenBufferTests.cpp index 441394ef088..b26801be0a1 100644 --- a/src/host/ut_host/ScreenBufferTests.cpp +++ b/src/host/ut_host/ScreenBufferTests.cpp @@ -2895,7 +2895,7 @@ void ScreenBufferTests::BackspaceDefaultAttrsWriteCharsLegacy() { BEGIN_TEST_METHOD_PROPERTIES() TEST_METHOD_PROPERTY(L"Data:writeSingly", L"{false, true}") - TEST_METHOD_PROPERTY(L"Data:writeCharsLegacyMode", L"{0, 1, 2, 3, 4, 5, 6, 7}") + TEST_METHOD_PROPERTY(L"Data:writeCharsLegacyMode", L"{0, 1, 2}") END_TEST_METHOD_PROPERTIES(); bool writeSingly; From d2f09277842d4c6ea1d1b0bc6cb7bbd460197cdc Mon Sep 17 00:00:00 2001 From: Leonard Hecker Date: Thu, 22 Jun 2023 23:20:19 +0200 Subject: [PATCH 3/9] Remove unused WCL flags --- src/host/cmdline.h | 10 ---------- 1 file changed, 10 deletions(-) diff --git a/src/host/cmdline.h b/src/host/cmdline.h index 0614f8143f3..a8669edadde 100644 --- a/src/host/cmdline.h +++ b/src/host/cmdline.h @@ -134,16 +134,6 @@ void RedrawCommandLine(COOKED_READ_DATA& cookedReadData); #define WC_INTERACTIVE 0x01 #define WC_KEEP_CURSOR_VISIBLE 0x02 -// This is no longer necessary. The buffer will always be Unicode. We don't need to perform special work to check if we're in a raster font -// and convert the entire buffer to match (and all insertions). -//#define WC_DESTRUCTIVE_BACKSPACE 0x01 - Replaced with WC_INTERACTIVE -//#define WC_PRINTABLE_CONTROL_CHARS 0x04 - Replaced with WC_INTERACTIVE -//#define WC_FALSIFY_UNICODE 0x08 -//#define WC_LIMIT_BACKSPACE 0x10 - Replaced with !WC_INTERACTIVE -//#define WC_NONDESTRUCTIVE_TAB 0x20 - This is not needed anymore, because the VT code handles tabs internally now. -//#define WC_NEWLINE_SAVE_X 0x40 - This has been replaced with an output mode flag instead as it's line discipline behavior that may not necessarily be coupled with VT. -//#define WC_DELAY_EOL_WRAP 0x80 - This is not needed anymore, because the AdaptDispatch class handles all VT output. - // Word delimiters bool IsWordDelim(const wchar_t wch); bool IsWordDelim(const std::wstring_view charData); From d543af076766ee7beb4103f3fbcdba9ff4dc5ceb Mon Sep 17 00:00:00 2001 From: Leonard Hecker Date: Fri, 23 Jun 2023 16:13:23 +0200 Subject: [PATCH 4/9] Address feedback --- src/buffer/out/textBuffer.cpp | 2 +- src/buffer/out/textBuffer.hpp | 2 +- src/host/_stream.cpp | 124 +++++++++++++++++----------------- 3 files changed, 65 insertions(+), 63 deletions(-) diff --git a/src/buffer/out/textBuffer.cpp b/src/buffer/out/textBuffer.cpp index 47b7d71541e..2a958fbd08d 100644 --- a/src/buffer/out/textBuffer.cpp +++ b/src/buffer/out/textBuffer.cpp @@ -933,7 +933,7 @@ const Cursor& TextBuffer::GetCursor() const noexcept return _cursor; } -[[nodiscard]] TextAttribute TextBuffer::GetCurrentAttributes() const noexcept +const TextAttribute& TextBuffer::GetCurrentAttributes() const noexcept { return _currentAttributes; } diff --git a/src/buffer/out/textBuffer.hpp b/src/buffer/out/textBuffer.hpp index ff7d274da5e..6733ff4bd91 100644 --- a/src/buffer/out/textBuffer.hpp +++ b/src/buffer/out/textBuffer.hpp @@ -133,7 +133,7 @@ class TextBuffer final til::CoordType TotalRowCount() const noexcept; - [[nodiscard]] TextAttribute GetCurrentAttributes() const noexcept; + const TextAttribute& GetCurrentAttributes() const noexcept; void SetCurrentAttributes(const TextAttribute& currentAttributes) noexcept; diff --git a/src/host/_stream.cpp b/src/host/_stream.cpp index bb58bf3e4e2..3927bea31ac 100644 --- a/src/host/_stream.cpp +++ b/src/host/_stream.cpp @@ -166,36 +166,31 @@ try static constexpr wchar_t tabSpaces[8]{ L' ', L' ', L' ', L' ', L' ', L' ', L' ', L' ' }; UNREFERENCED_PARAMETER(sOriginalXPosition); - UNREFERENCED_PARAMETER(pwchBuffer); - UNREFERENCED_PARAMETER(pwchBufferBackupLimit); auto& textBuffer = screenInfo.GetTextBuffer(); auto& cursor = textBuffer.GetCursor(); - const auto width = textBuffer.GetSize().Width(); - const auto Attributes = textBuffer.GetCurrentAttributes(); - const auto fKeepCursorVisible = WI_IsFlagSet(dwFlags, WC_KEEP_CURSOR_VISIBLE); - const auto fUnprocessed = WI_IsFlagClear(screenInfo.OutputMode, ENABLE_PROCESSED_OUTPUT); - const auto fWrapAtEOL = WI_IsFlagSet(screenInfo.OutputMode, ENABLE_WRAP_AT_EOL_OUTPUT); + const auto keepCursorVisible = WI_IsFlagSet(dwFlags, WC_KEEP_CURSOR_VISIBLE); + const auto wrapAtEOL = WI_IsFlagSet(screenInfo.OutputMode, ENABLE_WRAP_AT_EOL_OUTPUT); auto it = pwchRealUnicode; const auto end = it + *pcb / sizeof(wchar_t); - size_t TempNumSpaces = 0; + size_t numSpaces = 0; - if (cursor.IsDelayedEOLWrap() && fWrapAtEOL) + if (cursor.IsDelayedEOLWrap() && wrapAtEOL) { - auto CursorPosition = cursor.GetPosition(); - const auto delayedCursorPosition = cursor.GetDelayedAtPosition(); + auto pos = cursor.GetPosition(); + const auto delayed = cursor.GetDelayedAtPosition(); cursor.ResetDelayEOLWrap(); - if (delayedCursorPosition == CursorPosition) + if (delayed == pos) { - CursorPosition.x = 0; - CursorPosition.y++; - AdjustCursorPosition(screenInfo, CursorPosition, fKeepCursorVisible, psScrollY); + pos.x = 0; + pos.y++; + AdjustCursorPosition(screenInfo, pos, keepCursorVisible, psScrollY); } } - if (fUnprocessed) + if (WI_IsFlagClear(screenInfo.OutputMode, ENABLE_PROCESSED_OUTPUT)) { - TempNumSpaces += _writeCharsLegacyUnprocessed(screenInfo, dwFlags, psScrollY, { it, end }); + numSpaces += _writeCharsLegacyUnprocessed(screenInfo, dwFlags, psScrollY, { it, end }); it = end; } @@ -204,7 +199,7 @@ try const auto nextControlChar = std::find_if(it, end, [](const auto& wch) { return !IS_GLYPH_CHAR(wch); }); if (nextControlChar != it) { - TempNumSpaces += _writeCharsLegacyUnprocessed(screenInfo, dwFlags, psScrollY, { it, nextControlChar }); + numSpaces += _writeCharsLegacyUnprocessed(screenInfo, dwFlags, psScrollY, { it, nextControlChar }); it = nextControlChar; } @@ -217,7 +212,7 @@ try { break; } - TempNumSpaces += _writeCharsLegacyUnprocessed(screenInfo, dwFlags, psScrollY, { &tabSpaces[0], 1 }); + numSpaces += _writeCharsLegacyUnprocessed(screenInfo, dwFlags, psScrollY, { &tabSpaces[0], 1 }); continue; case UNICODE_BELL: if (WI_IsFlagSet(dwFlags, WC_INTERACTIVE)) @@ -228,29 +223,29 @@ try continue; case UNICODE_BACKSPACE: { - auto CursorPosition = cursor.GetPosition(); + auto pos = cursor.GetPosition(); if (WI_IsFlagClear(dwFlags, WC_INTERACTIVE)) { - CursorPosition.x = textBuffer.GetRowByOffset(CursorPosition.y).NavigateToPrevious(CursorPosition.x); - AdjustCursorPosition(screenInfo, CursorPosition, fKeepCursorVisible, psScrollY); + pos.x = textBuffer.GetRowByOffset(pos.y).NavigateToPrevious(pos.x); + AdjustCursorPosition(screenInfo, pos, keepCursorVisible, psScrollY); continue; } const auto moveUp = [&]() { - CursorPosition.x = -1; - AdjustCursorPosition(screenInfo, CursorPosition, fKeepCursorVisible, psScrollY); + pos.x = -1; + AdjustCursorPosition(screenInfo, pos, keepCursorVisible, psScrollY); const auto y = cursor.GetPosition().y; auto& row = textBuffer.GetRowByOffset(y); - CursorPosition.x = width; - CursorPosition.y = y; + pos.x = textBuffer.GetSize().RightExclusive(); + pos.y = y; if (row.WasDoubleBytePadded()) { - CursorPosition.x--; - TempNumSpaces--; + pos.x--; + numSpaces--; } row.SetWrapForced(false); @@ -259,7 +254,7 @@ try // We have to move up early because the tab handling code below needs // to be on the row of the tab already, so that we can call GetText(). - if (CursorPosition.x == 0 && CursorPosition.y != 0) + if (pos.x == 0 && pos.y != 0) { moveUp(); } @@ -268,13 +263,13 @@ try if (pwchBuffer != pwchBufferBackupLimit) { - const auto LastChar = pwchBuffer[-1]; + const auto lastChar = pwchBuffer[-1]; // Deleting tabs is a bit tricky, because they have a variable width between 1 and 8 spaces, // are stored as whitespace but are technically distinct from whitespace. - if (LastChar == UNICODE_TAB) + if (lastChar == UNICODE_TAB) { - const auto precedingText = textBuffer.GetRowByOffset(CursorPosition.y).GetText(CursorPosition.x - 8, CursorPosition.x); + const auto precedingText = textBuffer.GetRowByOffset(pos.y).GetText(pos.x - 8, pos.x); // First, we measure the amount of spaces that precede the cursor in the text buffer, // which is generally the amount of spaces that we end up deleting. We do it this way, @@ -294,17 +289,24 @@ try // But there's a problem: When you print " \t" it should delete 6 spaces and not 8. // In other words, we shouldn't delete any actual preceding whitespaces. We can ask // the "backup" buffer (= preceding text in the commandline) for this information. + // + // backupEnd points to the character immediately preceding the tab (LastChar). const auto backupEnd = pwchBuffer - 1; - const auto backupLimit = backupEnd - std::min(7, backupEnd - pwchBufferBackupLimit); + // backupLimit points to how far back we need to search. Even if we have 9000 characters in our command line, + // we'll only need to check a total of 8 whitespaces. "pwchBuffer - pwchBufferBackupLimit" will + // always be at least 1 because that's the \t character in the backup buffer. In other words, + // backupLimit will at a minimum be equal to backupEnd, or preced it by 7 more characters. + const auto backupLimit = pwchBuffer - std::min(8, pwchBuffer - pwchBufferBackupLimit); + // Now count how many spaces precede the \t character and remove them from the glyphCount. auto backupBeg = backupEnd; - for (; backupBeg != backupLimit && backupBeg[-1] == L' '; --backupBeg) + for (; backupBeg != backupLimit && backupBeg[-1] == L' '; --backupBeg, --glyphCount) { } glyphCount -= std::min(glyphCount - 1, gsl::narrow_cast(backupEnd - backupBeg)); } // Control chars in interactive mode where previously written out // as ^X for instance, so now we also need to delete 2 glyphs. - else if (IS_CONTROL_CHAR(LastChar)) + else if (IS_CONTROL_CHAR(lastChar)) { glyphCount = 2; } @@ -316,16 +318,16 @@ try // we need to start off with overwriting the text with whitespace. // It wouldn't make sense to check the cursor position again already. { - const auto previousColumn = CursorPosition.x; - CursorPosition.x = textBuffer.GetRowByOffset(CursorPosition.y).NavigateToPrevious(previousColumn); + const auto previousColumn = pos.x; + pos.x = textBuffer.GetRowByOffset(pos.y).NavigateToPrevious(previousColumn); RowWriteState state{ .text = { &tabSpaces[0], 8 }, - .columnBegin = CursorPosition.x, + .columnBegin = pos.x, .columnLimit = previousColumn, }; - textBuffer.WriteLine(CursorPosition.y, fWrapAtEOL, Attributes, state); - TempNumSpaces -= previousColumn - CursorPosition.x; + textBuffer.WriteLine(pos.y, wrapAtEOL, textBuffer.GetCurrentAttributes(), state); + numSpaces -= previousColumn - pos.x; } // The cursor movement logic is a little different for the last iteration, so we exit early here. @@ -336,7 +338,7 @@ try } // Otherwise, in case we need to delete 2 or more glyphs, we need to check it again. - if (CursorPosition.x == 0 && CursorPosition.y != 0) + if (pos.x == 0 && pos.y != 0) { moveUp(); } @@ -345,20 +347,20 @@ try // After the last iteration the cursor might now be in the first column after a line // that was previously padded with a whitespace in the last column due to a wide glyph. // Now that the wide glyph is presumably gone, we can move up a line. - if (CursorPosition.x == 0 && CursorPosition.y != 0 && textBuffer.GetRowByOffset(CursorPosition.y - 1).WasDoubleBytePadded()) + if (pos.x == 0 && pos.y != 0 && textBuffer.GetRowByOffset(pos.y - 1).WasDoubleBytePadded()) { moveUp(); } else { - AdjustCursorPosition(screenInfo, CursorPosition, fKeepCursorVisible, psScrollY); + AdjustCursorPosition(screenInfo, pos, keepCursorVisible, psScrollY); } // Notify accessibility to read the backspaced character. // See GH:12735, MSFT:31748387 if (screenInfo.HasAccessibilityEventing()) { - if (auto pConsoleWindow = ServiceLocator::LocateConsoleWindow()) + if (const auto pConsoleWindow = ServiceLocator::LocateConsoleWindow()) { LOG_IF_FAILED(pConsoleWindow->SignalUia(UIA_Text_TextChangedEventId)); } @@ -367,29 +369,29 @@ try } case UNICODE_TAB: { - auto CursorPosition = cursor.GetPosition(); - const auto tabCount = gsl::narrow_cast(NUMBER_OF_SPACES_IN_TAB(CursorPosition.x)); - TempNumSpaces += _writeCharsLegacyUnprocessed(screenInfo, dwFlags, psScrollY, { &tabSpaces[0], tabCount }); + const auto pos = cursor.GetPosition(); + const auto tabCount = gsl::narrow_cast(NUMBER_OF_SPACES_IN_TAB(pos.x)); + numSpaces += _writeCharsLegacyUnprocessed(screenInfo, dwFlags, psScrollY, { &tabSpaces[0], tabCount }); continue; } case UNICODE_LINEFEED: { - auto CursorPosition = cursor.GetPosition(); + auto pos = cursor.GetPosition(); if (WI_IsFlagClear(screenInfo.OutputMode, DISABLE_NEWLINE_AUTO_RETURN)) { - CursorPosition.x = 0; + pos.x = 0; } - textBuffer.GetRowByOffset(CursorPosition.y).SetWrapForced(false); - CursorPosition.y = CursorPosition.y + 1; - AdjustCursorPosition(screenInfo, CursorPosition, fKeepCursorVisible, psScrollY); + textBuffer.GetRowByOffset(pos.y).SetWrapForced(false); + pos.y = pos.y + 1; + AdjustCursorPosition(screenInfo, pos, keepCursorVisible, psScrollY); continue; } case UNICODE_CARRIAGERETURN: { - auto CursorPosition = cursor.GetPosition(); - CursorPosition.x = 0; - AdjustCursorPosition(screenInfo, CursorPosition, fKeepCursorVisible, psScrollY); + auto pos = cursor.GetPosition(); + pos.x = 0; + AdjustCursorPosition(screenInfo, pos, keepCursorVisible, psScrollY); continue; } default: @@ -399,19 +401,19 @@ try if (WI_IsFlagSet(dwFlags, WC_INTERACTIVE) && IS_CONTROL_CHAR(*it)) { const wchar_t wchs[2]{ L'^', static_cast(*it + L'@') }; - TempNumSpaces += _writeCharsLegacyUnprocessed(screenInfo, dwFlags, psScrollY, { &wchs[0], 2 }); + numSpaces += _writeCharsLegacyUnprocessed(screenInfo, dwFlags, psScrollY, { &wchs[0], 2 }); } else { - // TODO test // As a special favor to incompetent apps that attempt to display control chars, // convert to corresponding OEM Glyph Chars const auto cp = ServiceLocator::LocateGlobals().getConsoleInformation().OutputCP; - wchar_t buffer; - const auto result = MultiByteToWideChar(cp, MB_USEGLYPHCHARS, reinterpret_cast(it), 1, &buffer, 1); + const auto ch = gsl::narrow_cast(*it); + wchar_t wch = 0; + const auto result = MultiByteToWideChar(cp, MB_USEGLYPHCHARS, &ch, 1, &wch, 1); if (result == 1) { - TempNumSpaces += _writeCharsLegacyUnprocessed(screenInfo, dwFlags, psScrollY, { &buffer, 1 }); + numSpaces += _writeCharsLegacyUnprocessed(screenInfo, dwFlags, psScrollY, { &wch, 1 }); } } } @@ -419,7 +421,7 @@ try if (pcSpaces) { - *pcSpaces = TempNumSpaces; + *pcSpaces = numSpaces; } return S_OK; From f9840defd9e7f35ff83d9509b4fc2117dc633d41 Mon Sep 17 00:00:00 2001 From: Leonard Hecker Date: Fri, 23 Jun 2023 16:17:21 +0200 Subject: [PATCH 5/9] Fix backspace limit to the start of a prompt --- src/host/_stream.cpp | 28 +++++++++++++++++++++------- 1 file changed, 21 insertions(+), 7 deletions(-) diff --git a/src/host/_stream.cpp b/src/host/_stream.cpp index 3927bea31ac..19146703623 100644 --- a/src/host/_stream.cpp +++ b/src/host/_stream.cpp @@ -165,8 +165,6 @@ try { static constexpr wchar_t tabSpaces[8]{ L' ', L' ', L' ', L' ', L' ', L' ', L' ', L' ' }; - UNREFERENCED_PARAMETER(sOriginalXPosition); - auto& textBuffer = screenInfo.GetTextBuffer(); auto& cursor = textBuffer.GetCursor(); const auto keepCursorVisible = WI_IsFlagSet(dwFlags, WC_KEEP_CURSOR_VISIBLE); @@ -252,8 +250,8 @@ try row.SetDoubleBytePadded(false); }; - // We have to move up early because the tab handling code below needs - // to be on the row of the tab already, so that we can call GetText(). + // We have to move up early because the tab handling code below needs to be on + // the row of the tab already, so that we can call GetText() for precedingText. if (pos.x == 0 && pos.y != 0) { moveUp(); @@ -297,12 +295,28 @@ try // always be at least 1 because that's the \t character in the backup buffer. In other words, // backupLimit will at a minimum be equal to backupEnd, or preced it by 7 more characters. const auto backupLimit = pwchBuffer - std::min(8, pwchBuffer - pwchBufferBackupLimit); - // Now count how many spaces precede the \t character and remove them from the glyphCount. + // Now count how many spaces precede the \t character. "backupEnd - backupBeg" will be the amount. auto backupBeg = backupEnd; for (; backupBeg != backupLimit && backupBeg[-1] == L' '; --backupBeg, --glyphCount) { } - glyphCount -= std::min(glyphCount - 1, gsl::narrow_cast(backupEnd - backupBeg)); + + // There's one final problem: A prompt like... + // fputs("foo: ", stdout); + // fgets(buffer, stdin); + // ...has a trailing whitespace in front of our pwchBufferBackupLimit which we should not backspace over. + // sOriginalXPosition stores the start of the prompt at the pwchBufferBackupLimit. + if (backupBeg == pwchBufferBackupLimit) + { + glyphCount = pos.x - sOriginalXPosition; + } + + // Now that we finally know how many columns precede the cursor we can + // subtract the previously determined amount of ' ' from the '\t'. + glyphCount -= gsl::narrow_cast(backupEnd - backupBeg); + + // Can the above code leave glyphCount <= 0? Let's just not find out! + glyphCount = std::max(1, glyphCount); } // Control chars in interactive mode where previously written out // as ^X for instance, so now we also need to delete 2 glyphs. @@ -337,7 +351,7 @@ try break; } - // Otherwise, in case we need to delete 2 or more glyphs, we need to check it again. + // Otherwise, in case we need to delete 2 or more glyphs, we need to ensure we properly wrap lines back up. if (pos.x == 0 && pos.y != 0) { moveUp(); From 1528c4fd49edeb0f2fcee26944c63f35ee15c2e5 Mon Sep 17 00:00:00 2001 From: Leonard Hecker Date: Sun, 25 Jun 2023 15:24:01 +0200 Subject: [PATCH 6/9] Fix spelling --- src/host/_stream.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/host/_stream.cpp b/src/host/_stream.cpp index 19146703623..6e19152dd9e 100644 --- a/src/host/_stream.cpp +++ b/src/host/_stream.cpp @@ -293,7 +293,7 @@ try // backupLimit points to how far back we need to search. Even if we have 9000 characters in our command line, // we'll only need to check a total of 8 whitespaces. "pwchBuffer - pwchBufferBackupLimit" will // always be at least 1 because that's the \t character in the backup buffer. In other words, - // backupLimit will at a minimum be equal to backupEnd, or preced it by 7 more characters. + // backupLimit will at a minimum be equal to backupEnd, or precede it by 7 more characters. const auto backupLimit = pwchBuffer - std::min(8, pwchBuffer - pwchBufferBackupLimit); // Now count how many spaces precede the \t character. "backupEnd - backupBeg" will be the amount. auto backupBeg = backupEnd; From d982e255e7f527c9791d07ff0c3d6e1a66e765ad Mon Sep 17 00:00:00 2001 From: Leonard Hecker Date: Mon, 26 Jun 2023 00:26:03 +0200 Subject: [PATCH 7/9] Address feedback --- src/buffer/out/Row.cpp | 6 +++++ src/buffer/out/Row.hpp | 1 + src/buffer/out/textBuffer.cpp | 14 +++++----- src/buffer/out/textBuffer.hpp | 3 ++- src/host/_stream.cpp | 36 ++++++++++++++------------ src/terminal/adapter/adaptDispatch.cpp | 2 +- 6 files changed, 36 insertions(+), 26 deletions(-) diff --git a/src/buffer/out/Row.cpp b/src/buffer/out/Row.cpp index de72dec3bf4..93be3886c7f 100644 --- a/src/buffer/out/Row.cpp +++ b/src/buffer/out/Row.cpp @@ -117,6 +117,12 @@ LineRendition ROW::GetLineRendition() const noexcept return _lineRendition; } +uint16_t ROW::GetLineWidth() const noexcept +{ + const auto scale = _lineRendition != LineRendition::SingleWidth ? 1 : 0; + return _columnCount >> scale; +} + // Routine Description: // - Sets all properties of the ROW to default values // Arguments: diff --git a/src/buffer/out/Row.hpp b/src/buffer/out/Row.hpp index 9aed448366a..3733a11ffc7 100644 --- a/src/buffer/out/Row.hpp +++ b/src/buffer/out/Row.hpp @@ -123,6 +123,7 @@ class ROW final bool WasDoubleBytePadded() const noexcept; void SetLineRendition(const LineRendition lineRendition) noexcept; LineRendition GetLineRendition() const noexcept; + uint16_t GetLineWidth() const noexcept; void Reset(const TextAttribute& attr) noexcept; void TransferAttributes(const til::small_rle& attr, til::CoordType newWidth); diff --git a/src/buffer/out/textBuffer.cpp b/src/buffer/out/textBuffer.cpp index 3c189f10d93..f41a1442721 100644 --- a/src/buffer/out/textBuffer.cpp +++ b/src/buffer/out/textBuffer.cpp @@ -450,18 +450,11 @@ void TextBuffer::ConsumeGrapheme(std::wstring_view& chars) noexcept // This function is intended for writing regular "lines" of text as it'll set the wrap flag on the given row. // You can continue calling the function on the same row as long as state.columnEnd < state.columnLimit. -void TextBuffer::WriteLine(til::CoordType row, bool wrapAtEOL, const TextAttribute& attributes, RowWriteState& state) +void TextBuffer::Write(til::CoordType row, const TextAttribute& attributes, RowWriteState& state) { auto& r = GetRowByOffset(row); - r.ReplaceText(state); r.ReplaceAttributes(state.columnBegin, state.columnEnd, attributes); - - if (state.columnEnd >= state.columnLimit) - { - r.SetWrapForced(wrapAtEOL); - } - TriggerRedraw(Viewport::FromExclusive({ state.columnBeginDirty, row, state.columnEndDirty, row + 1 })); } @@ -956,6 +949,11 @@ void TextBuffer::SetCurrentAttributes(const TextAttribute& currentAttributes) no _currentAttributes = currentAttributes; } +void TextBuffer::SetWrapForced(const til::CoordType y, bool wrap) +{ + GetRowByOffset(y).SetWrapForced(wrap); +} + void TextBuffer::SetCurrentLineRendition(const LineRendition lineRendition, const TextAttribute& fillAttributes) { const auto cursorPosition = GetCursor().GetPosition(); diff --git a/src/buffer/out/textBuffer.hpp b/src/buffer/out/textBuffer.hpp index 0950990cedf..5efeaaa741d 100644 --- a/src/buffer/out/textBuffer.hpp +++ b/src/buffer/out/textBuffer.hpp @@ -98,7 +98,7 @@ class TextBuffer final // Text insertion functions static void ConsumeGrapheme(std::wstring_view& chars) noexcept; - void WriteLine(til::CoordType row, bool wrapAtEOL, const TextAttribute& attributes, RowWriteState& state); + void Write(til::CoordType row, const TextAttribute& attributes, RowWriteState& state); void FillRect(const til::rect& rect, const std::wstring_view& fill, const TextAttribute& attributes); OutputCellIterator Write(const OutputCellIterator givenIt); @@ -137,6 +137,7 @@ class TextBuffer final void SetCurrentAttributes(const TextAttribute& currentAttributes) noexcept; + void SetWrapForced(til::CoordType y, bool wrap); void SetCurrentLineRendition(const LineRendition lineRendition, const TextAttribute& fillAttributes); void ResetLineRenditionRange(const til::CoordType startRow, const til::CoordType endRow); LineRendition GetLineRendition(const til::CoordType row) const; diff --git a/src/host/_stream.cpp b/src/host/_stream.cpp index 6e19152dd9e..7c5e87fdd25 100644 --- a/src/host/_stream.cpp +++ b/src/host/_stream.cpp @@ -101,37 +101,41 @@ void AdjustCursorPosition(SCREEN_INFORMATION& screenInfo, _In_ til::point coordC // As the name implies, this writes text without processing its control characters. static size_t _writeCharsLegacyUnprocessed(SCREEN_INFORMATION& screenInfo, const DWORD dwFlags, til::CoordType* const psScrollY, const std::wstring_view& text) { - const auto fKeepCursorVisible = WI_IsFlagSet(dwFlags, WC_KEEP_CURSOR_VISIBLE); - const auto fWrapAtEOL = WI_IsFlagSet(screenInfo.OutputMode, ENABLE_WRAP_AT_EOL_OUTPUT); + const auto keepCursorVisible = WI_IsFlagSet(dwFlags, WC_KEEP_CURSOR_VISIBLE); + const auto wrapAtEOL = WI_IsFlagSet(screenInfo.OutputMode, ENABLE_WRAP_AT_EOL_OUTPUT); const auto hasAccessibilityEventing = screenInfo.HasAccessibilityEventing(); auto& textBuffer = screenInfo.GetTextBuffer(); - auto CursorPosition = textBuffer.GetCursor().GetPosition(); - size_t TempNumSpaces = 0; + size_t numSpaces = 0; RowWriteState state{ .text = text, - .columnLimit = til::CoordTypeMax, + .columnLimit = textBuffer.GetSize().RightExclusive(), }; while (!state.text.empty()) { - state.columnBegin = CursorPosition.x; - textBuffer.WriteLine(CursorPosition.y, fWrapAtEOL, textBuffer.GetCurrentAttributes(), state); - CursorPosition.x = state.columnEnd; + auto cursorPosition = textBuffer.GetCursor().GetPosition(); + + state.columnBegin = cursorPosition.x; + textBuffer.Write(cursorPosition.y, textBuffer.GetCurrentAttributes(), state); + cursorPosition.x = state.columnEnd; - const auto spaces = gsl::narrow_cast(state.columnEnd - state.columnBegin); - TempNumSpaces += spaces; + numSpaces += gsl::narrow_cast(state.columnEnd - state.columnBegin); + + if (wrapAtEOL && state.columnEnd >= state.columnLimit) + { + textBuffer.SetWrapForced(cursorPosition.y, true); + } - if (hasAccessibilityEventing && spaces != 0) + if (hasAccessibilityEventing && state.columnEnd > state.columnBegin) { - screenInfo.NotifyAccessibilityEventing(state.columnBegin, CursorPosition.y, state.columnEnd - 1, CursorPosition.y); + screenInfo.NotifyAccessibilityEventing(state.columnBegin, cursorPosition.y, state.columnEnd - 1, cursorPosition.y); } - AdjustCursorPosition(screenInfo, CursorPosition, fKeepCursorVisible, psScrollY); - CursorPosition = textBuffer.GetCursor().GetPosition(); + AdjustCursorPosition(screenInfo, cursorPosition, keepCursorVisible, psScrollY); } - return TempNumSpaces; + return numSpaces; } // Routine Description: @@ -340,7 +344,7 @@ try .columnBegin = pos.x, .columnLimit = previousColumn, }; - textBuffer.WriteLine(pos.y, wrapAtEOL, textBuffer.GetCurrentAttributes(), state); + textBuffer.Write(pos.y, textBuffer.GetCurrentAttributes(), state); numSpaces -= previousColumn - pos.x; } diff --git a/src/terminal/adapter/adaptDispatch.cpp b/src/terminal/adapter/adaptDispatch.cpp index 180b9276ba8..cc3cddf7dc1 100644 --- a/src/terminal/adapter/adaptDispatch.cpp +++ b/src/terminal/adapter/adaptDispatch.cpp @@ -138,7 +138,7 @@ void AdaptDispatch::_WriteToBuffer(const std::wstring_view string) state.columnBegin = cursorPosition.x; const auto textPositionBefore = state.text.data(); - textBuffer.WriteLine(cursorPosition.y, wrapAtEOL, attributes, state); + textBuffer.Write(cursorPosition.y, attributes, state); const auto textPositionAfter = state.text.data(); if (state.columnBeginDirty != state.columnEndDirty) From 4c87e6c5058e59ae6ae09dc4ac56ae6754f2ce86 Mon Sep 17 00:00:00 2001 From: Leonard Hecker Date: Mon, 26 Jun 2023 12:49:47 +0200 Subject: [PATCH 8/9] Why fix something now, if you can ignore it until later --- src/terminal/adapter/adaptDispatch.cpp | 7 +++++++ 1 file changed, 7 insertions(+) diff --git a/src/terminal/adapter/adaptDispatch.cpp b/src/terminal/adapter/adaptDispatch.cpp index cc3cddf7dc1..16aa57f6d83 100644 --- a/src/terminal/adapter/adaptDispatch.cpp +++ b/src/terminal/adapter/adaptDispatch.cpp @@ -141,6 +141,13 @@ void AdaptDispatch::_WriteToBuffer(const std::wstring_view string) textBuffer.Write(cursorPosition.y, attributes, state); const auto textPositionAfter = state.text.data(); + // TODO: A row should not be marked as wrapped just because we wrote the last column. + // It should be marked whenever we write _past_ it (above, _DoLineFeed call). See GH#15602. + if (wrapAtEOL && state.columnEnd >= state.columnLimit) + { + textBuffer.SetWrapForced(cursorPosition.y, true); + } + if (state.columnBeginDirty != state.columnEndDirty) { const til::rect changedRect{ state.columnBeginDirty, cursorPosition.y, state.columnEndDirty, cursorPosition.y + 1 }; From 109424402c862c551d380d67ad7d732724cca328 Mon Sep 17 00:00:00 2001 From: Leonard Hecker Date: Fri, 30 Jun 2023 15:15:03 +0200 Subject: [PATCH 9/9] Address feedback --- src/host/_stream.cpp | 10 +++++++++- 1 file changed, 9 insertions(+), 1 deletion(-) diff --git a/src/host/_stream.cpp b/src/host/_stream.cpp index 7c5e87fdd25..d85d4e105f8 100644 --- a/src/host/_stream.cpp +++ b/src/host/_stream.cpp @@ -177,6 +177,12 @@ try const auto end = it + *pcb / sizeof(wchar_t); size_t numSpaces = 0; + // In VT mode, when you have a 120-column terminal you can write 120 columns without the cursor wrapping. + // Whenever the cursor is in that 120th column IsDelayedEOLWrap() will return true. I'm not sure why the VT parts + // of the code base store this as a boolean. It's also unclear why we handle this here. The intention is likely + // so that when we exit VT mode and receive a write a potentially stored delayed wrap would still be handled. + // The way this code does it however isn't correct since it handles it like the old console APIs would and + // so writing a newline while being delay wrapped will print 2 newlines. if (cursor.IsDelayedEOLWrap() && wrapAtEOL) { auto pos = cursor.GetPosition(); @@ -190,6 +196,8 @@ try } } + // If ENABLE_PROCESSED_OUTPUT is set we search for C0 control characters and handle them like backspace, tab, etc. + // If it's not set, we can just straight up give everything to _writeCharsLegacyUnprocessed. if (WI_IsFlagClear(screenInfo.OutputMode, ENABLE_PROCESSED_OUTPUT)) { numSpaces += _writeCharsLegacyUnprocessed(screenInfo, dwFlags, psScrollY, { it, end }); @@ -322,7 +330,7 @@ try // Can the above code leave glyphCount <= 0? Let's just not find out! glyphCount = std::max(1, glyphCount); } - // Control chars in interactive mode where previously written out + // Control chars in interactive mode were previously written out // as ^X for instance, so now we also need to delete 2 glyphs. else if (IS_CONTROL_CHAR(lastChar)) {