WriteCharsLegacy: Add some notes in comments and rename WC_ECHO (#9605)

This commit clarifies some things inside WriteCharsLegacy by adding
comments and renaming parameter and enum names. It does not change any
logic.

`WC_ECHO` was used extensively to mean only one thing: whether to print
a control character like `\x18` (Ctrl+X>) as `^X`. It's been renamed
to make that abundantly clear.
This commit is contained in:
Dustin L. Howett 2021-03-24 14:26:50 -07:00 committed by GitHub
parent da24f7d939
commit da1e1a693e
No known key found for this signature in database
GPG key ID: 4AEE18F83AFDEB23
8 changed files with 57 additions and 37 deletions

View file

@ -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_ECHO,
WC_DESTRUCTIVE_BACKSPACE | WC_KEEP_CURSOR_VISIBLE | WC_PRINTABLE_CONTROL_CHARS,
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_ECHO,
WC_DESTRUCTIVE_BACKSPACE | WC_KEEP_CURSOR_VISIBLE | WC_PRINTABLE_CONTROL_CHARS,
nullptr));
cookedReadData.ScreenInfo().SetAttributes(realAttributes);
_pop();

View file

@ -317,9 +317,9 @@ constexpr unsigned int LOCAL_BUFFER_SIZE = 100;
// - 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_KEEP_CURSOR_VISIBLE change window origin desirable when hit rt. edge
// WC_ECHO if called by Read (echoing characters)
// WC_DESTRUCTIVE_BACKSPACE backspace overwrites characters.
// 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.
@ -392,9 +392,13 @@ constexpr unsigned int LOCAL_BUFFER_SIZE = 100;
{
#pragma prefast(suppress : 26019, "Buffer is taken in multiples of 2. Validation is ok.")
const wchar_t 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 wchar_t RealUnicodeChar = *pwchRealUnicode;
if (IS_GLYPH_CHAR(RealUnicodeChar) || fUnprocessed)
{
// WCL-NOTE: This operates on a single code unit instead of a whole codepoint. It will mis-measure surrogate pairs.
if (IsGlyphFullWidth(Char))
{
if (i < (LOCAL_BUFFER_SIZE - 1) && XPosition < (coordScreenBufferSize.X - 1))
@ -426,7 +430,7 @@ constexpr unsigned int LOCAL_BUFFER_SIZE = 100;
switch (RealUnicodeChar)
{
case UNICODE_BELL:
if (dwFlags & WC_ECHO)
if (dwFlags & WC_PRINTABLE_CONTROL_CHARS)
{
goto CtrlChar;
}
@ -470,11 +474,13 @@ constexpr unsigned int LOCAL_BUFFER_SIZE = 100;
default:
// if char is ctrl char, write ^char.
if ((dwFlags & WC_ECHO) && (IS_CONTROL_CHAR(RealUnicodeChar)))
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++;
@ -515,6 +521,10 @@ constexpr unsigned int LOCAL_BUFFER_SIZE = 100;
}
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;
}
}
@ -536,6 +546,7 @@ constexpr unsigned int LOCAL_BUFFER_SIZE = 100;
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 > gsl::narrow_cast<size_t>(coordScreenBufferSize.X) - CursorPosition.X)
{
i = gsl::narrow_cast<size_t>(coordScreenBufferSize.X) - CursorPosition.X;
@ -551,6 +562,9 @@ constexpr unsigned int LOCAL_BUFFER_SIZE = 100;
// 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;
// enforce a delayed newline if we're about to pass the end and the WC_DELAY_EOL_WRAP flag is set.
@ -570,6 +584,8 @@ constexpr unsigned int LOCAL_BUFFER_SIZE = 100;
Status = 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!
if (*pcb == BufferSize)
{
if (nullptr != pcSpaces)
@ -582,6 +598,10 @@ constexpr unsigned int LOCAL_BUFFER_SIZE = 100;
}
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.
@ -730,7 +750,7 @@ constexpr unsigned int LOCAL_BUFFER_SIZE = 100;
pwchBuffer + 1 - pwchBufferBackupLimit,
gsl::narrow_cast<size_t>(coordScreenBufferSize.X) - sOriginalXPosition,
sOriginalXPosition,
dwFlags & WC_ECHO))
dwFlags & WC_PRINTABLE_CONTROL_CHARS))
{
CursorPosition.X = coordScreenBufferSize.X - 1;
CursorPosition.Y = (SHORT)(cursor.GetPosition().Y - 1);
@ -898,9 +918,9 @@ constexpr unsigned int LOCAL_BUFFER_SIZE = 100;
// - 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_KEEP_CURSOR_VISIBLE change window origin (viewport) desirable when hit rt. edge
// WC_ECHO if called by Read (echoing characters)
// WC_DESTRUCTIVE_BACKSPACE backspace overwrites characters.
// 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.

View file

@ -57,9 +57,9 @@ Arguments:
bytes written.
NumSpaces - On output, the number of spaces consumed by the written characters.
dwFlags -
WC_DESTRUCTIVE_BACKSPACE backspace overwrites characters.
WC_KEEP_CURSOR_VISIBLE change window origin desirable when hit rt. edge
WC_ECHO if called by Read (echoing characters)
WC_DESTRUCTIVE_BACKSPACE backspace overwrites characters.
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:

View file

@ -256,7 +256,7 @@ void RedrawCommandLine(COOKED_READ_DATA& cookedReadData)
&cookedReadData.BytesRead(),
&cookedReadData.VisibleCharCount(),
cookedReadData.OriginalCursorPosition().X,
WC_DESTRUCTIVE_BACKSPACE | WC_KEEP_CURSOR_VISIBLE | WC_ECHO,
WC_DESTRUCTIVE_BACKSPACE | WC_KEEP_CURSOR_VISIBLE | WC_PRINTABLE_CONTROL_CHARS,
&ScrollY);
FAIL_FAST_IF_NTSTATUS_FAILED(Status);
@ -297,7 +297,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_ECHO,
WC_DESTRUCTIVE_BACKSPACE | WC_KEEP_CURSOR_VISIBLE | WC_PRINTABLE_CONTROL_CHARS,
&ScrollY));
cookedReadData.OriginalCursorPosition().Y += ScrollY;
}
@ -461,7 +461,7 @@ void CommandLine::_processHistoryCycling(COOKED_READ_DATA& cookedReadData,
&cookedReadData.BytesRead(),
&cookedReadData.VisibleCharCount(),
cookedReadData.OriginalCursorPosition().X,
WC_DESTRUCTIVE_BACKSPACE | WC_KEEP_CURSOR_VISIBLE | WC_ECHO,
WC_DESTRUCTIVE_BACKSPACE | WC_KEEP_CURSOR_VISIBLE | WC_PRINTABLE_CONTROL_CHARS,
&ScrollY));
cookedReadData.OriginalCursorPosition().Y += ScrollY;
}
@ -496,7 +496,7 @@ void CommandLine::_setPromptToOldestCommand(COOKED_READ_DATA& cookedReadData)
&cookedReadData.BytesRead(),
&cookedReadData.VisibleCharCount(),
cookedReadData.OriginalCursorPosition().X,
WC_DESTRUCTIVE_BACKSPACE | WC_KEEP_CURSOR_VISIBLE | WC_ECHO,
WC_DESTRUCTIVE_BACKSPACE | WC_KEEP_CURSOR_VISIBLE | WC_PRINTABLE_CONTROL_CHARS,
&ScrollY));
cookedReadData.OriginalCursorPosition().Y += ScrollY;
}
@ -532,7 +532,7 @@ void CommandLine::_setPromptToNewestCommand(COOKED_READ_DATA& cookedReadData)
&cookedReadData.BytesRead(),
&cookedReadData.VisibleCharCount(),
cookedReadData.OriginalCursorPosition().X,
WC_DESTRUCTIVE_BACKSPACE | WC_KEEP_CURSOR_VISIBLE | WC_ECHO,
WC_DESTRUCTIVE_BACKSPACE | WC_KEEP_CURSOR_VISIBLE | WC_PRINTABLE_CONTROL_CHARS,
&ScrollY));
cookedReadData.OriginalCursorPosition().Y += ScrollY;
}
@ -559,7 +559,7 @@ void CommandLine::DeletePromptAfterCursor(COOKED_READ_DATA& cookedReadData) noex
&cookedReadData.BytesRead(),
&cookedReadData.VisibleCharCount(),
cookedReadData.OriginalCursorPosition().X,
WC_DESTRUCTIVE_BACKSPACE | WC_KEEP_CURSOR_VISIBLE | WC_ECHO,
WC_DESTRUCTIVE_BACKSPACE | WC_KEEP_CURSOR_VISIBLE | WC_PRINTABLE_CONTROL_CHARS,
nullptr));
}
}
@ -586,7 +586,7 @@ COORD CommandLine::_deletePromptBeforeCursor(COOKED_READ_DATA& cookedReadData) n
&cookedReadData.BytesRead(),
&cookedReadData.VisibleCharCount(),
cookedReadData.OriginalCursorPosition().X,
WC_DESTRUCTIVE_BACKSPACE | WC_KEEP_CURSOR_VISIBLE | WC_ECHO,
WC_DESTRUCTIVE_BACKSPACE | WC_KEEP_CURSOR_VISIBLE | WC_PRINTABLE_CONTROL_CHARS,
nullptr));
}
return cookedReadData.OriginalCursorPosition();
@ -878,7 +878,7 @@ COORD CommandLine::_moveCursorRight(COOKED_READ_DATA& cookedReadData) noexcept
&CharsToWrite,
&NumSpaces,
cookedReadData.OriginalCursorPosition().X,
WC_DESTRUCTIVE_BACKSPACE | WC_KEEP_CURSOR_VISIBLE | WC_ECHO,
WC_DESTRUCTIVE_BACKSPACE | WC_KEEP_CURSOR_VISIBLE | WC_PRINTABLE_CONTROL_CHARS,
&ScrollY));
cookedReadData.OriginalCursorPosition().Y += ScrollY;
cookedReadData.VisibleCharCount() += NumSpaces;
@ -921,7 +921,7 @@ void CommandLine::_insertCtrlZ(COOKED_READ_DATA& cookedReadData) noexcept
&CharsToWrite,
&NumSpaces,
cookedReadData.OriginalCursorPosition().X,
WC_DESTRUCTIVE_BACKSPACE | WC_KEEP_CURSOR_VISIBLE | WC_ECHO,
WC_DESTRUCTIVE_BACKSPACE | WC_KEEP_CURSOR_VISIBLE | WC_PRINTABLE_CONTROL_CHARS,
&ScrollY));
cookedReadData.OriginalCursorPosition().Y += ScrollY;
cookedReadData.VisibleCharCount() += NumSpaces;
@ -971,7 +971,7 @@ void CommandLine::_fillPromptWithPreviousCommandFragment(COOKED_READ_DATA& cooke
&cchCount,
&NumSpaces,
cookedReadData.OriginalCursorPosition().X,
WC_DESTRUCTIVE_BACKSPACE | WC_KEEP_CURSOR_VISIBLE | WC_ECHO,
WC_DESTRUCTIVE_BACKSPACE | WC_KEEP_CURSOR_VISIBLE | WC_PRINTABLE_CONTROL_CHARS,
&ScrollY));
cookedReadData.OriginalCursorPosition().Y += ScrollY;
cookedReadData.VisibleCharCount() += NumSpaces;
@ -1018,7 +1018,7 @@ COORD CommandLine::_cycleMatchingCommandHistoryToPrompt(COOKED_READ_DATA& cooked
&cookedReadData.BytesRead(),
&cookedReadData.VisibleCharCount(),
cookedReadData.OriginalCursorPosition().X,
WC_DESTRUCTIVE_BACKSPACE | WC_KEEP_CURSOR_VISIBLE | WC_ECHO,
WC_DESTRUCTIVE_BACKSPACE | WC_KEEP_CURSOR_VISIBLE | WC_PRINTABLE_CONTROL_CHARS,
&ScrollY));
cookedReadData.OriginalCursorPosition().Y += ScrollY;
cursorPosition.Y += ScrollY;
@ -1073,7 +1073,7 @@ COORD CommandLine::DeleteFromRightOfCursor(COOKED_READ_DATA& cookedReadData) noe
&cookedReadData.BytesRead(),
&cookedReadData.VisibleCharCount(),
cookedReadData.OriginalCursorPosition().X,
WC_DESTRUCTIVE_BACKSPACE | WC_KEEP_CURSOR_VISIBLE | WC_ECHO,
WC_DESTRUCTIVE_BACKSPACE | WC_KEEP_CURSOR_VISIBLE | WC_PRINTABLE_CONTROL_CHARS,
nullptr));
}

