Skip to content

Commit f0291c6

Browse files
lheckerlhecker
authored
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.
1 parent e594d97 commit f0291c6

File tree

10 files changed

+178
-269
lines changed

10 files changed

+178
-269
lines changed

‎src/buffer/out/Row.cpp

+1-2
Original file line numberDiff line numberDiff line change
@@ -435,10 +435,9 @@ OutputCellIterator ROW::WriteCells(OutputCellIterator it, const til::CoordType c
435435
return it;
436436
}
437437

438-
bool ROW::SetAttrToEnd(const til::CoordType columnBegin, const TextAttribute attr)
438+
void ROW::SetAttrToEnd(const til::CoordType columnBegin, const TextAttribute attr)
439439
{
440440
_attr.replace(_clampedColumnInclusive(columnBegin), _attr.size(), attr);
441-
return true;
442441
}
443442

444443
void ROW::ReplaceAttributes(const til::CoordType beginIndex, const til::CoordType endIndex, const TextAttribute& newAttr)

‎src/buffer/out/Row.hpp

+1-1
Original file line numberDiff line numberDiff line change
@@ -133,7 +133,7 @@ class ROW final
133133

134134
void ClearCell(til::CoordType column);
135135
OutputCellIterator WriteCells(OutputCellIterator it, til::CoordType columnBegin, std::optional<bool> wrap = std::nullopt, std::optional<til::CoordType> limitRight = std::nullopt);
136-
bool SetAttrToEnd(til::CoordType columnBegin, TextAttribute attr);
136+
void SetAttrToEnd(til::CoordType columnBegin, TextAttribute attr);
137137
void ReplaceAttributes(til::CoordType beginIndex, til::CoordType endIndex, const TextAttribute& newAttr);
138138
void ReplaceCharacters(til::CoordType columnBegin, til::CoordType width, const std::wstring_view& chars);
139139
void ReplaceText(RowWriteState& state);

‎src/buffer/out/textBuffer.cpp

+128-189
Large diffs are not rendered by default.

‎src/buffer/out/textBuffer.hpp

+6-6
Original file line numberDiff line numberDiff line change
@@ -112,13 +112,13 @@ class TextBuffer final
112112
const std::optional<bool> setWrap = std::nullopt,
113113
const std::optional<til::CoordType> limitRight = std::nullopt);
114114

115-
bool InsertCharacter(const wchar_t wch, const DbcsAttribute dbcsAttribute, const TextAttribute attr);
116-
bool InsertCharacter(const std::wstring_view chars, const DbcsAttribute dbcsAttribute, const TextAttribute attr);
117-
bool IncrementCursor();
118-
bool NewlineCursor();
115+
void InsertCharacter(const wchar_t wch, const DbcsAttribute dbcsAttribute, const TextAttribute attr);
116+
void InsertCharacter(const std::wstring_view chars, const DbcsAttribute dbcsAttribute, const TextAttribute attr);
117+
void IncrementCursor();
118+
void NewlineCursor();
119119

120120
// Scroll needs access to this to quickly rotate around the buffer.
121-
bool IncrementCircularBuffer(const TextAttribute& fillAttributes = {});
121+
void IncrementCircularBuffer(const TextAttribute& fillAttributes = {});
122122

123123
til::point GetLastNonSpaceCharacter(std::optional<const Microsoft::Console::Types::Viewport> viewOptional = std::nullopt) const;
124124

@@ -241,7 +241,7 @@ class TextBuffer final
241241
void _SetWrapOnCurrentRow();
242242
void _AdjustWrapOnCurrentRow(const bool fSet);
243243
// Assist with maintaining proper buffer state for Double Byte character sequences
244-
bool _PrepareForDoubleByteSequence(const DbcsAttribute dbcsAttribute);
244+
void _PrepareForDoubleByteSequence(const DbcsAttribute dbcsAttribute);
245245
bool _AssertValidDoubleByteSequence(const DbcsAttribute dbcsAttribute);
246246
void _ExpandTextRow(til::inclusive_rect& selectionRow) const;
247247
DelimiterClass _GetDelimiterClassAt(const til::point pos, const std::wstring_view wordDelimiters) const;

