Eliminate the AdaptDefaults class (#12390)

## Summary of the Pull Request

The only method that was really needed in the `AdaptDefault` class was
the `PrintString` method, and that could easily be moved into the
`ConGetSet` interface. That lets us get rid of the `AdaptDefault` class
altogether, simplifying the construction of the `AdaptDispatch` class,
and brings us more in line with the `TerminalDispatch` implementation.

## PR Checklist
* [x] Closes #12318
* [x] CLA signed.
* [ ] Tests added/passed
* [ ] Documentation updated.
* [ ] Schema updated.
* [x] I've discussed this with core contributors already. Issue number
where discussion took place: #12318

## Detailed Description of the Pull Request / Additional comments

The `Execute` method was never used at all, so there was no problem with
losing that. But I also noticed there was an equivalent `ExecuteChar`
method in the `ITerminalApi` interface, which also wasn't being used, so
I've removed that too.

Then there was a `Print` method taking a single `wchar_t` parameter, but
that was ultimately implemented as a `PrintString` call anyway, so that
translation could easily be accomplished in the `AdaptDispatch` calling
code, the same way it's done in `TerminalDispatch`.

That left us with the `PrintString` method, which could simply be moved
into the `ConGetSet` interface, which would then more closely match the
`ITerminalApi` interface.

There was also a `GetResult` method, that returned the status of the
last `PrintString`, but that result was never actually checked anywhere.
What I've done now is make the `PrintString` method throw an exception
on failure, and that will be caught and logged in the `StateMachine`.

## Validation Steps Performed

I've started a bash shell in conhost to verify that it still works.
Since almost everything goes through `PrintString` in VT mode, it should
be obvious if something was broken.
This commit is contained in:
James Holderness 2022-02-09 12:04:40 +00:00 committed by GitHub
parent d63ab27c0c
commit f936c4443d
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
20 changed files with 31 additions and 187 deletions

View File

@ -20,7 +20,6 @@ namespace Microsoft::Terminal::Core
ITerminalApi& operator=(ITerminalApi&&) = default; ITerminalApi& operator=(ITerminalApi&&) = default;
virtual void PrintString(std::wstring_view string) = 0; virtual void PrintString(std::wstring_view string) = 0;
virtual void ExecuteChar(wchar_t wch) = 0;
virtual TextAttribute GetTextAttributes() const = 0; virtual TextAttribute GetTextAttributes() const = 0;
virtual void SetTextAttributes(const TextAttribute& attrs) = 0; virtual void SetTextAttributes(const TextAttribute& attrs) = 0;

View File

@ -96,7 +96,6 @@ public:
#pragma region ITerminalApi #pragma region ITerminalApi
// These methods are defined in TerminalApi.cpp // These methods are defined in TerminalApi.cpp
void PrintString(std::wstring_view stringView) override; void PrintString(std::wstring_view stringView) override;
void ExecuteChar(wchar_t wch) override;
TextAttribute GetTextAttributes() const override; TextAttribute GetTextAttributes() const override;
void SetTextAttributes(const TextAttribute& attrs) override; void SetTextAttributes(const TextAttribute& attrs) override;
Microsoft::Console::Types::Viewport GetBufferSize() override; Microsoft::Console::Types::Viewport GetBufferSize() override;

View File

@ -16,11 +16,6 @@ void Terminal::PrintString(std::wstring_view stringView)
_WriteBuffer(stringView); _WriteBuffer(stringView);
} }
void Terminal::ExecuteChar(wchar_t wch)
{
_WriteBuffer({ &wch, 1 });
}
TextAttribute Terminal::GetTextAttributes() const TextAttribute Terminal::GetTextAttributes() const
{ {
return _buffer->GetCurrentAttributes(); return _buffer->GetCurrentAttributes();

View File

@ -19,11 +19,6 @@ TerminalDispatch::TerminalDispatch(ITerminalApi& terminalApi) noexcept :
{ {
} }
void TerminalDispatch::Execute(const wchar_t wchControl)
{
_terminalApi.ExecuteChar(wchControl);
}
void TerminalDispatch::Print(const wchar_t wchPrintable) void TerminalDispatch::Print(const wchar_t wchPrintable)
{ {
_terminalApi.PrintString({ &wchPrintable, 1 }); _terminalApi.PrintString({ &wchPrintable, 1 });

View File

@ -12,7 +12,6 @@ class TerminalDispatch : public Microsoft::Console::VirtualTerminal::TermDispatc
public: public:
TerminalDispatch(::Microsoft::Terminal::Core::ITerminalApi& terminalApi) noexcept; TerminalDispatch(::Microsoft::Terminal::Core::ITerminalApi& terminalApi) noexcept;
void Execute(const wchar_t wchControl) override;
void Print(const wchar_t wchPrintable) override; void Print(const wchar_t wchPrintable) override;
void PrintString(const std::wstring_view string) override; void PrintString(const std::wstring_view string) override;

View File

@ -19,63 +19,18 @@ using Microsoft::Console::Interactivity::ServiceLocator;
using Microsoft::Console::VirtualTerminal::StateMachine; using Microsoft::Console::VirtualTerminal::StateMachine;
using Microsoft::Console::VirtualTerminal::TerminalInput; using Microsoft::Console::VirtualTerminal::TerminalInput;
WriteBuffer::WriteBuffer(_In_ Microsoft::Console::IIoProvider& io) : ConhostInternalGetSet::ConhostInternalGetSet(_In_ IIoProvider& io) :
_io{ io }, _io{ io }
_ntstatus{ STATUS_INVALID_DEVICE_STATE }
{ {
} }
// Routine Description:
// - Handles the print action from the state machine
// Arguments:
// - wch - The character to be printed.
// Return Value:
// - <none>
void WriteBuffer::Print(const wchar_t wch)
{
_DefaultCase(wch);
}
// Routine Description: // Routine Description:
// - Handles the print action from the state machine // - Handles the print action from the state machine
// Arguments: // Arguments:
// - string - The string to be printed. // - string - The string to be printed.
// Return Value: // Return Value:
// - <none> // - <none>
void WriteBuffer::PrintString(const std::wstring_view string) void ConhostInternalGetSet::PrintString(const std::wstring_view string)
{
_DefaultStringCase(string);
}
// Routine Description:
// - Handles the execute action from the state machine
// Arguments:
// - wch - The C0 control character to be executed.
// Return Value:
// - <none>
void WriteBuffer::Execute(const wchar_t wch)
{
_DefaultCase(wch);
}
// Routine Description:
// - Default text editing/printing handler for all characters that were not routed elsewhere by other state machine intercepts.
// Arguments:
// - wch - The character to be processed by our default text editing/printing mechanisms.
// Return Value:
// - <none>
void WriteBuffer::_DefaultCase(const wchar_t wch)
{
_DefaultStringCase({ &wch, 1 });
}
// Routine Description:
// - Default text editing/printing handler for all characters that were not routed elsewhere by other state machine intercepts.
// Arguments:
// - string - The string to be processed by our default text editing/printing mechanisms.
// Return Value:
// - <none>
void WriteBuffer::_DefaultStringCase(const std::wstring_view string)
{ {
size_t dwNumBytes = string.size() * sizeof(wchar_t); size_t dwNumBytes = string.size() * sizeof(wchar_t);
@ -88,21 +43,18 @@ void WriteBuffer::_DefaultStringCase(const std::wstring_view string)
// Defer the cursor drawing while we are iterating the string, for a better performance. // Defer the cursor drawing while we are iterating the string, for a better performance.
// We can not waste time displaying a cursor event when we know more text is coming right behind it. // We can not waste time displaying a cursor event when we know more text is coming right behind it.
cursor.StartDeferDrawing(); cursor.StartDeferDrawing();
_ntstatus = WriteCharsLegacy(_io.GetActiveOutputBuffer(), const auto ntstatus = WriteCharsLegacy(_io.GetActiveOutputBuffer(),
string.data(), string.data(),
string.data(), string.data(),
string.data(), string.data(),
&dwNumBytes, &dwNumBytes,
nullptr, nullptr,
_io.GetActiveOutputBuffer().GetTextBuffer().GetCursor().GetPosition().X, _io.GetActiveOutputBuffer().GetTextBuffer().GetCursor().GetPosition().X,
WC_LIMIT_BACKSPACE | WC_DELAY_EOL_WRAP, WC_LIMIT_BACKSPACE | WC_DELAY_EOL_WRAP,
nullptr); nullptr);
cursor.EndDeferDrawing(); cursor.EndDeferDrawing();
}
ConhostInternalGetSet::ConhostInternalGetSet(_In_ IIoProvider& io) : THROW_IF_NTSTATUS_FAILED(ntstatus);
_io{ io }
{
} }
// Routine Description: // Routine Description:

View File

@ -14,36 +14,11 @@ Author:
#pragma once #pragma once
#include "../terminal/adapter/adaptDefaults.hpp" #include "../terminal/adapter/conGetSet.hpp"
#include "../types/inc/IInputEvent.hpp" #include "../types/inc/IInputEvent.hpp"
#include "../inc/conattrs.hpp" #include "../inc/conattrs.hpp"
#include "IIoProvider.hpp" #include "IIoProvider.hpp"
class SCREEN_INFORMATION;
// The WriteBuffer class provides helpers for writing text into the TextBuffer that is backing a particular console screen buffer.
class WriteBuffer : public Microsoft::Console::VirtualTerminal::AdaptDefaults
{
public:
WriteBuffer(_In_ Microsoft::Console::IIoProvider& io);
// Implement Adapter callbacks for default cases (non-escape sequences)
void Print(const wchar_t wch) override;
void PrintString(const std::wstring_view string) override;
void Execute(const wchar_t wch) override;
[[nodiscard]] NTSTATUS GetResult() { return _ntstatus; };
private:
void _DefaultCase(const wchar_t wch);
void _DefaultStringCase(const std::wstring_view string);
Microsoft::Console::IIoProvider& _io;
NTSTATUS _ntstatus;
};
#include "../terminal/adapter/conGetSet.hpp"
// The ConhostInternalGetSet is for the Conhost process to call the entrypoints for its own Get/Set APIs. // The ConhostInternalGetSet is for the Conhost process to call the entrypoints for its own Get/Set APIs.
// Normally, these APIs are accessible from the outside of the conhost process (like by the process being "hosted") through // Normally, these APIs are accessible from the outside of the conhost process (like by the process being "hosted") through
// the kernelbase/32 exposed public APIs and routed by the console driver (condrv) to this console host. // the kernelbase/32 exposed public APIs and routed by the console driver (condrv) to this console host.
@ -54,6 +29,8 @@ class ConhostInternalGetSet final : public Microsoft::Console::VirtualTerminal::
public: public:
ConhostInternalGetSet(_In_ Microsoft::Console::IIoProvider& io); ConhostInternalGetSet(_In_ Microsoft::Console::IIoProvider& io);
void PrintString(const std::wstring_view string) override;
void GetConsoleScreenBufferInfoEx(CONSOLE_SCREEN_BUFFER_INFOEX& screenBufferInfo) const override; void GetConsoleScreenBufferInfoEx(CONSOLE_SCREEN_BUFFER_INFOEX& screenBufferInfo) const override;
void SetConsoleScreenBufferInfoEx(const CONSOLE_SCREEN_BUFFER_INFOEX& screenBufferInfo) override; void SetConsoleScreenBufferInfoEx(const CONSOLE_SCREEN_BUFFER_INFOEX& screenBufferInfo) override;

View File

@ -255,8 +255,7 @@ void SCREEN_INFORMATION::s_RemoveScreenBuffer(_In_ SCREEN_INFORMATION* const pSc
try try
{ {
auto getset = std::make_unique<ConhostInternalGetSet>(*this); auto getset = std::make_unique<ConhostInternalGetSet>(*this);
auto defaults = std::make_unique<WriteBuffer>(*this); auto adapter = std::make_unique<AdaptDispatch>(std::move(getset));
auto adapter = std::make_unique<AdaptDispatch>(std::move(getset), std::move(defaults));
auto engine = std::make_unique<OutputStateMachineEngine>(std::move(adapter)); auto engine = std::make_unique<OutputStateMachineEngine>(std::move(adapter));
// Note that at this point in the setup, we haven't determined if we're // Note that at this point in the setup, we haven't determined if we're
// in VtIo mode or not yet. We'll set the OutputStateMachine's // in VtIo mode or not yet. We'll set the OutputStateMachine's

View File

@ -29,7 +29,6 @@ public:
#pragma warning(disable : 26432) // suppress rule of 5 violation on interface because tampering with this is fraught with peril #pragma warning(disable : 26432) // suppress rule of 5 violation on interface because tampering with this is fraught with peril
virtual ~ITermDispatch() = 0; virtual ~ITermDispatch() = 0;
virtual void Execute(const wchar_t wchControl) = 0;
virtual void Print(const wchar_t wchPrintable) = 0; virtual void Print(const wchar_t wchPrintable) = 0;
virtual void PrintString(const std::wstring_view string) = 0; virtual void PrintString(const std::wstring_view string) = 0;

View File

@ -1,30 +0,0 @@
/*++
Copyright (c) Microsoft Corporation
Licensed under the MIT license.
Module Name:
- adaptDefaults.hpp
Abstract:
- This serves as an abstraction for the default cases in the state machine (which is to just print or execute a simple single character.
- This can also handle processing of an entire string of printable characters, as an optimization.
- When using the Windows Console API adapter (AdaptDispatch), this must be passed in to signify where standard actions should go.
Author(s):
- Michael Niksa (MiNiksa) 30-July-2015
- Mike Griese (migrie) 07-March-2016
--*/
#pragma once
namespace Microsoft::Console::VirtualTerminal
{
class AdaptDefaults
{
public:
virtual ~AdaptDefaults() = default;
virtual void Print(const wchar_t wch) = 0;
// These characters need to be mutable so that they can be processed by the TerminalInput translater.
virtual void PrintString(const std::wstring_view string) = 0;
virtual void Execute(const wchar_t wch) = 0;
};
}

View File

@ -26,17 +26,14 @@ bool NoOp() noexcept
} }
// Note: AdaptDispatch will take ownership of pConApi and pDefaults // Note: AdaptDispatch will take ownership of pConApi and pDefaults
AdaptDispatch::AdaptDispatch(std::unique_ptr<ConGetSet> pConApi, AdaptDispatch::AdaptDispatch(std::unique_ptr<ConGetSet> pConApi) :
std::unique_ptr<AdaptDefaults> pDefaults) :
_pConApi{ std::move(pConApi) }, _pConApi{ std::move(pConApi) },
_pDefaults{ std::move(pDefaults) },
_usingAltBuffer(false), _usingAltBuffer(false),
_isOriginModeRelative(false), // by default, the DECOM origin mode is absolute. _isOriginModeRelative(false), // by default, the DECOM origin mode is absolute.
_isDECCOLMAllowed(false), // by default, DECCOLM is not allowed. _isDECCOLMAllowed(false), // by default, DECCOLM is not allowed.
_termOutput() _termOutput()
{ {
THROW_HR_IF_NULL(E_INVALIDARG, _pConApi.get()); THROW_HR_IF_NULL(E_INVALIDARG, _pConApi.get());
THROW_HR_IF_NULL(E_INVALIDARG, _pDefaults.get());
_scrollMargins = { 0 }; // initially, there are no scroll margins. _scrollMargins = { 0 }; // initially, there are no scroll margins.
} }
@ -55,7 +52,7 @@ void AdaptDispatch::Print(const wchar_t wchPrintable)
// a character is only output if the DEL is translated to something else. // a character is only output if the DEL is translated to something else.
if (wchTranslated != AsciiChars::DEL) if (wchTranslated != AsciiChars::DEL)
{ {
_pDefaults->Print(wchTranslated); _pConApi->PrintString({ &wchTranslated, 1 });
} }
} }
@ -76,11 +73,11 @@ void AdaptDispatch::PrintString(const std::wstring_view string)
{ {
buffer.push_back(_termOutput.TranslateKey(wch)); buffer.push_back(_termOutput.TranslateKey(wch));
} }
_pDefaults->PrintString(buffer); _pConApi->PrintString(buffer);
} }
else else
{ {
_pDefaults->PrintString(string); _pConApi->PrintString(string);
} }
} }