View file

@ -133,7 +133,7 @@ void RedrawCommandLine(COOKED_READ_DATA& cookedReadData);
// Values for WriteChars(), WriteCharsLegacy() dwFlags
#define WC_DESTRUCTIVE_BACKSPACE 0x01
#define WC_KEEP_CURSOR_VISIBLE 0x02
#define WC_ECHO 0x04
#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).

View file

@ -115,7 +115,7 @@ BOOL CheckBisectStringW(_In_reads_bytes_(cBytes) const WCHAR* pwchBuffer,
// - pwchBuffer - Pointer to Unicode string buffer.
// - cWords - Number of Unicode string.
// - cBytes - Number of bisect position by byte counts.
// - fEcho - TRUE if called by Read (echoing characters)
// - fPrintableControlChars - TRUE if control characters are being expanded (to ^X)
// Return Value:
// - TRUE - Bisected character.
// - FALSE - Correctly.
@ -124,7 +124,7 @@ BOOL CheckBisectProcessW(const SCREEN_INFORMATION& ScreenInfo,
_In_ size_t cWords,
_In_ size_t cBytes,
_In_ SHORT sOriginalXPosition,
_In_ BOOL fEcho)
_In_ BOOL fPrintableControlChars)
{
if (WI_IsFlagSet(ScreenInfo.OutputMode, ENABLE_PROCESSED_OUTPUT))
{
@ -162,7 +162,7 @@ BOOL CheckBisectProcessW(const SCREEN_INFORMATION& ScreenInfo,
switch (Char)
{
case UNICODE_BELL:
if (fEcho)
if (fPrintableControlChars)
goto CtrlChar;
break;
case UNICODE_BACKSPACE:
@ -179,7 +179,7 @@ BOOL CheckBisectProcessW(const SCREEN_INFORMATION& ScreenInfo,
break;
}
default:
if (fEcho)
if (fPrintableControlChars)
{
CtrlChar:
if (cBytes < 2)

View file

@ -36,7 +36,7 @@ BOOL CheckBisectProcessW(const SCREEN_INFORMATION& ScreenInfo,
_In_ size_t cWords,
_In_ size_t cBytes,
_In_ SHORT sOriginalXPosition,
_In_ BOOL fEcho);
_In_ BOOL fPrintableControlChars);
int ConvertToOem(const UINT uiCodePage,
_In_reads_(cchSource) const WCHAR* const pwchSource,

View file

@ -533,7 +533,7 @@ bool COOKED_READ_DATA::ProcessInput(const wchar_t wchOrig,
&NumToWrite,
&NumSpaces,
_originalCursorPosition.X,
WC_DESTRUCTIVE_BACKSPACE | WC_KEEP_CURSOR_VISIBLE | WC_ECHO,
WC_DESTRUCTIVE_BACKSPACE | WC_KEEP_CURSOR_VISIBLE | WC_PRINTABLE_CONTROL_CHARS,
&ScrollY);
if (NT_SUCCESS(status))
{
@ -615,7 +615,7 @@ bool COOKED_READ_DATA::ProcessInput(const wchar_t wchOrig,
&NumToWrite,
nullptr,
_originalCursorPosition.X,
WC_DESTRUCTIVE_BACKSPACE | WC_KEEP_CURSOR_VISIBLE | WC_ECHO,
WC_DESTRUCTIVE_BACKSPACE | WC_KEEP_CURSOR_VISIBLE | WC_PRINTABLE_CONTROL_CHARS,
nullptr);
if (!NT_SUCCESS(status))
{
@ -716,7 +716,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_ECHO;
DWORD dwFlags = WC_DESTRUCTIVE_BACKSPACE | WC_PRINTABLE_CONTROL_CHARS;
if (wch == UNICODE_CARRIAGERETURN)
{
dwFlags |= WC_KEEP_CURSOR_VISIBLE;
@ -786,7 +786,7 @@ bool COOKED_READ_DATA::ProcessInput(const wchar_t wchOrig,
&NumToWrite,
nullptr,
_originalCursorPosition.X,
WC_DESTRUCTIVE_BACKSPACE | WC_KEEP_CURSOR_VISIBLE | WC_ECHO,
WC_DESTRUCTIVE_BACKSPACE | WC_KEEP_CURSOR_VISIBLE | WC_PRINTABLE_CONTROL_CHARS,
nullptr);
if (!NT_SUCCESS(status))
{
@ -848,7 +848,7 @@ size_t COOKED_READ_DATA::Write(const std::wstring_view wstr)
&bytesInserted,
&NumSpaces,
OriginalCursorPosition().X,
WC_DESTRUCTIVE_BACKSPACE | WC_KEEP_CURSOR_VISIBLE | WC_ECHO,
WC_DESTRUCTIVE_BACKSPACE | WC_KEEP_CURSOR_VISIBLE | WC_PRINTABLE_CONTROL_CHARS,
&ScrollY));
OriginalCursorPosition().Y += ScrollY;
VisibleCharCount() += NumSpaces;