‎src/host/_stream.cpp

+21-37
Original file line numberDiff line numberDiff line change
@@ -39,10 +39,7 @@ using Microsoft::Console::VirtualTerminal::StateMachine;
3939
// - coordCursor - New location of cursor.
4040
// - fKeepCursorVisible - TRUE if changing window origin desirable when hit right edge
4141
// Return Value:
42-
[[nodiscard]] NTSTATUS AdjustCursorPosition(SCREEN_INFORMATION& screenInfo,
43-
_In_ til::point coordCursor,
44-
const BOOL fKeepCursorVisible,
45-
_Inout_opt_ til::CoordType* psScrollY)
42+
void AdjustCursorPosition(SCREEN_INFORMATION& screenInfo, _In_ til::point coordCursor, const BOOL fKeepCursorVisible, _Inout_opt_ til::CoordType* psScrollY)
4643
{
4744
const auto bufferSize = screenInfo.GetBufferSize().Dimensions();
4845
if (coordCursor.x < 0)
@@ -71,16 +68,10 @@ using Microsoft::Console::VirtualTerminal::StateMachine;
7168
}
7269
}
7370

74-
auto Status = STATUS_SUCCESS;
75-
7671
if (coordCursor.y >= bufferSize.height)
7772
{
7873
// At the end of the buffer. Scroll contents of screen buffer so new position is visible.
79-
FAIL_FAST_IF(!(coordCursor.y == bufferSize.height));
80-
if (!StreamScrollRegion(screenInfo))
81-
{
82-
Status = STATUS_NO_MEMORY;
83-
}
74+
StreamScrollRegion(screenInfo);
8475

8576
if (nullptr != psScrollY)
8677
{
@@ -90,28 +81,21 @@ using Microsoft::Console::VirtualTerminal::StateMachine;
9081
}
9182

9283
const auto cursorMovedPastViewport = coordCursor.y > screenInfo.GetViewport().BottomInclusive();
93-
if (SUCCEEDED_NTSTATUS(Status))
84+
85+
// if at right or bottom edge of window, scroll right or down one char.
86+
if (cursorMovedPastViewport)
9487
{
95-
// if at right or bottom edge of window, scroll right or down one char.
96-
if (cursorMovedPastViewport)
97-
{
98-
til::point WindowOrigin;
99-
WindowOrigin.x = 0;
100-
WindowOrigin.y = coordCursor.y - screenInfo.GetViewport().BottomInclusive();
101-
Status = screenInfo.SetViewportOrigin(false, WindowOrigin, true);
102-
}
88+
til::point WindowOrigin;
89+
WindowOrigin.x = 0;
90+
WindowOrigin.y = coordCursor.y - screenInfo.GetViewport().BottomInclusive();
91+
LOG_IF_FAILED(screenInfo.SetViewportOrigin(false, WindowOrigin, true));
10392
}
10493

105-
if (SUCCEEDED_NTSTATUS(Status))
94+
if (fKeepCursorVisible)
10695
{
107-
if (fKeepCursorVisible)
108-
{
109-
screenInfo.MakeCursorVisible(coordCursor);
110-
}
111-
Status = screenInfo.SetCursorPosition(coordCursor, !!fKeepCursorVisible);
96+
screenInfo.MakeCursorVisible(coordCursor);
11297
}
113-
114-
return Status;
98+
LOG_IF_FAILED(screenInfo.SetCursorPosition(coordCursor, !!fKeepCursorVisible));
11599
}
116100

117101
// Routine Description:
@@ -180,7 +164,7 @@ using Microsoft::Console::VirtualTerminal::StateMachine;
180164
CursorPosition.x = 0;
181165
CursorPosition.y++;
182166

