Skip to content

Commit

Permalink
Address feedback from #17510 (#17645)
Browse files Browse the repository at this point in the history 
* Added/changed comments as mentioned.
* Improved the ugly `resize_and_overwrite` hack into the STL.
* Add `Write` functions for xterm's window API.
* The only reason we needed a move operator for `VtIo::Writer`
  is because we used it in a ternary in `CONSOLE_INFORMATION`.
  Ternaries are like if branches with hidden move assignments.
  Instead, we simply construct each `Writer` in place.
  No ternary = No move = No problems in life.
  The best benefit of this is that this makes calling `GetVtWriter`
  a hundred times cheaper.

Otherwise, I still need to extend a few tests in `VtIoTests`,
but I'm planning to do that later.
  • Loading branch information
@lhecker
lhecker authored Aug 7, 2024
1 parent 7d8455d commit f6a4155
Show file tree
Hide file tree
Showing 10 changed files with 96 additions and 67 deletions.
61 changes: 36 additions & 25 deletions src/host/VtIo.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -161,16 +161,16 @@ bool VtIo::IsUsingVt() const
}

{
auto writer = GetWriter();
Writer writer{ this };

// GH#4999 - Send a sequence to the connected terminal to request
// win32-input-mode from them. This will enable the connected terminal to
// send us full INPUT_RECORDs as input. If the terminal doesn't understand
// this sequence, it'll just ignore it.

writer.WriteUTF8(
"\033[?1004h" // Focus Event Mode
"\033[?9001h" // Win32 Input Mode
"\x1b[?1004h" // Focus Event Mode
"\x1b[?9001h" // Win32 Input Mode
);

// MSFT: 15813316
Expand Down Expand Up @@ -359,15 +359,13 @@ void VtIo::FormatAttributes(std::wstring& target, const TextAttribute& attribute
target.append(bufW, len);
}

VtIo::Writer VtIo::GetWriter() noexcept
{
_corked += 1;
return Writer{ this };
}

VtIo::Writer::Writer(VtIo* io) noexcept :
_io{ io }
{
if (_io)
{
_io->_corked += 1;
}
}

VtIo::Writer::~Writer() noexcept
Expand All @@ -381,21 +379,6 @@ VtIo::Writer::~Writer() noexcept
}
}

VtIo::Writer::Writer(Writer&& other) noexcept :
_io{ std::exchange(other._io, nullptr) }
{
}

VtIo::Writer& VtIo::Writer::operator=(Writer&& other) noexcept
{
if (this != &other)
{
this->~Writer();
_io = std::exchange(other._io, nullptr);
}
return *this;
}

VtIo::Writer::operator bool() const noexcept
{
return _io != nullptr;
Expand Down Expand Up @@ -538,11 +521,19 @@ void VtIo::Writer::WriteUTF16(std::wstring_view str) const
THROW_HR_MSG(E_INVALIDARG, "string too large");
}

// C++23's resize_and_overwrite is too valuable to not use.
// It reduce the CPU overhead by roughly half.
#if !defined(_HAS_CXX23) || !_HAS_CXX23
#define resize_and_overwrite _Resize_and_overwrite
#endif

