From fc0a06c3b6cae2ede1326eef202a800244365ebd Mon Sep 17 00:00:00 2001 From: Leonard Hecker Date: Wed, 25 Jun 2025 18:31:28 +0200 Subject: [PATCH] Preserve the cursor row during Clear Buffer (#18976) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Introduces an ABI change to the ConptyClearPseudoConsole signal. Otherwise, we have to make it so that the API call always retains the row the cursor is on, but I feel like that makes it worse. Closes #18732 Closes #18878 ## Validation Steps Performed * Launch `ConsoleMonitor.exe` * Create some text above & below the cursor in PowerShell * Clear Buffer * Buffer is cleared except for the cursor row ✅ * ...same in ConPTY ✅ --- .../TerminalConnection/ConptyConnection.cpp | 4 +- .../TerminalConnection/ConptyConnection.h | 2 +- .../TerminalConnection/ConptyConnection.idl | 2 +- src/cascadia/TerminalControl/ControlCore.cpp | 52 +++++++++++++------ .../UnitTests_Control/ControlCoreTests.cpp | 10 ++-- src/host/PtySignalInputThread.cpp | 17 ++++-- src/host/PtySignalInputThread.hpp | 7 ++- src/inc/conpty-static.h | 2 +- src/winconpty/winconpty.cpp | 9 ++-- src/winconpty/winconpty.h | 1 - 10 files changed, 69 insertions(+), 37 deletions(-) diff --git a/src/cascadia/TerminalConnection/ConptyConnection.cpp b/src/cascadia/TerminalConnection/ConptyConnection.cpp index 5778c0b6ed..6f15a8c1ed 100644 --- a/src/cascadia/TerminalConnection/ConptyConnection.cpp +++ b/src/cascadia/TerminalConnection/ConptyConnection.cpp @@ -563,13 +563,13 @@ namespace winrt::Microsoft::Terminal::TerminalConnection::implementation } } - void ConptyConnection::ClearBuffer() + void ConptyConnection::ClearBuffer(bool keepCursorRow) { // If we haven't connected yet, then we really don't need to do // anything. The connection should already start clear! if (_isConnected()) { - THROW_IF_FAILED(ConptyClearPseudoConsole(_hPC.get())); + THROW_IF_FAILED(ConptyClearPseudoConsole(_hPC.get(), keepCursorRow)); } } diff --git a/src/cascadia/TerminalConnection/ConptyConnection.h b/src/cascadia/TerminalConnection/ConptyConnection.h index 69edd7b7a8..4d847fdcd1 100644 --- a/src/cascadia/TerminalConnection/ConptyConnection.h +++ b/src/cascadia/TerminalConnection/ConptyConnection.h @@ -25,7 +25,7 @@ namespace winrt::Microsoft::Terminal::TerminalConnection::implementation void Resize(uint32_t rows, uint32_t columns); void ResetSize(); void Close() noexcept; - void ClearBuffer(); + void ClearBuffer(bool keepCursorRow); void ShowHide(const bool show); diff --git a/src/cascadia/TerminalConnection/ConptyConnection.idl b/src/cascadia/TerminalConnection/ConptyConnection.idl index e1ed83cc0c..5264238c79 100644 --- a/src/cascadia/TerminalConnection/ConptyConnection.idl +++ b/src/cascadia/TerminalConnection/ConptyConnection.idl @@ -15,7 +15,7 @@ namespace Microsoft.Terminal.TerminalConnection UInt16 ShowWindow { get; }; void ResetSize(); - void ClearBuffer(); + void ClearBuffer(Boolean keepCursorRow); void ShowHide(Boolean show); diff --git a/src/cascadia/TerminalControl/ControlCore.cpp b/src/cascadia/TerminalControl/ControlCore.cpp index b2ad7b7549..6a4147637d 100644 --- a/src/cascadia/TerminalControl/ControlCore.cpp +++ b/src/cascadia/TerminalControl/ControlCore.cpp @@ -2264,23 +2264,42 @@ namespace winrt::Microsoft::Terminal::Control::implementation // - void ControlCore::ClearBuffer(Control::ClearBufferType clearType) { - std::wstring_view command; - switch (clearType) - { - case ClearBufferType::Screen: - command = L"\x1b[H\x1b[2J"; - break; - case ClearBufferType::Scrollback: - command = L"\x1b[3J"; - break; - case ClearBufferType::All: - command = L"\x1b[H\x1b[2J\x1b[3J"; - break; - } - { const auto lock = _terminal->LockForWriting(); - _terminal->Write(command); + // In absolute buffer coordinates, including the scrollback (= Y is offset by the scrollback height). + const auto viewport = _terminal->GetViewport(); + // The absolute cursor coordinate. + const auto cursor = _terminal->GetViewportRelativeCursorPosition(); + + // GH#18732: Users want the row the cursor is on to be preserved across clears. + std::wstring sequence; + + if (clearType == ClearBufferType::Scrollback || clearType == ClearBufferType::All) + { + sequence.append(L"\x1b[3J"); + } + + if (clearType == ClearBufferType::Screen || clearType == ClearBufferType::All) + { + // Erase any viewport contents below (but not including) the cursor row. + if (viewport.Height() - cursor.y > 1) + { + fmt::format_to(std::back_inserter(sequence), FMT_COMPILE(L"\x1b[{};1H\x1b[J"), cursor.y + 2); + } + + // Erase any viewport contents above (but not including) the cursor row. + if (cursor.y > 0) + { + // An SU sequence would be simpler than this DL sequence, + // but SU isn't well standardized between terminals. + // Generally speaking, it's best avoiding it. + fmt::format_to(std::back_inserter(sequence), FMT_COMPILE(L"\x1b[H\x1b[{}M"), cursor.y); + } + + fmt::format_to(std::back_inserter(sequence), FMT_COMPILE(L"\x1b[1;{}H"), cursor.x + 1); + } + + _terminal->Write(sequence); } if (clearType == Control::ClearBufferType::Screen || clearType == Control::ClearBufferType::All) @@ -2289,8 +2308,7 @@ namespace winrt::Microsoft::Terminal::Control::implementation { // Since the clearing of ConPTY occurs asynchronously, this call can result weird issues, // where a console application still sees contents that we've already deleted, etc. - // The correct way would be for ConPTY to emit the appropriate CSI n J sequences. - conpty.ClearBuffer(); + conpty.ClearBuffer(true); } } } diff --git a/src/cascadia/UnitTests_Control/ControlCoreTests.cpp b/src/cascadia/UnitTests_Control/ControlCoreTests.cpp index 999809cb39..69b00b69db 100644 --- a/src/cascadia/UnitTests_Control/ControlCoreTests.cpp +++ b/src/cascadia/UnitTests_Control/ControlCoreTests.cpp @@ -248,7 +248,7 @@ namespace ControlUnitTests _standardInit(core); Log::Comment(L"Print 40 rows of 'Foo', and a single row of 'Bar' " - L"(leaving the cursor afer 'Bar')"); + L"(leaving the cursor after 'Bar')"); for (auto i = 0; i < 40; ++i) { conn->WriteInput(winrt_wstring_to_array_view(L"Foo\r\n")); @@ -285,7 +285,7 @@ namespace ControlUnitTests _standardInit(core); Log::Comment(L"Print 40 rows of 'Foo', and a single row of 'Bar' " - L"(leaving the cursor afer 'Bar')"); + L"(leaving the cursor after 'Bar')"); for (auto i = 0; i < 40; ++i) { conn->WriteInput(winrt_wstring_to_array_view(L"Foo\r\n")); @@ -304,9 +304,9 @@ namespace ControlUnitTests Log::Comment(L"Check the buffer after the clear"); VERIFY_ARE_EQUAL(20, core->_terminal->GetViewport().Height()); - VERIFY_ARE_EQUAL(41, core->ScrollOffset()); + VERIFY_ARE_EQUAL(21, core->ScrollOffset()); VERIFY_ARE_EQUAL(20, core->ViewHeight()); - VERIFY_ARE_EQUAL(61, core->BufferHeight()); + VERIFY_ARE_EQUAL(41, core->BufferHeight()); // In this test, we can't actually check if we cleared the buffer // contents. ConPTY will handle the actual clearing of the buffer @@ -322,7 +322,7 @@ namespace ControlUnitTests _standardInit(core); Log::Comment(L"Print 40 rows of 'Foo', and a single row of 'Bar' " - L"(leaving the cursor afer 'Bar')"); + L"(leaving the cursor after 'Bar')"); for (auto i = 0; i < 40; ++i) { conn->WriteInput(winrt_wstring_to_array_view(L"Foo\r\n")); diff --git a/src/host/PtySignalInputThread.cpp b/src/host/PtySignalInputThread.cpp index 2829ebdbcc..0b03612d9a 100644 --- a/src/host/PtySignalInputThread.cpp +++ b/src/host/PtySignalInputThread.cpp @@ -124,7 +124,13 @@ try } case PtySignal::ClearBuffer: { - _DoClearBuffer(); + ClearBufferData msg = { 0 }; + if (!_GetData(&msg, sizeof(msg))) + { + return S_OK; + } + + _DoClearBuffer(msg.keepCursorRow != 0); break; } case PtySignal::ResizeWindow: @@ -180,7 +186,7 @@ void PtySignalInputThread::_DoResizeWindow(const ResizeWindowData& data) _api.ResizeWindow(data.sx, data.sy); } -void PtySignalInputThread::_DoClearBuffer() const +void PtySignalInputThread::_DoClearBuffer(const bool keepCursorRow) const { LockConsole(); auto Unlock = wil::scope_exit([&] { UnlockConsole(); }); @@ -196,8 +202,11 @@ void PtySignalInputThread::_DoClearBuffer() const auto& gci = ServiceLocator::LocateGlobals().getConsoleInformation(); auto& screenInfo = gci.GetActiveOutputBuffer(); - auto& stateMachine = screenInfo.GetStateMachine(); - stateMachine.ProcessString(L"\x1b[H\x1b[2J"); + auto& tb = screenInfo.GetTextBuffer(); + const auto cursor = tb.GetCursor().GetPosition(); + + tb.ClearScrollback(cursor.y, keepCursorRow ? 1 : 0); + tb.GetCursor().SetPosition({ keepCursorRow ? cursor.x : 0, 0 }); } void PtySignalInputThread::_DoShowHide(const ShowHideData& data) diff --git a/src/host/PtySignalInputThread.hpp b/src/host/PtySignalInputThread.hpp index eead316ba9..9ff4fb0c22 100644 --- a/src/host/PtySignalInputThread.hpp +++ b/src/host/PtySignalInputThread.hpp @@ -55,6 +55,11 @@ namespace Microsoft::Console unsigned short show; // used as a bool, but passed as a ushort }; + struct ClearBufferData + { + unsigned short keepCursorRow; + }; + struct SetParentData { uint64_t handle; @@ -64,7 +69,7 @@ namespace Microsoft::Console [[nodiscard]] bool _GetData(_Out_writes_bytes_(cbBuffer) void* const pBuffer, const DWORD cbBuffer); void _DoResizeWindow(const ResizeWindowData& data); void _DoSetWindowParent(const SetParentData& data); - void _DoClearBuffer() const; + void _DoClearBuffer(bool keepCursorRow) const; void _DoShowHide(const ShowHideData& data); void _Shutdown(); diff --git a/src/inc/conpty-static.h b/src/inc/conpty-static.h index aa31c67fa3..3aca15dba7 100644 --- a/src/inc/conpty-static.h +++ b/src/inc/conpty-static.h @@ -38,7 +38,7 @@ CONPTY_EXPORT HRESULT WINAPI ConptyCreatePseudoConsole(COORD size, HANDLE hInput CONPTY_EXPORT HRESULT WINAPI ConptyCreatePseudoConsoleAsUser(HANDLE hToken, COORD size, HANDLE hInput, HANDLE hOutput, DWORD dwFlags, HPCON* phPC); CONPTY_EXPORT HRESULT WINAPI ConptyResizePseudoConsole(HPCON hPC, COORD size); -CONPTY_EXPORT HRESULT WINAPI ConptyClearPseudoConsole(HPCON hPC); +CONPTY_EXPORT HRESULT WINAPI ConptyClearPseudoConsole(HPCON hPC, BOOL keepCursorRow); CONPTY_EXPORT HRESULT WINAPI ConptyShowHidePseudoConsole(HPCON hPC, bool show); CONPTY_EXPORT HRESULT WINAPI ConptyReparentPseudoConsole(HPCON hPC, HWND newParent); CONPTY_EXPORT HRESULT WINAPI ConptyReleasePseudoConsole(HPCON hPC); diff --git a/src/winconpty/winconpty.cpp b/src/winconpty/winconpty.cpp index 144ea987a8..969a8fbdfe 100644 --- a/src/winconpty/winconpty.cpp +++ b/src/winconpty/winconpty.cpp @@ -278,15 +278,16 @@ HRESULT _ResizePseudoConsole(_In_ const PseudoConsole* const pPty, _In_ const CO // Return Value: // - S_OK if the call succeeded, else an appropriate HRESULT for failing to // write the clear message to the pty. -HRESULT _ClearPseudoConsole(_In_ const PseudoConsole* const pPty) +static HRESULT _ClearPseudoConsole(_In_ const PseudoConsole* const pPty, BOOL keepCursorRow) noexcept { if (pPty == nullptr) { return E_INVALIDARG; } - unsigned short signalPacket[1]; + unsigned short signalPacket[2]; signalPacket[0] = PTY_SIGNAL_CLEAR_WINDOW; + signalPacket[1] = keepCursorRow ? 1 : 0; const auto fSuccess = WriteFile(pPty->hSignal, signalPacket, sizeof(signalPacket), nullptr, nullptr); return fSuccess ? S_OK : HRESULT_FROM_WIN32(GetLastError()); @@ -492,13 +493,13 @@ extern "C" HRESULT WINAPI ConptyResizePseudoConsole(_In_ HPCON hPC, _In_ COORD s // - This is used exclusively by ConPTY to support GH#1193, GH#1882. This allows // a terminal to clear the contents of the ConPTY buffer, which is important // if the user would like to be able to clear the terminal-side buffer. -extern "C" HRESULT WINAPI ConptyClearPseudoConsole(_In_ HPCON hPC) +extern "C" HRESULT WINAPI ConptyClearPseudoConsole(_In_ HPCON hPC, BOOL keepCursorRow) { const PseudoConsole* const pPty = (PseudoConsole*)hPC; auto hr = pPty == nullptr ? E_INVALIDARG : S_OK; if (SUCCEEDED(hr)) { - hr = _ClearPseudoConsole(pPty); + hr = _ClearPseudoConsole(pPty, keepCursorRow); } return hr; } diff --git a/src/winconpty/winconpty.h b/src/winconpty/winconpty.h index 4bb43ddf8f..8a245226ca 100644 --- a/src/winconpty/winconpty.h +++ b/src/winconpty/winconpty.h @@ -68,7 +68,6 @@ HRESULT _CreatePseudoConsole(const HANDLE hToken, _Inout_ PseudoConsole* pPty); HRESULT _ResizePseudoConsole(_In_ const PseudoConsole* const pPty, _In_ const COORD size); -HRESULT _ClearPseudoConsole(_In_ const PseudoConsole* const pPty); HRESULT _ShowHidePseudoConsole(_In_ const PseudoConsole* const pPty, const bool show); HRESULT _ReparentPseudoConsole(_In_ const PseudoConsole* const pPty, _In_ const HWND newParent); void _ClosePseudoConsoleMembers(_In_ PseudoConsole* pPty);