Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Make the terminal parser/adapter and related classes use modern, safe structures #3956

Merged
merged 19 commits into from
Dec 19, 2019
Merged
Show file tree
Hide file tree
Changes from 11 commits
Commits
Show all changes
19 commits
Select commit Hold shift + click to select a range
7461678
Adjust the public methods to the state machine to use more modern con…
miniksa Dec 12, 2019
169bfb3
State Machine, Output State Machine Engine, and Tracing classes use r…
miniksa Dec 13, 2019
64d1496
InputStateMachine and associated fallout
miniksa Dec 13, 2019
fee4b54
Dropping hungarians, using unique_ptr constructors, BOOL to bool, usi…
miniksa Dec 16, 2019
dcf0e8f
Fix logical conversion errors discovered by running test suite.
miniksa Dec 16, 2019
d97a4da
Remove many but probably not all of the std::wstring intermediates be…
miniksa Dec 16, 2019
68bc729
found the other one.
miniksa Dec 17, 2019
e73d516
Fix a few things I found in self-review.
miniksa Dec 17, 2019
3a92383
Merge branch 'master' into dev/miniksa/gardening3
miniksa Dec 17, 2019
a2c234a
Merge branch 'master' into dev/miniksa/gardening3
miniksa Dec 17, 2019
576da99
code formatting run.
miniksa Dec 17, 2019
344a0f7
Some projects leaked into audit that weren't ready yet. Also _SetRgbC…
miniksa Dec 17, 2019
d731486
fix signed/unsigned mismatches in tests that prevented x86 compile.
miniksa Dec 17, 2019
c0fc28a
turn front/back back into array indexes because front/back is confusing.
miniksa Dec 19, 2019
601a449
A few other odds and ends from the PR. Missing comment. Used a ref wh…
miniksa Dec 19, 2019
c7b8cfd
conversion mistake + failure to update test adapter with ref removal.
miniksa Dec 19, 2019
aff93e2
String front for odd deref of iterator in tracing class.
miniksa Dec 19, 2019
ea8e3dc
Use vector of intermediates so we can theoretically have >1 in the fu…
miniksa Dec 19, 2019
51c2b3a
code format
miniksa Dec 19, 2019
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
8 changes: 4 additions & 4 deletions OpenConsole.sln
Original file line number Diff line number Diff line change
Expand Up @@ -352,8 +352,8 @@ Global
{2FD12FBB-1DDB-46D8-B818-1023C624CACA}.Release|x86.Build.0 = Release|Win32
{3AE13314-1939-4DFA-9C14-38CA0834050C}.AuditMode|Any CPU.ActiveCfg = AuditMode|Win32
{3AE13314-1939-4DFA-9C14-38CA0834050C}.AuditMode|ARM64.ActiveCfg = Release|ARM64
{3AE13314-1939-4DFA-9C14-38CA0834050C}.AuditMode|x64.ActiveCfg = Release|x64
{3AE13314-1939-4DFA-9C14-38CA0834050C}.AuditMode|x64.Build.0 = Release|x64
{3AE13314-1939-4DFA-9C14-38CA0834050C}.AuditMode|x64.ActiveCfg = AuditMode|x64
{3AE13314-1939-4DFA-9C14-38CA0834050C}.AuditMode|x64.Build.0 = AuditMode|x64
{3AE13314-1939-4DFA-9C14-38CA0834050C}.AuditMode|x86.ActiveCfg = Release|Win32
{3AE13314-1939-4DFA-9C14-38CA0834050C}.Debug|Any CPU.ActiveCfg = Debug|Win32
{3AE13314-1939-4DFA-9C14-38CA0834050C}.Debug|ARM64.ActiveCfg = Debug|ARM64
Expand Down Expand Up @@ -970,8 +970,8 @@ Global
{CA5CAD1A-C46D-4588-B1C0-40F31AE9100B}.Release|x86.Build.0 = Release|Win32
{CA5CAD1A-ABCD-429C-B551-8562EC954746}.AuditMode|Any CPU.ActiveCfg = AuditMode|Win32
{CA5CAD1A-ABCD-429C-B551-8562EC954746}.AuditMode|ARM64.ActiveCfg = Release|ARM64
{CA5CAD1A-ABCD-429C-B551-8562EC954746}.AuditMode|x64.ActiveCfg = Release|x64
{CA5CAD1A-ABCD-429C-B551-8562EC954746}.AuditMode|x64.Build.0 = Release|x64
{CA5CAD1A-ABCD-429C-B551-8562EC954746}.AuditMode|x64.ActiveCfg = AuditMode|x64
{CA5CAD1A-ABCD-429C-B551-8562EC954746}.AuditMode|x64.Build.0 = AuditMode|x64
{CA5CAD1A-ABCD-429C-B551-8562EC954746}.AuditMode|x86.ActiveCfg = Release|Win32
{CA5CAD1A-ABCD-429C-B551-8562EC954746}.Debug|Any CPU.ActiveCfg = Debug|Win32
{CA5CAD1A-ABCD-429C-B551-8562EC954746}.Debug|ARM64.ActiveCfg = Debug|ARM64
Expand Down
14 changes: 7 additions & 7 deletions src/cascadia/TerminalCore/ITerminalApi.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,7 @@ namespace Microsoft::Terminal::Core
ITerminalApi& operator=(const ITerminalApi&) = default;
ITerminalApi& operator=(ITerminalApi&&) = default;