183-
Status = AdjustCursorPosition(screenInfo, CursorPosition, WI_IsFlagSet(dwFlags, WC_KEEP_CURSOR_VISIBLE), psScrollY);
167+
AdjustCursorPosition(screenInfo, CursorPosition, WI_IsFlagSet(dwFlags, WC_KEEP_CURSOR_VISIBLE), psScrollY);
184168

185169
CursorPosition = cursor.GetPosition();
186170
}
@@ -372,7 +356,7 @@ using Microsoft::Console::VirtualTerminal::StateMachine;
372356
// WCL-NOTE: wrong place (typically inside another character).
373357
CursorPosition.x = XPosition;
374358

375-
Status = AdjustCursorPosition(screenInfo, CursorPosition, WI_IsFlagSet(dwFlags, WC_KEEP_CURSOR_VISIBLE), psScrollY);
359+
AdjustCursorPosition(screenInfo, CursorPosition, WI_IsFlagSet(dwFlags, WC_KEEP_CURSOR_VISIBLE), psScrollY);
376360

377361
// WCL-NOTE: If we have processed the entire input string during our "fast one-line print" handler,
378362
// WCL-NOTE: we are done as there is nothing more to do. Neat!
@@ -501,7 +485,7 @@ using Microsoft::Console::VirtualTerminal::StateMachine;
501485
CursorPosition.x -= 1;
502486
TempNumSpaces -= 1;
503487

504-
Status = AdjustCursorPosition(screenInfo, CursorPosition, dwFlags & WC_KEEP_CURSOR_VISIBLE, psScrollY);
488+
AdjustCursorPosition(screenInfo, CursorPosition, dwFlags & WC_KEEP_CURSOR_VISIBLE, psScrollY);
505489
if (dwFlags & WC_DESTRUCTIVE_BACKSPACE)
506490
{
507491
try
@@ -523,7 +507,7 @@ using Microsoft::Console::VirtualTerminal::StateMachine;
523507
CursorPosition.x = 0;
524508
OutputDebugStringA(("CONSRV: Ignoring backspace to previous line\n"));
525509
}
526-
Status = AdjustCursorPosition(screenInfo, CursorPosition, (dwFlags & WC_KEEP_CURSOR_VISIBLE) != 0, psScrollY);
510+
AdjustCursorPosition(screenInfo, CursorPosition, (dwFlags & WC_KEEP_CURSOR_VISIBLE) != 0, psScrollY);
527511
if (dwFlags & WC_DESTRUCTIVE_BACKSPACE)
528512
{
529513
try
@@ -548,7 +532,7 @@ using Microsoft::Console::VirtualTerminal::StateMachine;
548532
// on the prev row if it was set
549533
textBuffer.GetRowByOffset(CursorPosition.y).SetWrapForced(false);
550534

551-
Status = AdjustCursorPosition(screenInfo, CursorPosition, dwFlags & WC_KEEP_CURSOR_VISIBLE, psScrollY);
535+
AdjustCursorPosition(screenInfo, CursorPosition, dwFlags & WC_KEEP_CURSOR_VISIBLE, psScrollY);
552536
}
553537
}
554538
// Notify accessibility to read the backspaced character.
@@ -598,7 +582,7 @@ using Microsoft::Console::VirtualTerminal::StateMachine;
598582
}
599583
CATCH_LOG();
600584

