From cd8c12586b130bf9143a1f58d02ef0629d4cc4a5 Mon Sep 17 00:00:00 2001 From: Mike Griese Date: Fri, 23 Aug 2024 14:24:34 -0500 Subject: [PATCH] Clip the "current commandline" at the cursor position (#17781) This is particularly relevant to pwsh with the "ghost text" enabled. In that scenario, pwsh writes out the predicted command to the right of the cursor. With `showSuggestions(useCommandline=true)`, we'd auto-include that text in the filter, and that was effectively useless. This instead defaults us to not use anything to the right of the cursor (inclusive) for what we consider "the current commandline" closes #17772 --- .wt.json | 6 ++ src/buffer/out/textBuffer.cpp | 19 +++++-- src/buffer/out/textBuffer.hpp | 2 +- src/cascadia/TerminalControl/ControlCore.cpp | 5 +- .../UnitTests_Control/ControlCoreTests.cpp | 57 +++++++++++++++++++ src/host/ut_host/ScreenBufferTests.cpp | 4 ++ 6 files changed, 87 insertions(+), 6 deletions(-) diff --git a/.wt.json b/.wt.json index 44f718f42e..f0091b5e32 100644 --- a/.wt.json +++ b/.wt.json @@ -23,6 +23,12 @@ "name": "Upload package to nuget feed", "icon": "\uE898", "description": "Go download a .nupkg, put it in ~/Downloads, and use this to push to our private feed." + }, + { + "input": "runut /name:**\u001b[D", + "name": "Run a test", + "icon": "", + "description": "Enter the name of a test to run" } ] } diff --git a/src/buffer/out/textBuffer.cpp b/src/buffer/out/textBuffer.cpp index c4f38d85d9..9e1e70e3d0 100644 --- a/src/buffer/out/textBuffer.cpp +++ b/src/buffer/out/textBuffer.cpp @@ -3266,23 +3266,30 @@ MarkExtents TextBuffer::_scrollMarkExtentForRow(const til::CoordType rowOffset, return mark; } -std::wstring TextBuffer::_commandForRow(const til::CoordType rowOffset, const til::CoordType bottomInclusive) const +std::wstring TextBuffer::_commandForRow(const til::CoordType rowOffset, + const til::CoordType bottomInclusive, + const bool clipAtCursor) const { std::wstring commandBuilder; MarkKind lastMarkKind = MarkKind::Prompt; + const auto cursorPosition = GetCursor().GetPosition(); for (auto y = rowOffset; y <= bottomInclusive; y++) { + const bool onCursorRow = clipAtCursor && y == cursorPosition.y; // Now we need to iterate over text attributes. We need to find a // segment of Prompt attributes, we'll skip those. Then there should be // Command attributes. Collect up all of those, till we get to the next // Output attribute. - const auto& row = GetRowByOffset(y); const auto runs = row.Attributes().runs(); auto x = 0; for (const auto& [attr, length] : runs) { - const auto nextX = gsl::narrow_cast(x + length); + auto nextX = gsl::narrow_cast(x + length); + if (onCursorRow) + { + nextX = std::min(nextX, gsl::narrow_cast(cursorPosition.x)); + } const auto markKind{ attr.GetMarkAttributes() }; if (markKind != lastMarkKind) { @@ -3302,6 +3309,10 @@ std::wstring TextBuffer::_commandForRow(const til::CoordType rowOffset, const ti } // advance to next run of text x = nextX; + if (onCursorRow && x == cursorPosition.x) + { + return commandBuilder; + } } // we went over all the runs in this row, but we're not done yet. Keep iterating on the next row. } @@ -3325,7 +3336,7 @@ std::wstring TextBuffer::CurrentCommand() const // This row did start a prompt! Find the prompt that starts here. // Presumably, no rows below us will have prompts, so pass in the last // row with text as the bottom - return _commandForRow(promptY, _estimateOffsetOfLastCommittedRow()); + return _commandForRow(promptY, _estimateOffsetOfLastCommittedRow(), true); } return L""; } diff --git a/src/buffer/out/textBuffer.hpp b/src/buffer/out/textBuffer.hpp index a6a4ea88f7..a39ebd4db4 100644 --- a/src/buffer/out/textBuffer.hpp +++ b/src/buffer/out/textBuffer.hpp @@ -326,7 +326,7 @@ private: til::point _GetWordEndForSelection(const til::point target, const std::wstring_view wordDelimiters) const; void _PruneHyperlinks(); - std::wstring _commandForRow(const til::CoordType rowOffset, const til::CoordType bottomInclusive) const; + std::wstring _commandForRow(const til::CoordType rowOffset, const til::CoordType bottomInclusive, const bool clipAtCursor = false) const; MarkExtents _scrollMarkExtentForRow(const til::CoordType rowOffset, const til::CoordType bottomInclusive) const; bool _createPromptMarkIfNeeded(); diff --git a/src/cascadia/TerminalControl/ControlCore.cpp b/src/cascadia/TerminalControl/ControlCore.cpp index f433e69140..9be62aad0c 100644 --- a/src/cascadia/TerminalControl/ControlCore.cpp +++ b/src/cascadia/TerminalControl/ControlCore.cpp @@ -2291,7 +2291,10 @@ namespace winrt::Microsoft::Terminal::Control::implementation // If the very last thing in the list of recent commands, is exactly the // same as the current command, then let's not include it in the // history. It's literally the thing the user has typed, RIGHT now. - if (!commands.empty() && commands.back() == trimmedCurrentCommand) + // (also account for the fact that the cursor may be in the middle of a commandline) + if (!commands.empty() && + !trimmedCurrentCommand.empty() && + std::wstring_view{ commands.back() }.substr(0, trimmedCurrentCommand.size()) == trimmedCurrentCommand) { commands.pop_back(); } diff --git a/src/cascadia/UnitTests_Control/ControlCoreTests.cpp b/src/cascadia/UnitTests_Control/ControlCoreTests.cpp index 8c76135d4e..4620afae09 100644 --- a/src/cascadia/UnitTests_Control/ControlCoreTests.cpp +++ b/src/cascadia/UnitTests_Control/ControlCoreTests.cpp @@ -41,6 +41,8 @@ namespace ControlUnitTests TEST_METHOD(TestSelectCommandSimple); TEST_METHOD(TestSelectOutputSimple); TEST_METHOD(TestCommandContext); + TEST_METHOD(TestCommandContextWithPwshGhostText); + TEST_METHOD(TestSelectOutputScrolling); TEST_METHOD(TestSelectOutputExactWrap); @@ -556,6 +558,61 @@ namespace ControlUnitTests } } + void ControlCoreTests::TestCommandContextWithPwshGhostText() + { + auto [settings, conn] = _createSettingsAndConnection(); + Log::Comment(L"Create ControlCore object"); + auto core = createCore(*settings, *conn); + VERIFY_IS_NOT_NULL(core); + _standardInit(core); + + Log::Comment(L"Print some text"); + + _writePrompt(conn, L"C:\\Windows"); + conn->WriteInput(winrt_wstring_to_array_view(L"Foo-bar")); + conn->WriteInput(winrt_wstring_to_array_view(L"\x1b]133;C\x7")); + + conn->WriteInput(winrt_wstring_to_array_view(L"\r\n")); + conn->WriteInput(winrt_wstring_to_array_view(L"This is some text \r\n")); + conn->WriteInput(winrt_wstring_to_array_view(L"with varying amounts \r\n")); + conn->WriteInput(winrt_wstring_to_array_view(L"of whitespace \r\n")); + + _writePrompt(conn, L"C:\\Windows"); + + Log::Comment(L"Check the command context"); + + const WEX::TestExecution::DisableVerifyExceptions disableExceptionsScope; + { + auto historyContext{ core->CommandHistory() }; + VERIFY_ARE_EQUAL(1u, historyContext.History().Size()); + VERIFY_ARE_EQUAL(L"", historyContext.CurrentCommandline()); + } + + Log::Comment(L"Write 'BarBar' to the command..."); + conn->WriteInput(winrt_wstring_to_array_view(L"BarBar")); + { + auto historyContext{ core->CommandHistory() }; + // BarBar shouldn't be in the history, it should be the current command + VERIFY_ARE_EQUAL(1u, historyContext.History().Size()); + VERIFY_ARE_EQUAL(L"BarBar", historyContext.CurrentCommandline()); + } + + Log::Comment(L"then move the cursor to the left"); + // This emulates the state the buffer is in when pwsh does it's "ghost + // text" thing. We don't want to include all that ghost text in the + // current commandline. + conn->WriteInput(winrt_wstring_to_array_view(L"\x1b[D")); + conn->WriteInput(winrt_wstring_to_array_view(L"\x1b[D")); + { + auto historyContext{ core->CommandHistory() }; + VERIFY_ARE_EQUAL(1u, historyContext.History().Size()); + // The current commandline is only the text to the left of the cursor + auto curr{ historyContext.CurrentCommandline() }; + VERIFY_ARE_EQUAL(4u, curr.size()); + VERIFY_ARE_EQUAL(L"BarB", curr); + } + } + void ControlCoreTests::TestSelectOutputScrolling() { auto [settings, conn] = _createSettingsAndConnection(); diff --git a/src/host/ut_host/ScreenBufferTests.cpp b/src/host/ut_host/ScreenBufferTests.cpp index beaab2c304..f4aa82ac42 100644 --- a/src/host/ut_host/ScreenBufferTests.cpp +++ b/src/host/ut_host/ScreenBufferTests.cpp @@ -8337,6 +8337,10 @@ void ScreenBufferTests::SimpleMarkCommand() void ScreenBufferTests::SimpleWrappedCommand() { + BEGIN_TEST_METHOD_PROPERTIES() + TEST_METHOD_PROPERTY(L"IsolationLevel", L"Method") + END_TEST_METHOD_PROPERTIES() + auto& g = ServiceLocator::LocateGlobals(); auto& gci = g.getConsoleInformation(); auto& si = gci.GetActiveOutputBuffer();