Skip to content

Commit

Permalink
Add support for naming windows with the -w parameter (#9300)
Browse files Browse the repository at this point in the history 
This finishes the implementation of `--window` to also accept a string
as the "name" of the window. So you can say 

```sh
wt -w foo new-tab
wt -w foo split-pane
```

and have both those commands execute in the same window, the one named
"foo". This is just slightly more ergonomic than manually using the IDs
of windows. In the future, I'll be working on renaming windows, and
displaying these names. 

> #### `--window,-w <window-id>`
> Run these commands in the given Windows Terminal session. This enables opening
> new tabs, splits, etc. in already running Windows Terminal windows.
> * If `window-id` is `0`, run the given commands in _the current window_.
> * If `window-id` is a negative number, or the reserved name `new`, run the
>   commands in a _new_ Terminal window.
> * If `window-id` is the ID or name of an existing window, then run the
>   commandline in that window.
> * If `window-id` is _not_ the ID or name of an existing window, create a new
>   window. That window will be assigned the ID or name provided in the
>   commandline. The provided subcommands will be run in that new window.
> * If `window-id` is omitted, then obey the value of `windowingBehavior` when
>   determining which window to run the command in.

Before this PR, I think we didn't actually properly support assigning
the id with `wt -w 12345`. If `12345` didn't exist, it would make a new
window, but just assign it the next id, not assign it 12345.

## References
* #4472, #8135
* https://github.com/microsoft/terminal/projects/5

## Validation Steps Performed
Ran tests
Messed with naming windows, working as expected.

Closes https://github.com/microsoft/terminal/projects/5#card-51431478
  • Loading branch information
@zadjii-msft
zadjii-msft committed Mar 17, 2021
1 parent 8346968 commit 43c469f
Show file tree
Hide file tree
Showing 18 changed files with 757 additions and 97 deletions.
187 changes: 145 additions & 42 deletions src/cascadia/LocalTests_TerminalApp/CommandlineTest.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -1594,59 +1594,155 @@ namespace TerminalAppLocalTests
void CommandlineTest::TestFindTargetWindow()
{
{
Log::Comment(L"wt.exe with no args should always use the value from"
L" the settings (passed as the second argument).");

std::vector<winrt::hstring> args{ L"wt.exe" };
VERIFY_ARE_EQUAL(WindowingBehaviorUseNew,
appImpl::AppLogic::_doFindTargetWindow({ args }, WindowingMode::UseNew));
auto result = appImpl::AppLogic::_doFindTargetWindow({ args }, WindowingMode::UseNew);
VERIFY_ARE_EQUAL(WindowingBehaviorUseNew, result.WindowId());
VERIFY_ARE_EQUAL(L"", result.WindowName());

VERIFY_ARE_EQUAL(WindowingBehaviorUseExisting,
appImpl::AppLogic::_doFindTargetWindow({ args }, WindowingMode::UseExisting));
result = appImpl::AppLogic::_doFindTargetWindow({ args }, WindowingMode::UseExisting);
VERIFY_ARE_EQUAL(WindowingBehaviorUseExisting, result.WindowId());
VERIFY_ARE_EQUAL(L"", result.WindowName());

VERIFY_ARE_EQUAL(WindowingBehaviorUseAnyExisting,
appImpl::AppLogic::_doFindTargetWindow({ args }, WindowingMode::UseAnyExisting));
result = appImpl::AppLogic::_doFindTargetWindow({ args }, WindowingMode::UseAnyExisting);
VERIFY_ARE_EQUAL(WindowingBehaviorUseAnyExisting, result.WindowId());
VERIFY_ARE_EQUAL(L"", result.WindowName());
}
{
Log::Comment(L"-w -1 should always result in a new window");

std::vector<winrt::hstring> args{ L"wt.exe", L"-w", L"-1" };
VERIFY_ARE_EQUAL(WindowingBehaviorUseNew,
appImpl::AppLogic::_doFindTargetWindow({ args }, WindowingMode::UseNew));
auto result = appImpl::AppLogic::_doFindTargetWindow({ args }, WindowingMode::UseNew);
VERIFY_ARE_EQUAL(WindowingBehaviorUseNew, result.WindowId());
VERIFY_ARE_EQUAL(L"", result.WindowName());

result = appImpl::AppLogic::_doFindTargetWindow({ args }, WindowingMode::UseExisting);
VERIFY_ARE_EQUAL(WindowingBehaviorUseNew, result.WindowId());
VERIFY_ARE_EQUAL(L"", result.WindowName());

result = appImpl::AppLogic::_doFindTargetWindow({ args }, WindowingMode::UseAnyExisting);
VERIFY_ARE_EQUAL(WindowingBehaviorUseNew, result.WindowId());
VERIFY_ARE_EQUAL(L"", result.WindowName());
}
{
Log::Comment(L"\"new\" should always result in a new window");

std::vector<winrt::hstring> args{ L"wt.exe", L"-w", L"new" };
auto result = appImpl::AppLogic::_doFindTargetWindow({ args }, WindowingMode::UseNew);
VERIFY_ARE_EQUAL(WindowingBehaviorUseNew, result.WindowId());
VERIFY_ARE_EQUAL(L"", result.WindowName());

VERIFY_ARE_EQUAL(WindowingBehaviorUseNew,
appImpl::AppLogic::_doFindTargetWindow({ args }, WindowingMode::UseExisting));
result = appImpl::AppLogic::_doFindTargetWindow({ args }, WindowingMode::UseExisting);
VERIFY_ARE_EQUAL(WindowingBehaviorUseNew, result.WindowId());
VERIFY_ARE_EQUAL(L"", result.WindowName());

VERIFY_ARE_EQUAL(WindowingBehaviorUseNew,
appImpl::AppLogic::_doFindTargetWindow({ args }, WindowingMode::UseAnyExisting));
result = appImpl::AppLogic::_doFindTargetWindow({ args }, WindowingMode::UseAnyExisting);
VERIFY_ARE_EQUAL(WindowingBehaviorUseNew, result.WindowId());
VERIFY_ARE_EQUAL(L"", result.WindowName());
}
{
Log::Comment(L"-w with a negative number should always result in a "
L"new window");

std::vector<winrt::hstring> args{ L"wt.exe", L"-w", L"-12345" };
VERIFY_ARE_EQUAL(WindowingBehaviorUseNew,
appImpl::AppLogic::_doFindTargetWindow({ args }, WindowingMode::UseNew));
auto result = appImpl::AppLogic::_doFindTargetWindow({ args }, WindowingMode::UseNew);
VERIFY_ARE_EQUAL(WindowingBehaviorUseNew, result.WindowId());
VERIFY_ARE_EQUAL(L"", result.WindowName());

result = appImpl::AppLogic::_doFindTargetWindow({ args }, WindowingMode::UseExisting);
VERIFY_ARE_EQUAL(WindowingBehaviorUseNew, result.WindowId());
VERIFY_ARE_EQUAL(L"", result.WindowName());

result = appImpl::AppLogic::_doFindTargetWindow({ args }, WindowingMode::UseAnyExisting);
VERIFY_ARE_EQUAL(WindowingBehaviorUseNew, result.WindowId());
VERIFY_ARE_EQUAL(L"", result.WindowName());
}
{
Log::Comment(L"-w with a positive number should result in us trying"
L" to either make a new one or find an existing one "
L"with that ID, depending on the provided argument");

std::vector<winrt::hstring> args{ L"wt.exe", L"-w", L"12345" };
auto result = appImpl::AppLogic::_doFindTargetWindow({ args }, WindowingMode::UseNew);
VERIFY_ARE_EQUAL(12345, result.WindowId());
VERIFY_ARE_EQUAL(L"", result.WindowName());

VERIFY_ARE_EQUAL(WindowingBehaviorUseNew,
appImpl::AppLogic::_doFindTargetWindow({ args }, WindowingMode::UseExisting));
result = appImpl::AppLogic::_doFindTargetWindow({ args }, WindowingMode::UseExisting);
VERIFY_ARE_EQUAL(12345, result.WindowId());
VERIFY_ARE_EQUAL(L"", result.WindowName());

VERIFY_ARE_EQUAL(WindowingBehaviorUseNew,
appImpl::AppLogic::_doFindTargetWindow({ args }, WindowingMode::UseAnyExisting));
result = appImpl::AppLogic::_doFindTargetWindow({ args }, WindowingMode::UseAnyExisting);
VERIFY_ARE_EQUAL(12345, result.WindowId());
VERIFY_ARE_EQUAL(L"", result.WindowName());
}
{
Log::Comment(L"-w 0 should always use the \"current\" window");

std::vector<winrt::hstring> args{ L"wt.exe", L"-w", L"0" };
VERIFY_ARE_EQUAL(WindowingBehaviorUseCurrent,
appImpl::AppLogic::_doFindTargetWindow({ args }, WindowingMode::UseNew));
auto result = appImpl::AppLogic::_doFindTargetWindow({ args }, WindowingMode::UseNew);
VERIFY_ARE_EQUAL(WindowingBehaviorUseCurrent, result.WindowId());
VERIFY_ARE_EQUAL(L"", result.WindowName());

result = appImpl::AppLogic::_doFindTargetWindow({ args }, WindowingMode::UseExisting);
VERIFY_ARE_EQUAL(WindowingBehaviorUseCurrent, result.WindowId());
VERIFY_ARE_EQUAL(L"", result.WindowName());

result = appImpl::AppLogic::_doFindTargetWindow({ args }, WindowingMode::UseAnyExisting);
VERIFY_ARE_EQUAL(WindowingBehaviorUseCurrent, result.WindowId());
VERIFY_ARE_EQUAL(L"", result.WindowName());
}
{
Log::Comment(L"-w last should always use the most recent window on "
L"this desktop");

std::vector<winrt::hstring> args{ L"wt.exe", L"-w", L"last" };
auto result = appImpl::AppLogic::_doFindTargetWindow({ args }, WindowingMode::UseNew);
VERIFY_ARE_EQUAL(WindowingBehaviorUseExisting, result.WindowId());
VERIFY_ARE_EQUAL(L"", result.WindowName());

VERIFY_ARE_EQUAL(WindowingBehaviorUseCurrent,
appImpl::AppLogic::_doFindTargetWindow({ args }, WindowingMode::UseExisting));
result = appImpl::AppLogic::_doFindTargetWindow({ args }, WindowingMode::UseExisting);
VERIFY_ARE_EQUAL(WindowingBehaviorUseExisting, result.WindowId());
VERIFY_ARE_EQUAL(L"", result.WindowName());

VERIFY_ARE_EQUAL(WindowingBehaviorUseCurrent,
appImpl::AppLogic::_doFindTargetWindow({ args }, WindowingMode::UseAnyExisting));
result = appImpl::AppLogic::_doFindTargetWindow({ args }, WindowingMode::UseAnyExisting);
VERIFY_ARE_EQUAL(WindowingBehaviorUseExisting, result.WindowId());
VERIFY_ARE_EQUAL(L"", result.WindowName());
}
{
Log::Comment(L"Make sure we follow the provided argument when a "
L"--window-id wasn't explicitly provided");

std::vector<winrt::hstring> args{ L"wt.exe", L"new-tab" };
VERIFY_ARE_EQUAL(WindowingBehaviorUseNew,
appImpl::AppLogic::_doFindTargetWindow({ args }, WindowingMode::UseNew));
auto result = appImpl::AppLogic::_doFindTargetWindow({ args }, WindowingMode::UseNew);
VERIFY_ARE_EQUAL(WindowingBehaviorUseNew, result.WindowId());
VERIFY_ARE_EQUAL(L"", result.WindowName());

result = appImpl::AppLogic::_doFindTargetWindow({ args }, WindowingMode::UseExisting);
VERIFY_ARE_EQUAL(WindowingBehaviorUseExisting, result.WindowId());
VERIFY_ARE_EQUAL(L"", result.WindowName());

result = appImpl::AppLogic::_doFindTargetWindow({ args }, WindowingMode::UseAnyExisting);
VERIFY_ARE_EQUAL(WindowingBehaviorUseAnyExisting, result.WindowId());
VERIFY_ARE_EQUAL(L"", result.WindowName());
}
{
Log::Comment(L"Even if someone uses a subcommand as a window name, "
L"that should work");

std::vector<winrt::hstring> args{ L"wt.exe", L"-w", L"new-tab" };
auto result = appImpl::AppLogic::_doFindTargetWindow({ args }, WindowingMode::UseNew);
VERIFY_ARE_EQUAL(WindowingBehaviorUseName, result.WindowId());
VERIFY_ARE_EQUAL(L"new-tab", result.WindowName());

VERIFY_ARE_EQUAL(WindowingBehaviorUseExisting,
appImpl::AppLogic::_doFindTargetWindow({ args }, WindowingMode::UseExisting));
result = appImpl::AppLogic::_doFindTargetWindow({ args }, WindowingMode::UseExisting);
VERIFY_ARE_EQUAL(WindowingBehaviorUseName, result.WindowId());
VERIFY_ARE_EQUAL(L"new-tab", result.WindowName());

VERIFY_ARE_EQUAL(WindowingBehaviorUseAnyExisting,
appImpl::AppLogic::_doFindTargetWindow({ args }, WindowingMode::UseAnyExisting));
result = appImpl::AppLogic::_doFindTargetWindow({ args }, WindowingMode::UseAnyExisting);
VERIFY_ARE_EQUAL(WindowingBehaviorUseName, result.WindowId());
VERIFY_ARE_EQUAL(L"new-tab", result.WindowName());
}
}

Expand All @@ -1657,19 +1753,23 @@ namespace TerminalAppLocalTests
// This is a little helper to make sure that these args _always_ return
// UseNew, regardless of the windowing behavior.
auto testHelper = [](auto&& args) {
VERIFY_ARE_EQUAL(WindowingBehaviorUseNew,
appImpl::AppLogic::_doFindTargetWindow({ args }, WindowingMode::UseNew));
auto result = appImpl::AppLogic::_doFindTargetWindow({ args }, WindowingMode::UseNew);
VERIFY_ARE_EQUAL(WindowingBehaviorUseNew, result.WindowId());
VERIFY_ARE_EQUAL(L"", result.WindowName());

VERIFY_ARE_EQUAL(WindowingBehaviorUseNew,
appImpl::AppLogic::_doFindTargetWindow({ args }, WindowingMode::UseExisting));
result = appImpl::AppLogic::_doFindTargetWindow({ args }, WindowingMode::UseExisting);
VERIFY_ARE_EQUAL(WindowingBehaviorUseNew, result.WindowId());
VERIFY_ARE_EQUAL(L"", result.WindowName());

VERIFY_ARE_EQUAL(WindowingBehaviorUseNew,
appImpl::AppLogic::_doFindTargetWindow({ args }, WindowingMode::UseAnyExisting));
result = appImpl::AppLogic::_doFindTargetWindow({ args }, WindowingMode::UseAnyExisting);
VERIFY_ARE_EQUAL(WindowingBehaviorUseNew, result.WindowId());
VERIFY_ARE_EQUAL(L"", result.WindowName());
};

testHelper(std::vector<winrt::hstring>{ L"wt.exe", L"--help" });
testHelper(std::vector<winrt::hstring>{ L"wt.exe", L"new-tab", L"--help" });
testHelper(std::vector<winrt::hstring>{ L"wt.exe", L"-w", L"0", L"new-tab", L"--help" });
testHelper(std::vector<winrt::hstring>{ L"wt.exe", L"-w", L"foo", L"new-tab", L"--help" });
testHelper(std::vector<winrt::hstring>{ L"wt.exe", L"new-tab", L";", L"--help" });
}

