Skip to content

Commit

Permalink
Clean up command history context passed to suggestions UI (#17245)
Browse files Browse the repository at this point in the history
This is fallout from #16937.

* Typing a command then backspacing the chars then asking for
suggestions would think the current commandline ended with spaces,
making filtering very hard.
* The currently typed command would _also_ appear in the command
history, which isn't useful.

I actually did TDD for this and wrote the test first, then confirmed
again running through the build script, I wasn't hitting any of the
earlier issues.

Closes #17241
Closes #17243

(cherry picked from commit bf8a647)
Service-Card-Id: 92638413
Service-Version: 1.21
  • Loading branch information
zadjii-msft authored and DHowett committed Jun 7, 2024
1 parent bc365c9 commit 8038dc6
Show file tree
Hide file tree
Showing 2 changed files with 77 additions and 7 deletions.
33 changes: 26 additions & 7 deletions src/cascadia/TerminalControl/ControlCore.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -2235,20 +2235,39 @@ namespace winrt::Microsoft::Terminal::Control::implementation

std::vector<winrt::hstring> commands;
const auto bufferCommands{ textBuffer.Commands() };
for (const auto& commandInBuffer : bufferCommands)
{
const auto strEnd = commandInBuffer.find_last_not_of(UNICODE_SPACE);

auto trimToHstring = [](const auto& s) -> winrt::hstring {
const auto strEnd = s.find_last_not_of(UNICODE_SPACE);
if (strEnd != std::string::npos)
{
const auto trimmed = commandInBuffer.substr(0, strEnd + 1);
const auto trimmed = s.substr(0, strEnd + 1);
return winrt::hstring{ trimmed };
}
return winrt::hstring{ L"" };
};

const auto currentCommand = _terminal->CurrentCommand();
const auto trimmedCurrentCommand = trimToHstring(currentCommand);

commands.push_back(winrt::hstring{ trimmed });
for (const auto& commandInBuffer : bufferCommands)
{
if (const auto hstr{ trimToHstring(commandInBuffer) };
(!hstr.empty() && hstr != trimmedCurrentCommand))
{
commands.push_back(hstr);
}
}

auto context = winrt::make_self<CommandHistoryContext>(std::move(commands));
context->CurrentCommandline(winrt::hstring{ _terminal->CurrentCommand() });
// 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)
{
commands.pop_back();
}

auto context = winrt::make_self<CommandHistoryContext>(std::move(commands));
context->CurrentCommandline(trimmedCurrentCommand);
return *context;
}

Expand Down
51 changes: 51 additions & 0 deletions src/cascadia/UnitTests_Control/ControlCoreTests.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -40,6 +40,7 @@ namespace ControlUnitTests

TEST_METHOD(TestSelectCommandSimple);
TEST_METHOD(TestSelectOutputSimple);
TEST_METHOD(TestCommandContext);
TEST_METHOD(TestSelectOutputScrolling);
TEST_METHOD(TestSelectOutputExactWrap);

Expand Down Expand Up @@ -509,6 +510,56 @@ namespace ControlUnitTests
VERIFY_ARE_EQUAL(expectedEnd, end);
}
}
void ControlCoreTests::TestCommandContext()
{
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(L"Foo-bar");
conn->WriteInput(L"\x1b]133;C\x7");

conn->WriteInput(L"\r\n");
conn->WriteInput(L"This is some text \r\n");
conn->WriteInput(L"with varying amounts \r\n");
conn->WriteInput(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 'Bar' to the command...");
conn->WriteInput(L"Bar");
{
auto historyContext{ core->CommandHistory() };
// Bar shouldn't be in the history, it should be the current command
VERIFY_ARE_EQUAL(1u, historyContext.History().Size());
VERIFY_ARE_EQUAL(L"Bar", historyContext.CurrentCommandline());
}

Log::Comment(L"then delete it");
conn->WriteInput(L"\b \b");
conn->WriteInput(L"\b \b");
conn->WriteInput(L"\b \b");
{
auto historyContext{ core->CommandHistory() };
VERIFY_ARE_EQUAL(1u, historyContext.History().Size());
// The current commandline is now empty
VERIFY_ARE_EQUAL(L"", historyContext.CurrentCommandline());
}
}

void ControlCoreTests::TestSelectOutputScrolling()
{
Expand Down

0 comments on commit 8038dc6

Please sign in to comment.