Address feedback from #17510

src/host/VtIo.cpp

@@ -161,7 +161,7 @@ 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
@@ -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
@@ -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;
@@ -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.
@@ -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[] = {

src/host/VtIo.hpp

@@ -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;
 
@@ -60,12 +60,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);

src/host/_output.cpp

@@ -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();

src/host/_stream.cpp

@@ -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))
@@ -359,6 +360,9 @@ 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);
@@ -368,6 +372,7 @@ void WriteCharsVT(SCREEN_INFORMATION& screenInfo, const std::wstring_view& str)
                 { "\x1b[?1004h\x1b[?9001h" }, // RIS: Focus Event Mode + Win32 Input Mode
                 { "\033[?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)]);
         }

src/host/consoleInformation.cpp

@@ -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

src/host/ut_host/VtIoTests.cpp

@@ -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;

src/terminal/adapter/adaptDispatch.cpp

@@ -1998,6 +1998,7 @@ bool AdaptDispatch::_ModeParamsHelper(const DispatchTypes::ModeParams param, con
         return true;
     case DispatchTypes::ModeParams::FOCUS_EVENT_MODE:
         _terminalInput.SetInputMode(TerminalInput::Mode::FocusEvent, enable);
+        // ConPTY always wants to know about focus events, so let it know that it needs to re-enable this mode.
         _api.GetStateMachine().InjectSequence(InjectionType::DECSET_FOCUS);
         return true;
     case DispatchTypes::ModeParams::ALTERNATE_SCROLL:
@@ -3241,6 +3242,8 @@ bool AdaptDispatch::HardReset()
         _macroBuffer = nullptr;
     }
 
+    // A hard reset will disable all the modes that ConPTY relies on,
+    // so let it know that it needs to re-enable those modes.
     _api.GetStateMachine().InjectSequence(InjectionType::RIS);
     return true;
 }
@@ -3346,9 +3349,6 @@ bool AdaptDispatch::_EraseAll()
         _api.NotifyBufferRotation(delta);
         newPageTop -= delta;
         newPageBottom -= delta;
-        // We don't want to trigger a scroll in pty mode, because we're going to
-        // pass through the ED sequence anyway, and this will just result in the
-        // buffer being scrolled up by two pages instead of one.
         textBuffer.TriggerScroll({ 0, -delta });
     }
     // Move the viewport if necessary.

src/terminal/parser/stateMachine.hpp

@@ -37,10 +37,15 @@ namespace Microsoft::Console::VirtualTerminal
     // the their indexes.
     static_assert(MAX_PARAMETER_COUNT * MAX_SUBPARAMETER_COUNT <= 256);
 
+    // When we encounter something like a RIS (hard reset), ConPTY must re-enable
+    // modes that it relies on (like the Win32 Input Mode). To do this, the VT
+    // parser tells it the positions of any such relevant VT sequences.
     enum class InjectionType : size_t
     {
         RIS,
         DECSET_FOCUS,
+
+        Count,
     };
 
     struct Injection