601-
Status = AdjustCursorPosition(screenInfo, CursorPosition, (dwFlags & WC_KEEP_CURSOR_VISIBLE) != 0, psScrollY);
585+
AdjustCursorPosition(screenInfo, CursorPosition, (dwFlags & WC_KEEP_CURSOR_VISIBLE) != 0, psScrollY);
602586
break;
603587
}
604588
case UNICODE_CARRIAGERETURN:
@@ -609,7 +593,7 @@ using Microsoft::Console::VirtualTerminal::StateMachine;
609593
pwchBuffer++;
610594
CursorPosition.x = 0;
611595
CursorPosition.y = cursor.GetPosition().y;
612-
Status = AdjustCursorPosition(screenInfo, CursorPosition, (dwFlags & WC_KEEP_CURSOR_VISIBLE) != 0, psScrollY);
596+
AdjustCursorPosition(screenInfo, CursorPosition, (dwFlags & WC_KEEP_CURSOR_VISIBLE) != 0, psScrollY);
613597
break;
614598
}
615599
case UNICODE_LINEFEED:
@@ -632,7 +616,7 @@ using Microsoft::Console::VirtualTerminal::StateMachine;
632616
textBuffer.GetRowByOffset(cursor.GetPosition().y).SetWrapForced(false);
633617
}
634618

635-
Status = AdjustCursorPosition(screenInfo, CursorPosition, (dwFlags & WC_KEEP_CURSOR_VISIBLE) != 0, psScrollY);
619+
AdjustCursorPosition(screenInfo, CursorPosition, (dwFlags & WC_KEEP_CURSOR_VISIBLE) != 0, psScrollY);
636620
break;
637621
}
638622
default:
@@ -678,7 +662,7 @@ using Microsoft::Console::VirtualTerminal::StateMachine;
678662
// is too wide to fit on the current line).
679663
Row.SetDoubleBytePadded(true);
680664

681-
Status = AdjustCursorPosition(screenInfo, CursorPosition, dwFlags & WC_KEEP_CURSOR_VISIBLE, psScrollY);
665+
AdjustCursorPosition(screenInfo, CursorPosition, dwFlags & WC_KEEP_CURSOR_VISIBLE, psScrollY);
682666
continue;
683667
}
684668
break;

‎src/host/_stream.h

+1-4
Original file line numberDiff line numberDiff line change
@@ -35,10 +35,7 @@ Routine Description:
3535
3636
Return Value:
3737
--*/
38-
[[nodiscard]] NTSTATUS AdjustCursorPosition(SCREEN_INFORMATION& screenInfo,
39-
_In_ til::point coordCursor,
40-
const BOOL fKeepCursorVisible,
41-
_Inout_opt_ til::CoordType* psScrollY);
38+
void AdjustCursorPosition(SCREEN_INFORMATION& screenInfo, _In_ til::point coordCursor, const BOOL fKeepCursorVisible, _Inout_opt_ til::CoordType* psScrollY);
4239