// NOTE: Throwing inside resize_and_overwrite invokes undefined behavior.
_io->_back._Resize_and_overwrite(totalUTF8Cap, [&](char* buf, const size_t) noexcept {
_io->_back.resize_and_overwrite(totalUTF8Cap, [&](char* buf, const size_t) noexcept {
const auto len = WideCharToMultiByte(CP_UTF8, 0, str.data(), gsl::narrow_cast<int>(incomingUTF16Len), buf + existingUTF8Len, gsl::narrow_cast<int>(incomingUTF8Cap), nullptr, nullptr);
return existingUTF8Len + std::max(0, len);
});

#undef resize_and_overwrite
}

// When DISABLE_NEWLINE_AUTO_RETURN is not set (Bad! Don't do it!) we'll do newline translation for you.
Expand Down Expand Up @@ -637,6 +628,12 @@ void VtIo::Writer::WriteUCS2(wchar_t ch) const

void VtIo::Writer::WriteUCS2StripControlChars(wchar_t ch) const
{
// If any of the values in the buffer are C0 or C1 controls, we need to
// convert them to printable codepoints, otherwise they'll end up being
// evaluated as control characters by the receiving terminal. We use the
// DOS 437 code page for the C0 controls and DEL, and just a `?` for the
// C1 controls, since that's what you would most likely have seen in the
// legacy v1 console with raster fonts.
if (ch < 0x20)
{
static constexpr wchar_t lut[] = {
Expand Down Expand Up @@ -697,6 +694,20 @@ void VtIo::Writer::WriteASB(bool enabled) const
_io->_back.append(&buf[0], std::size(buf) - 1);
}

void VtIo::Writer::WriteWindowVisibility(bool visible) const
{
char buf[] = "\x1b[1t";
buf[2] = visible ? '1' : '2';
_io->_back.append(&buf[0], std::size(buf) - 1);
}

void VtIo::Writer::WriteWindowTitle(std::wstring_view title) const
{
WriteUTF8("\x1b]0;");
WriteUTF16StripControlChars(title);
WriteUTF8("\x1b\\");
}

void VtIo::Writer::WriteAttributes(const TextAttribute& attributes) const
{
FormatAttributes(_io->_back, attributes);
Expand Down
10 changes: 4 additions & 6 deletions src/host/VtIo.hpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -22,8 +22,8 @@ namespace Microsoft::Console::VirtualTerminal

Writer(const Writer&) = delete;
Writer& operator=(const Writer&) = delete;
Writer(Writer&& other) noexcept;
Writer& operator=(Writer&& other) noexcept;
Writer(Writer&& other) = delete;
Writer& operator=(Writer&& other) = delete;

explicit operator bool() const noexcept;

Expand All @@ -41,6 +41,8 @@ namespace Microsoft::Console::VirtualTerminal
void WriteSGR1006(bool enabled) const;
void WriteDECAWM(bool enabled) const;
void WriteASB(bool enabled) const;
void WriteWindowVisibility(bool visible) const;
void WriteWindowTitle(std::wstring_view title) const;
void WriteAttributes(const TextAttribute& attributes) const;
void WriteInfos(til::point target, std::span<const CHAR_INFO> infos) const;

Expand All @@ -60,12 +62,8 @@ namespace Microsoft::Console::VirtualTerminal
bool IsUsingVt() const;
[[nodiscard]] HRESULT StartIfNeeded();

[[nodiscard]] HRESULT SuppressResizeRepaint();
[[nodiscard]] HRESULT SetCursorPosition(const til::point coordCursor);
[[nodiscard]] HRESULT SwitchScreenBuffer(const bool useAltBuffer);
void SendCloseEvent();
void CreatePseudoWindow();
Writer GetWriter() noexcept;

private:
[[nodiscard]] HRESULT _Initialize(const HANDLE InHandle, const HANDLE OutHandle, _In_opt_ const HANDLE SignalHandle);
Expand Down
8 changes: 1 addition & 7 deletions src/host/_output.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -383,13 +383,7 @@ static FillConsoleResult FillConsoleImpl(SCREEN_INFORMATION& screenInfo, FillCon
LockConsole();
const auto unlock = wil::scope_exit([&] { UnlockConsole(); });

// GH#3126 - This is a shim for powershell's `Clear-Host` function. In
// the vintage console, `Clear-Host` is supposed to clear the entire
// buffer. In conpty however, there's no difference between the viewport
// and the entirety of the buffer. We're going to see if this API call
// exactly matched the way we expect powershell to call it. If it does,
// then let's manually emit a ^[[3J to the connected terminal, so that
// their entire buffer will be cleared as well.
// See FillConsoleOutputCharacterWImpl and its identical code.
if (enablePowershellShim)
{
auto& gci = ServiceLocator::LocateGlobals().getConsoleInformation();
Expand Down
7 changes: 6 additions & 1 deletion src/host/_stream.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -347,6 +347,7 @@ void WriteCharsVT(SCREEN_INFORMATION& screenInfo, const std::wstring_view& str)
const auto& injections = stateMachine.GetInjections();
size_t offset = 0;

// DISABLE_NEWLINE_AUTO_RETURN not being set is equivalent to a LF -> CRLF translation.
const auto write = [&](size_t beg, size_t end) {
const auto chunk = til::safe_slice_abs(str, beg, end);
if (WI_IsFlagSet(screenInfo.OutputMode, DISABLE_NEWLINE_AUTO_RETURN))
Expand All @@ -359,15 +360,19 @@ void WriteCharsVT(SCREEN_INFORMATION& screenInfo, const std::wstring_view& str)
}
};

// When we encounter something like a RIS (hard reset), we must re-enable
// modes that we rely on (like the Win32 Input Mode). To do this, the VT
// parser tells us the positions of any such relevant VT sequences.
for (const auto& injection : injections)
{
write(offset, injection.offset);
offset = injection.offset;

static constexpr std::array<std::string_view, 2> mapping{ {
{ "\x1b[?1004h\x1b[?9001h" }, // RIS: Focus Event Mode + Win32 Input Mode
{ "\033[?1004h" } // DECSET_FOCUS: Focus Event Mode
{ "\x1b[?1004h" } // DECSET_FOCUS: Focus Event Mode
} };
static_assert(static_cast<size_t>(InjectionType::Count) == mapping.size(), "you need to update the mapping array");

writer.WriteUTF8(mapping[static_cast<size_t>(injection.type)]);
}
Expand Down
10 changes: 8 additions & 2 deletions src/host/consoleInformation.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -125,12 +125,18 @@ VtIo* CONSOLE_INFORMATION::GetVtIo() noexcept

VtIo::Writer CONSOLE_INFORMATION::GetVtWriter() noexcept
{
return _vtIo.IsUsingVt() ? _vtIo.GetWriter() : VtIo::Writer{};
// If we're not ConPTY, we return an empty writer, which indicates to the caller to do nothing.
const auto ok = _vtIo.IsUsingVt();
return VtIo::Writer{ ok ? &_vtIo : nullptr };
}

VtIo::Writer CONSOLE_INFORMATION::GetVtWriterForBuffer(const SCREEN_INFORMATION* context) noexcept
{
return _vtIo.IsUsingVt() && (pCurrentScreenBuffer == context || pCurrentScreenBuffer == context->GetAltBuffer()) ? _vtIo.GetWriter() : VtIo::Writer{};
// If the given context is not the current screen buffer, we also return an empty writer.
// We check both for equality and the alt buffer, because we may switch between the main/alt
// buffer while processing the input and this method should return a valid writer in both cases.
const auto ok = _vtIo.IsUsingVt() && (pCurrentScreenBuffer == context || pCurrentScreenBuffer == context->GetAltBuffer());
return VtIo::Writer{ ok ? &_vtIo : nullptr };
}

bool CONSOLE_INFORMATION::IsInVtIoMode() const noexcept
Expand Down
4 changes: 1 addition & 3 deletions src/host/getset.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -1631,9 +1631,7 @@ void ApiRoutines::GetConsoleDisplayModeImpl(ULONG& flags) noexcept

if (auto writer = gci.GetVtWriter())
{
writer.WriteUTF8("\x1b]0;");
writer.WriteUTF16StripControlChars(title);
writer.WriteUTF8("\x7");
writer.WriteWindowTitle(title);
writer.Submit();
}

Expand Down
48 changes: 31 additions & 17 deletions src/host/ut_host/VtIoTests.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -85,7 +85,6 @@ class ::Microsoft::Console::VirtualTerminal::VtIoTests
TEST_CLASS_SETUP(ClassSetup)
{
wil::unique_hfile tx;
//std::tie(tx, rx) = createOverlappedPipe(16 * 1024);
THROW_IF_WIN32_BOOL_FALSE(CreatePipe(rx.addressof(), tx.addressof(), nullptr, 16 * 1024));

DWORD mode = PIPE_READMODE_BYTE | PIPE_NOWAIT;
Expand Down Expand Up @@ -142,15 +141,15 @@ class ::Microsoft::Console::VirtualTerminal::VtIoTests

THROW_IF_FAILED(routines.SetConsoleTitleWImpl(
L"foobar"));
expected = "\x1b]0;foobar\a";
expected = "\x1b]0;foobar\x1b\\";
actual = readOutput();
VERIFY_ARE_EQUAL(expected, actual);

THROW_IF_FAILED(routines.SetConsoleTitleWImpl(
L"foo"
"\u0001\u001f"
"bar"));
expected = "\x1b]0;foo☺▼bar\a";
expected = "\x1b]0;foo☺▼bar\x1b\\";
actual = readOutput();
VERIFY_ARE_EQUAL(expected, actual);

Expand All @@ -159,7 +158,7 @@ class ::Microsoft::Console::VirtualTerminal::VtIoTests
"\u0001\u001f"
"bar"
"\u007f\u009f"));
expected = "\x1b]0;foo☺▼bar⌂?\a";
expected = "\x1b]0;foo☺▼bar⌂?\x1b\\";
actual = readOutput();
VERIFY_ARE_EQUAL(expected, actual);
}
Expand Down Expand Up @@ -333,21 +332,29 @@ class ::Microsoft::Console::VirtualTerminal::VtIoTests

TEST_METHOD(FillConsoleOutputAttribute)
{
setupInitialContents();

size_t cellsModified = 0;
std::string_view expected;
std::string_view actual;

// Writing nothing should produce nothing.
THROW_IF_FAILED(routines.FillConsoleOutputAttributeImpl(*screenInfo, red, 0, {}, cellsModified));
THROW_IF_FAILED(routines.FillConsoleOutputAttributeImpl(*screenInfo, red, 0, {}, cellsModified, false));
expected = "";
actual = readOutput();
VERIFY_ARE_EQUAL(0u, cellsModified);
VERIFY_ARE_EQUAL(expected, actual);

// PowerShell uses ScrollConsoleScreenBufferW + FillConsoleOutputCharacterW to
// clear the buffer contents and that gets translated to a clear screen sequence.
// We ignore FillConsoleOutputCharacterW in favor of ScrollConsoleScreenBufferW.
THROW_IF_FAILED(routines.FillConsoleOutputAttributeImpl(*screenInfo, FOREGROUND_BLUE | FOREGROUND_GREEN | FOREGROUND_RED, 8 * 4, {}, cellsModified, true));
expected = "";
actual = readOutput();
VERIFY_ARE_EQUAL(expected, actual);

setupInitialContents();

// Writing at the start of a line.
THROW_IF_FAILED(routines.FillConsoleOutputAttributeImpl(*screenInfo, red, 3, { 0, 0 }, cellsModified));
THROW_IF_FAILED(routines.FillConsoleOutputAttributeImpl(*screenInfo, red, 3, { 0, 0 }, cellsModified, false));
expected =
decsc() //
cup(1, 1) sgr_red("ABa") //
Expand All @@ -357,7 +364,7 @@ class ::Microsoft::Console::VirtualTerminal::VtIoTests
VERIFY_ARE_EQUAL(expected, actual);

// Writing at the end of a line.
THROW_IF_FAILED(routines.FillConsoleOutputAttributeImpl(*screenInfo, red, 3, { 5, 0 }, cellsModified));
THROW_IF_FAILED(routines.FillConsoleOutputAttributeImpl(*screenInfo, red, 3, { 5, 0 }, cellsModified, false));
expected =
decsc() //
cup(1, 6) sgr_red("Dcd") //
Expand All @@ -367,7 +374,7 @@ class ::Microsoft::Console::VirtualTerminal::VtIoTests
VERIFY_ARE_EQUAL(expected, actual);

// Writing across 2 lines.
THROW_IF_FAILED(routines.FillConsoleOutputAttributeImpl(*screenInfo, blu, 8, { 4, 1 }, cellsModified));
THROW_IF_FAILED(routines.FillConsoleOutputAttributeImpl(*screenInfo, blu, 8, { 4, 1 }, cellsModified, false));
expected =
decsc() //
cup(2, 5) sgr_blu("GHgh") //
Expand All @@ -380,20 +387,27 @@ class ::Microsoft::Console::VirtualTerminal::VtIoTests

TEST_METHOD(FillConsoleOutputCharacterW)
{
setupInitialContents();

size_t cellsModified = 0;
std::string_view expected;
std::string_view actual;

// Writing nothing should produce nothing.
THROW_IF_FAILED(routines.FillConsoleOutputCharacterWImpl(*screenInfo, L'a', 0, {}, cellsModified));
THROW_IF_FAILED(routines.FillConsoleOutputCharacterWImpl(*screenInfo, L'a', 0, {}, cellsModified, false));
expected = "";
actual = readOutput();
VERIFY_ARE_EQUAL(expected, actual);

// PowerShell uses ScrollConsoleScreenBufferW + FillConsoleOutputCharacterW to
// clear the buffer contents and that gets translated to a clear screen sequence.
THROW_IF_FAILED(routines.FillConsoleOutputCharacterWImpl(*screenInfo, L' ', 8 * 4, {}, cellsModified, true));
expected = "\x1b[H\x1b[2J\x1b[3J";
actual = readOutput();
VERIFY_ARE_EQUAL(expected, actual);

setupInitialContents();

// Writing at the start of a line.
THROW_IF_FAILED(routines.FillConsoleOutputCharacterWImpl(*screenInfo, L'a', 3, { 0, 0 }, cellsModified));
THROW_IF_FAILED(routines.FillConsoleOutputCharacterWImpl(*screenInfo, L'a', 3, { 0, 0 }, cellsModified, false));
expected =
decsc() //
cup(1, 1) sgr_red("aa") sgr_blu("a") //
Expand All @@ -402,7 +416,7 @@ class ::Microsoft::Console::VirtualTerminal::VtIoTests
VERIFY_ARE_EQUAL(expected, actual);

// Writing at the end of a line.
THROW_IF_FAILED(routines.FillConsoleOutputCharacterWImpl(*screenInfo, L'b', 3, { 5, 0 }, cellsModified));
THROW_IF_FAILED(routines.FillConsoleOutputCharacterWImpl(*screenInfo, L'b', 3, { 5, 0 }, cellsModified, false));
expected =
decsc() //
cup(1, 6) sgr_red("b") sgr_blu("bb") //
Expand All @@ -411,7 +425,7 @@ class ::Microsoft::Console::VirtualTerminal::VtIoTests
VERIFY_ARE_EQUAL(expected, actual);

// Writing across 2 lines.
THROW_IF_FAILED(routines.FillConsoleOutputCharacterWImpl(*screenInfo, L'c', 8, { 4, 1 }, cellsModified));
THROW_IF_FAILED(routines.FillConsoleOutputCharacterWImpl(*screenInfo, L'c', 8, { 4, 1 }, cellsModified, false));
expected =
decsc() //
cup(2, 5) sgr_red("cc") sgr_blu("cc") //
Expand All @@ -421,7 +435,7 @@ class ::Microsoft::Console::VirtualTerminal::VtIoTests
VERIFY_ARE_EQUAL(expected, actual);

// Writing 3 wide chars while intersecting the last column.
THROW_IF_FAILED(routines.FillConsoleOutputCharacterWImpl(*screenInfo, L'✨', 3, { 5, 1 }, cellsModified));
THROW_IF_FAILED(routines.FillConsoleOutputCharacterWImpl(*screenInfo, L'✨', 3, { 5, 1 }, cellsModified, false));
expected =
decsc() //
cup(2, 6) sgr_red("✨") sgr_blu(" ") //
Expand Down
4 changes: 1 addition & 3 deletions src/interactivity/base/InteractivityFactory.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -499,9 +499,7 @@ void InteractivityFactory::_WritePseudoWindowCallback(bool showOrHide)
auto& gci = ServiceLocator::LocateGlobals().getConsoleInformation();
if (auto writer = gci.GetVtWriter())
{
char buf[] = "\x1b[1t";
buf[2] = showOrHide ? '1' : '2';
writer.WriteUTF8(buf);
writer.WriteWindowVisibility(showOrHide);
writer.Submit();
}
}
Expand Down
Loading

0 comments on commit f6a4155

Please sign in to comment.