Expand All @@ -1680,14 +1780,17 @@ namespace TerminalAppLocalTests
// This is a little helper to make sure that these args _always_ return
// UseNew, regardless of the windowing behavior.
auto testHelper = [](auto&& args) {
VERIFY_ARE_EQUAL(WindowingBehaviorUseNew,
appImpl::AppLogic::_doFindTargetWindow({ args }, WindowingMode::UseNew));
auto result = appImpl::AppLogic::_doFindTargetWindow({ args }, WindowingMode::UseNew);
VERIFY_ARE_EQUAL(WindowingBehaviorUseNew, result.WindowId());
VERIFY_ARE_EQUAL(L"", result.WindowName());

VERIFY_ARE_EQUAL(WindowingBehaviorUseNew,
appImpl::AppLogic::_doFindTargetWindow({ args }, WindowingMode::UseExisting));
result = appImpl::AppLogic::_doFindTargetWindow({ args }, WindowingMode::UseExisting);
VERIFY_ARE_EQUAL(WindowingBehaviorUseNew, result.WindowId());
VERIFY_ARE_EQUAL(L"", result.WindowName());

VERIFY_ARE_EQUAL(WindowingBehaviorUseNew,
appImpl::AppLogic::_doFindTargetWindow({ args }, WindowingMode::UseAnyExisting));
result = appImpl::AppLogic::_doFindTargetWindow({ args }, WindowingMode::UseAnyExisting);
VERIFY_ARE_EQUAL(WindowingBehaviorUseNew, result.WindowId());
VERIFY_ARE_EQUAL(L"", result.WindowName());
};