virtual bool PrintString(std::wstring_view stringView) = 0;
virtual bool PrintString(std::wstring_view string) = 0;
virtual bool ExecuteChar(wchar_t wch) = 0;

virtual bool SetTextToDefaults(bool foreground, bool background) = 0;
Expand All @@ -29,20 +29,20 @@ namespace Microsoft::Terminal::Core
virtual bool SetCursorPosition(short x, short y) = 0;
virtual COORD GetCursorPosition() = 0;

virtual bool DeleteCharacter(const unsigned int uiCount) = 0;
virtual bool InsertCharacter(const unsigned int uiCount) = 0;
virtual bool EraseCharacters(const unsigned int numChars) = 0;
virtual bool DeleteCharacter(const size_t count) = 0;
virtual bool InsertCharacter(const size_t count) = 0;
virtual bool EraseCharacters(const size_t numChars) = 0;
virtual bool EraseInLine(const ::Microsoft::Console::VirtualTerminal::DispatchTypes::EraseType eraseType) = 0;
virtual bool EraseInDisplay(const ::Microsoft::Console::VirtualTerminal::DispatchTypes::EraseType eraseType) = 0;

virtual bool SetWindowTitle(std::wstring_view title) = 0;

virtual bool SetColorTableEntry(const size_t tableIndex, const DWORD dwColor) = 0;
virtual bool SetColorTableEntry(const size_t tableIndex, const DWORD color) = 0;

virtual bool SetCursorStyle(const ::Microsoft::Console::VirtualTerminal::DispatchTypes::CursorStyle cursorStyle) = 0;

virtual bool SetDefaultForeground(const DWORD dwColor) = 0;
virtual bool SetDefaultBackground(const DWORD dwColor) = 0;
virtual bool SetDefaultForeground(const DWORD color) = 0;
virtual bool SetDefaultBackground(const DWORD color) = 0;

