From f0291c6501bf85bb5484d406b60dc2a68d483ae1 Mon Sep 17 00:00:00 2001 From: Leonard Hecker Date: Fri, 23 Jun 2023 01:24:10 +0200 Subject: [PATCH] Remove boolean success return values from TextBuffer (#15566) I've removed these because it made some of my new code pretty convoluted for now good reason as most of these functions aren't exception safe to begin with. Basically, their boolean status is often just a pretense because they can crash or throw anyways. Furthermore, `WriteCharsLegacy` failed to check the status code returned by `AdjustCursorPosition` in some of its parts too. In the future we should instead probably strive to continue make our legacy code more exception safe. --- 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 0ea79d6649b..2d6b3e7b8d5 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 6e4d8f0062e..1ce38c0668e 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 7f6eaafdc91..fc737e1b26d 100644 --- a/src/buffer/out/textBuffer.cpp +++ b/src/buffer/out/textBuffer.cpp @@ -419,9 +419,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) @@ -437,10 +436,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 @@ -615,53 +613,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: @@ -673,9 +655,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: @@ -714,7 +696,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. @@ -724,7 +706,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) { @@ -732,9 +713,8 @@ bool TextBuffer::IncrementCursor() _SetWrapOnCurrentRow(); // Then move the cursor to a new line - fSuccess = NewlineCursor(); + NewlineCursor(); } - return fSuccess; } //Routine Description: @@ -743,9 +723,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 @@ -759,13 +738,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: @@ -774,7 +748,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). @@ -799,7 +773,6 @@ bool TextBuffer::IncrementCircularBuffer(const TextAttribute& fillAttributes) _firstRow = 0; } } - return true; } //Routine Description: @@ -2414,6 +2387,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(); @@ -2430,7 +2404,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++) @@ -2486,20 +2459,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. @@ -2532,16 +2497,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 @@ -2568,61 +2526,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(); } } } @@ -2655,77 +2610,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 88418c4a760..b98f8d231cd 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; @@ -241,7 +241,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); } } }