testHelper(std::vector<winrt::hstring>{ L"wt.exe", L"--version" });
Expand Down
1 change: 1 addition & 0 deletions src/cascadia/Remoting/FindTargetWindowArgs.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -27,6 +27,7 @@ namespace winrt::Microsoft::Terminal::Remoting::implementation
{
WINRT_PROPERTY(winrt::Microsoft::Terminal::Remoting::CommandlineArgs, Args, nullptr);
WINRT_PROPERTY(int, ResultTargetWindow, -1);
WINRT_PROPERTY(winrt::hstring, ResultTargetWindowName);

public:
FindTargetWindowArgs(winrt::Microsoft::Terminal::Remoting::CommandlineArgs args) :
Expand Down
65 changes: 63 additions & 2 deletions src/cascadia/Remoting/Monarch.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -147,6 +147,59 @@ namespace winrt::Microsoft::Terminal::Remoting::implementation
}
}

// Method Description:
// - Find the ID of the peasant with the given name. If no such peasant
// exists, then we'll return 0. If we encounter any peasants who have died
// during this process, then we'll remove them from the set of _peasants
// Arguments:
// - name: The window name to look for
// Return Value:
// - 0 if we didn't find the given peasant, otherwise a positive number for
// the window's ID.
uint64_t Monarch::_lookupPeasantIdForName(std::wstring_view name)
{
if (name.empty())
{
return 0;
}

std::vector<uint64_t> peasantsToErase{};
uint64_t result = 0;
for (const auto& [id, p] : _peasants)
{
try
{
auto otherName = p.WindowName();
if (otherName == name)
{
result = id;
break;
}
}
catch (...)
{
LOG_CAUGHT_EXCEPTION();
// Normally, we'd just erase the peasant here. However, we can't
// erase from the map while we're iterating over it like this.
// Instead, pull a good ole Java and collect this id for removal
// later.
peasantsToErase.push_back(id);
}
}

// Remove the dead peasants we came across while iterating.
for (const auto& id : peasantsToErase)
{
// Remove the peasant from the list of peasants
_peasants.erase(id);
// Remove the peasant from the list of MRU windows. They're dead.
// They can't be the MRU anymore.
_clearOldMruEntries(id);
}

return result;
}