4340
/*++
4441
Routine Description:

‎src/host/cmdline.cpp

+2-4
Original file line numberDiff line numberDiff line change
@@ -268,8 +268,7 @@ void RedrawCommandLine(COOKED_READ_DATA& cookedReadData)
268268
{
269269
CursorPosition.x++;
270270
}
271-
Status = AdjustCursorPosition(cookedReadData.ScreenInfo(), CursorPosition, TRUE, nullptr);
272-
FAIL_FAST_IF_NTSTATUS_FAILED(Status);
271+
AdjustCursorPosition(cookedReadData.ScreenInfo(), CursorPosition, TRUE, nullptr);
273272
}
274273
}
275274

@@ -1298,8 +1297,7 @@ til::point CommandLine::DeleteFromRightOfCursor(COOKED_READ_DATA& cookedReadData
12981297

12991298
if (UpdateCursorPosition && cookedReadData.IsEchoInput())
13001299
{
1301-
Status = AdjustCursorPosition(cookedReadData.ScreenInfo(), cursorPosition, true, nullptr);
1302-
FAIL_FAST_IF_NTSTATUS_FAILED(Status);
1300+
AdjustCursorPosition(cookedReadData.ScreenInfo(), cursorPosition, true, nullptr);
13031301
}
13041302

13051303
return STATUS_SUCCESS;

‎src/host/output.cpp

+16-19
Original file line numberDiff line numberDiff line change
@@ -299,33 +299,30 @@ static void _ScrollScreen(SCREEN_INFORMATION& screenInfo, const Viewport& source
299299
// - screenInfo - reference to screen buffer info.
300300
// Return Value:
301301
// - true if we succeeded in scrolling the buffer, otherwise false (if we're out of memory)
302-
bool StreamScrollRegion(SCREEN_INFORMATION& screenInfo)
302+
void StreamScrollRegion(SCREEN_INFORMATION& screenInfo)
303303
{
304304
// Rotate the circular buffer around and wipe out the previous final line.
305305
auto& buffer = screenInfo.GetTextBuffer();
306-
auto fSuccess = buffer.IncrementCircularBuffer(buffer.GetCurrentAttributes());
307-
if (fSuccess)
306+
buffer.IncrementCircularBuffer(buffer.GetCurrentAttributes());
307+
308+
// Trigger a graphical update if we're active.
309+
if (screenInfo.IsActiveScreenBuffer())
308310
{
309-
// Trigger a graphical update if we're active.
310-
if (screenInfo.IsActiveScreenBuffer())
311-
{
312-
til::point coordDelta;
313-
coordDelta.y = -1;
311+
til::point coordDelta;
312+
coordDelta.y = -1;
314313

315-
auto pNotifier = ServiceLocator::LocateAccessibilityNotifier();
316-
if (pNotifier)
317-
{
318-
// Notify accessibility that a scroll has occurred.
319-
pNotifier->NotifyConsoleUpdateScrollEvent(coordDelta.x, coordDelta.y);
320-
}
314+
auto pNotifier = ServiceLocator::LocateAccessibilityNotifier();
315+
if (pNotifier)
316+
{
317+
// Notify accessibility that a scroll has occurred.
318+
pNotifier->NotifyConsoleUpdateScrollEvent(coordDelta.x, coordDelta.y);
319+
}
321320

322-
if (ServiceLocator::LocateGlobals().pRender != nullptr)
323-
{
324-
ServiceLocator::LocateGlobals().pRender->TriggerScroll(&coordDelta);
325-
}
321+
if (ServiceLocator::LocateGlobals().pRender != nullptr)
322+
{
323+
ServiceLocator::LocateGlobals().pRender->TriggerScroll(&coordDelta);
326324
}
327325
}
328-
return fSuccess;
329326
}
330327

331328
// Routine Description:

‎src/host/output.h

+1-1
Original file line numberDiff line numberDiff line change
@@ -47,7 +47,7 @@ void ScrollRegion(SCREEN_INFORMATION& screenInfo,
4747

4848
VOID SetConsoleWindowOwner(const HWND hwnd, _Inout_opt_ ConsoleProcessHandle* pProcessData);
4949

50-
bool StreamScrollRegion(SCREEN_INFORMATION& screenInfo);
50+
void StreamScrollRegion(SCREEN_INFORMATION& screenInfo);
5151

5252
// For handling process handle state, not the window state itself.
5353
void CloseConsoleProcessState();

‎src/host/readDataCooked.cpp

+1-6
Original file line numberDiff line numberDiff line change
@@ -757,12 +757,7 @@ bool COOKED_READ_DATA::ProcessInput(const wchar_t wchOrig,
757757
// adjust cursor position for WriteChars
758758
_originalCursorPosition.y += ScrollY;
759759
CursorPosition.y += ScrollY;
760-
status = AdjustCursorPosition(_screenInfo, CursorPosition, TRUE, nullptr);
761-
if (FAILED_NTSTATUS(status))
762-
{
763-
_bytesRead = 0;
764-
return true;
765-
}
760+
AdjustCursorPosition(_screenInfo, CursorPosition, TRUE, nullptr);
766761
}
767762
}
768763
}

0 commit comments

Comments
 (0)