View File

@ -16,7 +16,6 @@ Author(s):
#include "termDispatch.hpp" #include "termDispatch.hpp"
#include "conGetSet.hpp" #include "conGetSet.hpp"
#include "adaptDefaults.hpp"
#include "FontBuffer.hpp" #include "FontBuffer.hpp"
#include "terminalOutput.hpp" #include "terminalOutput.hpp"
#include "..\..\types\inc\sgrStack.hpp" #include "..\..\types\inc\sgrStack.hpp"
@ -26,16 +25,10 @@ namespace Microsoft::Console::VirtualTerminal
class AdaptDispatch : public ITermDispatch class AdaptDispatch : public ITermDispatch
{ {
public: public:
AdaptDispatch(std::unique_ptr<ConGetSet> pConApi, AdaptDispatch(std::unique_ptr<ConGetSet> pConApi);
std::unique_ptr<AdaptDefaults> pDefaults);
void Execute(const wchar_t wchControl) override
{
_pDefaults->Execute(wchControl);
}
void PrintString(const std::wstring_view string) override;
void Print(const wchar_t wchPrintable) override; void Print(const wchar_t wchPrintable) override;
void PrintString(const std::wstring_view string) override;
bool CursorUp(const size_t distance) override; // CUU bool CursorUp(const size_t distance) override; // CUU
bool CursorDown(const size_t distance) override; // CUD bool CursorDown(const size_t distance) override; // CUD
@ -199,7 +192,6 @@ namespace Microsoft::Console::VirtualTerminal
bool _initDefaultTabStops = true; bool _initDefaultTabStops = true;
std::unique_ptr<ConGetSet> _pConApi; std::unique_ptr<ConGetSet> _pConApi;
std::unique_ptr<AdaptDefaults> _pDefaults;
TerminalOutput _termOutput; TerminalOutput _termOutput;
std::unique_ptr<FontBuffer> _fontBuffer; std::unique_ptr<FontBuffer> _fontBuffer;
std::optional<unsigned int> _initialCodePage; std::optional<unsigned int> _initialCodePage;

View File

@ -34,6 +34,9 @@ namespace Microsoft::Console::VirtualTerminal
public: public:
virtual ~ConGetSet() = default; virtual ~ConGetSet() = default;
virtual void PrintString(const std::wstring_view string) = 0;
virtual void GetConsoleScreenBufferInfoEx(CONSOLE_SCREEN_BUFFER_INFOEX& screenBufferInfo) const = 0; virtual void GetConsoleScreenBufferInfoEx(CONSOLE_SCREEN_BUFFER_INFOEX& screenBufferInfo) const = 0;
virtual void SetConsoleScreenBufferInfoEx(const CONSOLE_SCREEN_BUFFER_INFOEX& screenBufferInfo) = 0; virtual void SetConsoleScreenBufferInfoEx(const CONSOLE_SCREEN_BUFFER_INFOEX& screenBufferInfo) = 0;
virtual void SetCursorPosition(const COORD position) = 0; virtual void SetCursorPosition(const COORD position) = 0;

View File

@ -22,7 +22,6 @@
</ClCompile> </ClCompile>
</ItemGroup> </ItemGroup>
<ItemGroup> <ItemGroup>
<ClInclude Include="..\adaptDefaults.hpp" />
<ClInclude Include="..\adaptDispatch.hpp" /> <ClInclude Include="..\adaptDispatch.hpp" />
<ClInclude Include="..\charsets.hpp" /> <ClInclude Include="..\charsets.hpp" />
<ClInclude Include="..\DispatchTypes.hpp" /> <ClInclude Include="..\DispatchTypes.hpp" />

View File

@ -41,9 +41,6 @@
</ClCompile> </ClCompile>
</ItemGroup> </ItemGroup>
<ItemGroup> <ItemGroup>
<ClInclude Include="..\adaptDefaults.hpp">
<Filter>Header Files</Filter>
</ClInclude>
<ClInclude Include="..\adaptDispatch.hpp"> <ClInclude Include="..\adaptDispatch.hpp">
<Filter>Header Files</Filter> <Filter>Header Files</Filter>
</ClInclude> </ClInclude>

View File

@ -22,7 +22,6 @@ namespace Microsoft::Console::VirtualTerminal
class Microsoft::Console::VirtualTerminal::TermDispatch : public Microsoft::Console::VirtualTerminal::ITermDispatch class Microsoft::Console::VirtualTerminal::TermDispatch : public Microsoft::Console::VirtualTerminal::ITermDispatch
{ {
public: public:
void Execute(const wchar_t wchControl) override = 0;
void Print(const wchar_t wchPrintable) override = 0; void Print(const wchar_t wchPrintable) override = 0;
void PrintString(const std::wstring_view string) override = 0; void PrintString(const std::wstring_view string) override = 0;

View File

@ -58,6 +58,10 @@ using namespace Microsoft::Console::VirtualTerminal;
class TestGetSet final : public ConGetSet class TestGetSet final : public ConGetSet
{ {
public: public:
void PrintString(const std::wstring_view /*string*/) override
{
}
void GetConsoleScreenBufferInfoEx(CONSOLE_SCREEN_BUFFER_INFOEX& sbiex) const override void GetConsoleScreenBufferInfoEx(CONSOLE_SCREEN_BUFFER_INFOEX& sbiex) const override
{ {
Log::Comment(L"GetConsoleScreenBufferInfoEx MOCK returning data..."); Log::Comment(L"GetConsoleScreenBufferInfoEx MOCK returning data...");
@ -649,21 +653,6 @@ private:
HANDLE _hCon; HANDLE _hCon;
}; };
class DummyAdapter : public AdaptDefaults
{
void Print(const wchar_t /*wch*/) override
{
}
void PrintString(const std::wstring_view /*string*/) override
{
}
void Execute(const wchar_t /*wch*/) override
{
}
};
class AdapterTest class AdapterTest
{ {
public: public:
@ -677,11 +666,9 @@ public:
fSuccess = api.get() != nullptr; fSuccess = api.get() != nullptr;
if (fSuccess) if (fSuccess)
{ {
auto adapter = std::make_unique<DummyAdapter>();
// give AdaptDispatch ownership of _testGetSet // give AdaptDispatch ownership of _testGetSet
_testGetSet = api.get(); // keep a copy for us but don't manage its lifetime anymore. _testGetSet = api.get(); // keep a copy for us but don't manage its lifetime anymore.
_pDispatch = std::make_unique<AdaptDispatch>(std::move(api), std::move(adapter)); _pDispatch = std::make_unique<AdaptDispatch>(std::move(api));
fSuccess = _pDispatch != nullptr; fSuccess = _pDispatch != nullptr;
} }
return fSuccess; return fSuccess;

View File

@ -17,8 +17,3 @@ void EchoDispatch::PrintString(const std::wstring_view string)
const std::wstring nullTermString(string); // string_view not guaranteed null terminated, but wprintf needs it. const std::wstring nullTermString(string); // string_view not guaranteed null terminated, but wprintf needs it.
wprintf(L"PrintString: \"%s\" (%zd chars)\r\n", nullTermString.data(), nullTermString.size()); wprintf(L"PrintString: \"%s\" (%zd chars)\r\n", nullTermString.data(), nullTermString.size());
} }
void EchoDispatch::Execute(const wchar_t wchControl)
{
wprintf(L"Execute: 0x%x\r\n", wchControl);
}

View File

@ -14,7 +14,6 @@ namespace Microsoft
public: public:
void Print(const wchar_t wchPrintable) override; void Print(const wchar_t wchPrintable) override;
void PrintString(const std::wstring_view string) override; void PrintString(const std::wstring_view string) override;
void Execute(const wchar_t wchControl) override;
}; };
} }
} }

View File

@ -36,10 +36,6 @@ namespace Microsoft
class DummyDispatch final : public TermDispatch class DummyDispatch final : public TermDispatch
{ {
public: public:
virtual void Execute(const wchar_t /*wchControl*/) override
{
}
virtual void Print(const wchar_t /*wchPrintable*/) override virtual void Print(const wchar_t /*wchPrintable*/) override
{ {
} }
@ -986,10 +982,6 @@ class Microsoft::Console::VirtualTerminal::OutputEngineTest final
class StatefulDispatch final : public TermDispatch class StatefulDispatch final : public TermDispatch
{ {
public: public:
virtual void Execute(const wchar_t /*wchControl*/) override
{
}
virtual void Print(const wchar_t wchPrintable) override virtual void Print(const wchar_t wchPrintable) override
{ {
_printString += wchPrintable; _printString += wchPrintable;