// Method Description:
// - Handler for the `Peasant::WindowActivated` event. We'll make a in-proc
// copy of the WindowActivatedArgs from the peasant. That way, we won't
Expand Down Expand Up @@ -400,6 +453,7 @@ namespace winrt::Microsoft::Terminal::Remoting::implementation
// After the event was handled, ResultTargetWindow() will be filled with
// the parsed result.
const auto targetWindow = findWindowArgs->ResultTargetWindow();
const auto targetWindowName = findWindowArgs->ResultTargetWindowName();

TraceLoggingWrite(g_hRemotingProvider,
"Monarch_ProposeCommandline",
Expand All @@ -410,6 +464,7 @@ namespace winrt::Microsoft::Terminal::Remoting::implementation
// that goes with it. Alternatively, if we were given a magic windowing
// constant, we can use that to look up an appropriate peasant.
if (targetWindow >= 0 ||
targetWindow == WindowingBehaviorUseName ||
targetWindow == WindowingBehaviorUseExisting ||
targetWindow == WindowingBehaviorUseAnyExisting)
{
Expand All @@ -429,6 +484,9 @@ namespace winrt::Microsoft::Terminal::Remoting::implementation
case WindowingBehaviorUseAnyExisting:
windowID = _getMostRecentPeasantID(false);
break;
case WindowingBehaviorUseName:
windowID = _lookupPeasantIdForName(targetWindowName);
break;
default:
windowID = ::base::saturated_cast<uint64_t>(targetWindow);
break;
Expand All @@ -449,7 +507,6 @@ namespace winrt::Microsoft::Terminal::Remoting::implementation
if (auto targetPeasant{ _getPeasant(windowID) })
{
auto result{ winrt::make_self<Remoting::implementation::ProposeCommandlineResult>(false) };

try
{
// This will raise the peasant's ExecuteCommandlineRequested
Expand All @@ -463,6 +520,7 @@ namespace winrt::Microsoft::Terminal::Remoting::implementation
// If we fail to propose the commandline to the peasant (it
// died?) then just tell this process to become a new window
// instead.
result->WindowName(targetWindowName);
result->ShouldCreateWindow(true);

// If this fails, it'll be logged in the following
Expand Down Expand Up @@ -497,6 +555,7 @@ namespace winrt::Microsoft::Terminal::Remoting::implementation

auto result{ winrt::make_self<Remoting::implementation::ProposeCommandlineResult>(true) };
result->Id(windowID);
result->WindowName(targetWindowName);
return *result;
}
}
Expand All @@ -508,6 +567,8 @@ namespace winrt::Microsoft::Terminal::Remoting::implementation
TraceLoggingLevel(WINEVENT_LEVEL_VERBOSE));

// In this case, no usable ID was provided. Return { true, nullopt }
return winrt::make<Remoting::implementation::ProposeCommandlineResult>(true);
auto result = winrt::make_self<Remoting::implementation::ProposeCommandlineResult>(true);
result->WindowName(targetWindowName);
return *result;
}
}

0 comments on commit 43c469f

Please sign in to comment.