protected:
ITerminalApi() = default;
Expand Down
7 changes: 5 additions & 2 deletions src/cascadia/TerminalCore/Terminal.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -50,7 +50,10 @@ Terminal::Terminal() :
_selectionAnchor{ 0, 0 },
_endSelectionPosition{ 0, 0 }
{
_stateMachine = std::make_unique<StateMachine>(new OutputStateMachineEngine(new TerminalDispatch(*this)));
auto dispatch = std::make_unique<TerminalDispatch>(*this);
auto engine = std::make_unique<OutputStateMachineEngine>(std::move(dispatch));

_stateMachine = std::make_unique<StateMachine>(std::move(engine));

auto passAlongInput = [&](std::deque<std::unique_ptr<IInputEvent>>& inEventsToWrite) {
if (!_pfnWriteInput)
Expand Down Expand Up @@ -199,7 +202,7 @@ void Terminal::Write(std::wstring_view stringView)
{
auto lock = LockForWriting();

_stateMachine->ProcessString(stringView.data(), stringView.size());
_stateMachine->ProcessString(stringView);
}

// Method Description:
Expand Down
12 changes: 6 additions & 6 deletions src/cascadia/TerminalCore/Terminal.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -75,16 +75,16 @@ class Microsoft::Terminal::Core::Terminal final :
bool ReverseText(bool reversed) override;
bool SetCursorPosition(short x, short y) override;
COORD GetCursorPosition() override;
bool DeleteCharacter(const unsigned int uiCount) override;
bool InsertCharacter(const unsigned int uiCount) override;
bool EraseCharacters(const unsigned int numChars) override;
bool DeleteCharacter(const size_t count) override;
bool InsertCharacter(const size_t count) override;
bool EraseCharacters(const size_t numChars) override;
bool EraseInLine(const ::Microsoft::Console::VirtualTerminal::DispatchTypes::EraseType eraseType) override;
bool EraseInDisplay(const ::Microsoft::Console::VirtualTerminal::DispatchTypes::EraseType eraseType) override;
bool SetWindowTitle(std::wstring_view title) override;
bool SetColorTableEntry(const size_t tableIndex, const COLORREF dwColor) override;
bool SetColorTableEntry(const size_t tableIndex, const COLORREF color) override;
bool SetCursorStyle(const ::Microsoft::Console::VirtualTerminal::DispatchTypes::CursorStyle cursorStyle) override;
bool SetDefaultForeground(const COLORREF dwColor) override;
bool SetDefaultBackground(const COLORREF dwColor) override;
bool SetDefaultForeground(const COLORREF color) override;
bool SetDefaultBackground(const COLORREF color) override;
#pragma endregion

#pragma region ITerminalInput
Expand Down
40 changes: 20 additions & 20 deletions src/cascadia/TerminalCore/TerminalApi.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -128,19 +128,19 @@ COORD Terminal::GetCursorPosition()
}

// Method Description:
// - deletes uiCount characters starting from the cursor's current position
// - deletes count characters starting from the cursor's current position
// - it moves over the remaining text to 'replace' the deleted text
// - for example, if the buffer looks like this ('|' is the cursor): [abc|def]
// - calling DeleteCharacter(1) will change it to: [abc|ef],
// - i.e. the 'd' gets deleted and the 'ef' gets shifted over 1 space and **retain their previous text attributes**
// Arguments:
// - uiCount, the number of characters to delete
// - count, the number of characters to delete
// Return value:
// - true if succeeded, false otherwise
bool Terminal::DeleteCharacter(const unsigned int uiCount)
bool Terminal::DeleteCharacter(const size_t count)
{
SHORT dist;
if (!SUCCEEDED(UIntToShort(uiCount, &dist)))
if (!SUCCEEDED(SizeTToShort(count, &dist)))
{
return false;
}
Expand Down Expand Up @@ -175,22 +175,22 @@ bool Terminal::DeleteCharacter(const unsigned int uiCount)
}

// Method Description:
// - Inserts uiCount spaces starting from the cursor's current position, moving over the existing text
// - Inserts count spaces starting from the cursor's current position, moving over the existing text
// - for example, if the buffer looks like this ('|' is the cursor): [abc|def]
// - calling InsertCharacter(1) will change it to: [abc| def],
// - i.e. the 'def' gets shifted over 1 space and **retain their previous text attributes**
// Arguments:
// - uiCount, the number of spaces to insert
// - count, the number of spaces to insert
// Return value:
// - true if succeeded, false otherwise
bool Terminal::InsertCharacter(const unsigned int uiCount)
bool Terminal::InsertCharacter(const size_t count)
{
// NOTE: the code below is _extremely_ similar to DeleteCharacter
// We will want to use this same logic and implement a helper function instead
// that does the 'move a region from here to there' operation
// TODO: Github issue #2163
SHORT dist;
if (!SUCCEEDED(UIntToShort(uiCount, &dist)))
if (!SUCCEEDED(SizeTToShort(count, &dist)))
{
return false;
}
Expand Down Expand Up @@ -227,7 +227,7 @@ bool Terminal::InsertCharacter(const unsigned int uiCount)
return true;
}

bool Terminal::EraseCharacters(const unsigned int numChars)
bool Terminal::EraseCharacters(const size_t numChars)
{
const auto absoluteCursorPos = _buffer->GetCursor().GetPosition();
const auto viewport = _GetMutableViewport();
Expand Down Expand Up @@ -374,17 +374,17 @@ bool Terminal::SetWindowTitle(std::wstring_view title)

// Method Description:
// - Updates the value in the colortable at index tableIndex to the new color
// dwColor. dwColor is a COLORREF, format 0x00BBGGRR.
// color. color is a COLORREF, format 0x00BBGGRR.
// Arguments:
// - tableIndex: the index of the color table to update.
// - dwColor: the new COLORREF to use as that color table value.
// - color: the new COLORREF to use as that color table value.
// Return Value:
// - true iff we successfully updated the color table entry.
bool Terminal::SetColorTableEntry(const size_t tableIndex, const COLORREF dwColor)
bool Terminal::SetColorTableEntry(const size_t tableIndex, const COLORREF color)
{
try
{
_colorTable.at(tableIndex) = dwColor;
_colorTable.at(tableIndex) = color;

// Repaint everything - the colors might have changed
_buffer->GetRenderTarget().TriggerRedrawAll();
Expand Down Expand Up @@ -449,12 +449,12 @@ bool Terminal::SetCursorStyle(const DispatchTypes::CursorStyle cursorStyle)
// Method Description:
// - Updates the default foreground color from a COLORREF, format 0x00BBGGRR.
// Arguments:
// - dwColor: the new COLORREF to use as the default foreground color
// - color: the new COLORREF to use as the default foreground color
// Return Value:
// - true
bool Terminal::SetDefaultForeground(const COLORREF dwColor)
bool Terminal::SetDefaultForeground(const COLORREF color)
{
_defaultFg = dwColor;
_defaultFg = color;

// Repaint everything - the colors might have changed
_buffer->GetRenderTarget().TriggerRedrawAll();
Expand All @@ -464,13 +464,13 @@ bool Terminal::SetDefaultForeground(const COLORREF dwColor)
// Method Description:
// - Updates the default background color from a COLORREF, format 0x00BBGGRR.
// Arguments:
// - dwColor: the new COLORREF to use as the default background color
// - color: the new COLORREF to use as the default background color
// Return Value:
// - true
bool Terminal::SetDefaultBackground(const COLORREF dwColor)
bool Terminal::SetDefaultBackground(const COLORREF color)
{
_defaultBg = dwColor;
_pfnBackgroundColorChanged(dwColor);
_defaultBg = color;
_pfnBackgroundColorChanged(color);

// Repaint everything - the colors might have changed
_buffer->GetRenderTarget().TriggerRedrawAll();
Expand Down
66 changes: 33 additions & 33 deletions src/cascadia/TerminalCore/TerminalDispatch.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -25,45 +25,45 @@ void TerminalDispatch::Print(const wchar_t wchPrintable)
_terminalApi.PrintString({ &wchPrintable, 1 });
}

void TerminalDispatch::PrintString(const wchar_t* const rgwch, const size_t cch)
void TerminalDispatch::PrintString(const std::wstring_view string)
{
_terminalApi.PrintString({ rgwch, cch });
_terminalApi.PrintString(string);
}

bool TerminalDispatch::CursorPosition(const unsigned int uiLine,
const unsigned int uiColumn)
bool TerminalDispatch::CursorPosition(const size_t line,
const size_t column)
{
const auto columnInBufferSpace = uiColumn - 1;
const auto lineInBufferSpace = uiLine - 1;
short x = static_cast<short>(uiColumn - 1);
short y = static_cast<short>(uiLine - 1);
const auto columnInBufferSpace = column - 1;
const auto lineInBufferSpace = line - 1;
short x = static_cast<short>(column - 1);
short y = static_cast<short>(line - 1);
return _terminalApi.SetCursorPosition(x, y);
}

bool TerminalDispatch::CursorForward(const unsigned int uiDistance)
bool TerminalDispatch::CursorForward(const size_t distance)
{
const auto cursorPos = _terminalApi.GetCursorPosition();
const COORD newCursorPos{ cursorPos.X + gsl::narrow<short>(uiDistance), cursorPos.Y };
const COORD newCursorPos{ cursorPos.X + gsl::narrow<short>(distance), cursorPos.Y };
return _terminalApi.SetCursorPosition(newCursorPos.X, newCursorPos.Y);
}

bool TerminalDispatch::CursorBackward(const unsigned int uiDistance)
bool TerminalDispatch::CursorBackward(const size_t distance)
{
const auto cursorPos = _terminalApi.GetCursorPosition();
const COORD newCursorPos{ cursorPos.X - gsl::narrow<short>(uiDistance), cursorPos.Y };
const COORD newCursorPos{ cursorPos.X - gsl::narrow<short>(distance), cursorPos.Y };
return _terminalApi.SetCursorPosition(newCursorPos.X, newCursorPos.Y);
}

bool TerminalDispatch::CursorUp(const unsigned int uiDistance)
bool TerminalDispatch::CursorUp(const size_t distance)
{
const auto cursorPos = _terminalApi.GetCursorPosition();
const COORD newCursorPos{ cursorPos.X, cursorPos.Y + gsl::narrow<short>(uiDistance) };
const COORD newCursorPos{ cursorPos.X, cursorPos.Y + gsl::narrow<short>(distance) };
return _terminalApi.SetCursorPosition(newCursorPos.X, newCursorPos.Y);
}

bool TerminalDispatch::EraseCharacters(const unsigned int uiNumChars)
bool TerminalDispatch::EraseCharacters(const size_t numChars)
{
return _terminalApi.EraseCharacters(uiNumChars);
return _terminalApi.EraseCharacters(numChars);
}

bool TerminalDispatch::SetWindowTitle(std::wstring_view title)
Expand All @@ -75,13 +75,13 @@ bool TerminalDispatch::SetWindowTitle(std::wstring_view title)
// - Sets a single entry of the colortable to a new value
// Arguments:
// - tableIndex: The VT color table index
// - dwColor: The new RGB color value to use.
// - color: The new RGB color value to use.
// Return Value:
// True if handled successfully. False otherwise.
bool TerminalDispatch::SetColorTableEntry(const size_t tableIndex,
const DWORD dwColor)
const DWORD color)
{
return _terminalApi.SetColorTableEntry(tableIndex, dwColor);
return _terminalApi.SetColorTableEntry(tableIndex, color);
}

bool TerminalDispatch::SetCursorStyle(const DispatchTypes::CursorStyle cursorStyle)
Expand All @@ -92,23 +92,23 @@ bool TerminalDispatch::SetCursorStyle(const DispatchTypes::CursorStyle cursorSty
// Method Description:
// - Sets the default foreground color to a new value
// Arguments:
// - dwColor: The new RGB color value to use, in 0x00BBGGRR form
// - color: The new RGB color value to use, in 0x00BBGGRR form
// Return Value:
// True if handled successfully. False otherwise.
bool TerminalDispatch::SetDefaultForeground(const DWORD dwColor)
bool TerminalDispatch::SetDefaultForeground(const DWORD color)
{
return _terminalApi.SetDefaultForeground(dwColor);
return _terminalApi.SetDefaultForeground(color);
}

// Method Description:
// - Sets the default background color to a new value
// Arguments:
// - dwColor: The new RGB color value to use, in 0x00BBGGRR form
// - color: The new RGB color value to use, in 0x00BBGGRR form
// Return Value:
// True if handled successfully. False otherwise.
bool TerminalDispatch::SetDefaultBackground(const DWORD dwColor)
bool TerminalDispatch::SetDefaultBackground(const DWORD color)
{
return _terminalApi.SetDefaultBackground(dwColor);
return _terminalApi.SetDefaultBackground(color);
}

// Method Description:
Expand All @@ -123,25 +123,25 @@ bool TerminalDispatch::EraseInLine(const DispatchTypes::EraseType eraseType)
}

// Method Description:
// - Deletes uiCount number of characters starting from where the cursor is currently
// - Deletes count number of characters starting from where the cursor is currently
// Arguments:
// - uiCount, the number of characters to delete
// - count, the number of characters to delete
// Return Value:
// True if handled successfully. False otherwise.
bool TerminalDispatch::DeleteCharacter(const unsigned int uiCount)
bool TerminalDispatch::DeleteCharacter(const size_t count)
{
return _terminalApi.DeleteCharacter(uiCount);
return _terminalApi.DeleteCharacter(count);
}

// Method Description:
// - Adds uiCount number of spaces starting from where the cursor is currently
// - Adds count number of spaces starting from where the cursor is currently
// Arguments:
// - uiCount, the number of spaces to add
// - count, the number of spaces to add
// Return Value:
// True if handled successfully, false otherwise
bool TerminalDispatch::InsertCharacter(const unsigned int uiCount)
bool TerminalDispatch::InsertCharacter(const size_t count)
{
return _terminalApi.InsertCharacter(uiCount);
return _terminalApi.InsertCharacter(count);
}

// Method Description:
Expand Down
Loading