Skip to content

Commit

Permalink
Add support for renaming windows (#9662)
Browse files Browse the repository at this point in the history
## Summary of the Pull Request

This PR adds support for renaming windows.

![window-renaming-000](https://user-images.githubusercontent.com/18356694/113034344-9a30be00-9157-11eb-9443-975f3c294f56.gif)
![window-renaming-001](https://user-images.githubusercontent.com/18356694/113034452-b5033280-9157-11eb-9e35-e5ac80fef0bc.gif)


It does so through two new actions:
* `renameWindow` takes a `name` parameter, and attempts to set the window's name
  to the provided name. This is useful if you always want to hit <kbd>F3</kbd>
  and rename a window to "foo" (READ: probably not that useful)
* `openWindowRenamer` is more interesting: it opens a `TeachingTip` with a
  `TextBox`. When the user hits Ok, it'll request a rename for the provided
  value. This lets the user pick a new name for the window at runtime.

In both cases, if there's already a window with that name, then the monarch will
reject the rename, and pop a `Toast` in the window informing the user that the
rename failed. Nifty!

## References
* Builds on the toasts from #9523
* #5000 - process model megathread

## PR Checklist
* [x] Closes https://github.com/microsoft/terminal/projects/5#card-50771747
* [x] I work here
* [x] Tests addded (and pass with the help of #9660)
* [ ] Requires documentation to be updated

## Detailed Description of the Pull Request / Additional comments

I'm sending this PR while finishing up the tests. I figured I'll have time to sneak them in before I get the necessary reviews.

> PAIN: We can't immediately focus the textbox in the TeachingTip. It's
> not technically focusable until it is opened. However, it doesn't
> provide an even tto tell us when it is opened. That's tracked in
> microsoft/microsoft-ui-xaml#1607. So for now, the user _needs_ to
> click on the text box manually.
> We're also not using a ContentDialog for this, because in Xaml
> Islands a text box in a ContentDialog won't recieve _any_ keypresses.
> Fun!

## Validation Steps Performed

I've been playing with 

```json
        { "keys": "f1", "command": "identifyWindow" },
        { "keys": "f2", "command": "identifyWindows" },
        { "keys": "f3", "command": "openWindowRenamer" },
        { "keys": "f4", "command": { "action": "renameWindow", "name": "foo" } },
        { "keys": "f5", "command": { "action": "renameWindow", "name": "bar" } },
```

and they seem to work as expected
  • Loading branch information
zadjii-msft authored Apr 2, 2021
1 parent 4b7d955 commit fb597ed
Show file tree
Hide file tree
Showing 31 changed files with 679 additions and 13 deletions.
36 changes: 36 additions & 0 deletions doc/cascadia/profiles.schema.json
Original file line number Diff line number Diff line change
Expand Up @@ -85,12 +85,14 @@
"openNewTabDropdown",
"openSettings",
"openTabColorPicker",
"openWindowRenamer",
"paste",
"prevTab",
"renameTab",
"openTabRenamer",
"resetFontSize",
"resizePane",
"renameWindow",
"scrollDown",
"scrollDownPage",
"scrollUp",
Expand Down Expand Up @@ -651,6 +653,38 @@
}
]
},
"RenameTabAction": {
"description": "Arguments corresponding to a renameTab Action",
"allOf": [
{ "$ref": "#/definitions/ShortcutAction" },
{
"properties": {
"action": { "type": "string", "pattern": "renameTab" },
"title": {
"type": "string",
"default": "",
"description": "A title to assign to the tab. If omitted or null, this action will restore the tab's title to the original value."
}
}
}
]
},
"RenameWindowAction": {
"description": "Arguments corresponding to a renameWindow Action",
"allOf": [
{ "$ref": "#/definitions/ShortcutAction" },
{
"properties": {
"action": { "type": "string", "pattern": "renameWindow" },
"name": {
"type": "string",
"default": "",
"description": "A name to assign to the window."
}
}
}
]
},
"Keybinding": {
"additionalProperties": false,
"properties": {
Expand Down Expand Up @@ -679,6 +713,8 @@
{ "$ref": "#/definitions/NewWindowAction" },
{ "$ref": "#/definitions/NextTabAction" },
{ "$ref": "#/definitions/PrevTabAction" },
{ "$ref": "#/definitions/RenameTabAction" },
{ "$ref": "#/definitions/RenameWindowAction" },
{ "type": "null" }
]
},
Expand Down
56 changes: 56 additions & 0 deletions src/cascadia/LocalTests_TerminalApp/TabTests.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -85,6 +85,9 @@ namespace TerminalAppLocalTests
TEST_METHOD(NextMRUTab);
TEST_METHOD(VerifyCommandPaletteTabSwitcherOrder);

TEST_METHOD(TestWindowRenameSuccessful);
TEST_METHOD(TestWindowRenameFailure);

TEST_CLASS_SETUP(ClassSetup)
{
return true;
Expand Down Expand Up @@ -948,4 +951,57 @@ namespace TerminalAppLocalTests
// will also dismiss itself immediately when that's called. So we can't
// really inspect the contents of the list in this test, unfortunately.
}

void TabTests::TestWindowRenameSuccessful()
{
auto page = _commonSetup();
page->RenameWindowRequested([&page](auto&&, const winrt::TerminalApp::RenameWindowRequestedArgs args) {
// In the real terminal, this would bounce up to the monarch and
// come back down. Instead, immediately call back and set the name.
page->WindowName(args.ProposedName());
});

bool windowNameChanged = false;
page->PropertyChanged([&page, &windowNameChanged](auto&&, const winrt::WUX::Data::PropertyChangedEventArgs& args) mutable {
if (args.PropertyName() == L"WindowNameForDisplay")
{
windowNameChanged = true;
}
});

TestOnUIThread([&page]() {
page->_RequestWindowRename(winrt::hstring{ L"Foo" });
});
TestOnUIThread([&]() {
VERIFY_ARE_EQUAL(L"Foo", page->_WindowName);
VERIFY_IS_TRUE(windowNameChanged,
L"The window name should have changed, and we should have raised a notification that WindowNameForDisplay changed");
});
}
void TabTests::TestWindowRenameFailure()
{
auto page = _commonSetup();
page->RenameWindowRequested([&page](auto&&, auto&&) {
// In the real terminal, this would bounce up to the monarch and
// come back down. Instead, immediately call back to tell the terminal it failed.
page->RenameFailed();
});

bool windowNameChanged = false;

page->PropertyChanged([&page, &windowNameChanged](auto&&, const winrt::WUX::Data::PropertyChangedEventArgs& args) mutable {
if (args.PropertyName() == L"WindowNameForDisplay")
{
windowNameChanged = true;
}
});

TestOnUIThread([&page]() {
page->_RequestWindowRename(winrt::hstring{ L"Foo" });
});
TestOnUIThread([&]() {
VERIFY_IS_FALSE(windowNameChanged,
L"The window name should not have changed, we should have rejected the change.");
});
}
}
1 change: 1 addition & 0 deletions src/cascadia/LocalTests_TerminalApp/pch.h
Original file line number Diff line number Diff line change
Expand Up @@ -45,6 +45,7 @@ Author(s):
#include <winrt/Windows.ui.input.h>
#include <winrt/Windows.UI.Xaml.Controls.h>
#include <winrt/Windows.UI.Xaml.Controls.Primitives.h>
#include <winrt/Windows.UI.Xaml.Data.h>
#include <winrt/Windows.ui.xaml.media.h>
#include <winrt/Windows.ui.xaml.input.h>
#include <winrt/Windows.UI.Xaml.Markup.h>
Expand Down
6 changes: 6 additions & 0 deletions src/cascadia/Remoting/Microsoft.Terminal.RemotingLib.vcxproj
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,9 @@
<ClInclude Include="ProposeCommandlineResult.h">
<DependentUpon>Monarch.idl</DependentUpon>
</ClInclude>
<ClInclude Include="RenameRequestArgs.h">
<DependentUpon>Peasant.idl</DependentUpon>
</ClInclude>
<ClInclude Include="WindowActivatedArgs.h">
<DependentUpon>Peasant.idl</DependentUpon>
</ClInclude>
Expand All @@ -51,6 +54,9 @@
<ClCompile Include="ProposeCommandlineResult.cpp">
<DependentUpon>Monarch.idl</DependentUpon>
</ClCompile>
<ClCompile Include="RenameRequestArgs.cpp">
<DependentUpon>Peasant.idl</DependentUpon>
</ClCompile>
<ClCompile Include="WindowActivatedArgs.cpp">
<DependentUpon>Peasant.idl</DependentUpon>
</ClCompile>
Expand Down
49 changes: 49 additions & 0 deletions src/cascadia/Remoting/Monarch.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -76,6 +76,7 @@ namespace winrt::Microsoft::Terminal::Remoting::implementation
// Add an event listener to the peasant's WindowActivated event.
peasant.WindowActivated({ this, &Monarch::_peasantWindowActivated });
peasant.IdentifyWindowsRequested({ this, &Monarch::_identifyWindows });
peasant.RenameRequested({ this, &Monarch::_renameRequested });

_peasants[newPeasantsId] = peasant;

Expand Down Expand Up @@ -631,4 +632,52 @@ namespace winrt::Microsoft::Terminal::Remoting::implementation
};
_forAllPeasantsIgnoringTheDead(callback, onError);
}

// Method Description:
// - This is an event handler for the RenameRequested event. A
// Peasant may raise that event when they want to be renamed to something else.
// - We will check if there are any other windows with this name. If there
// are, then we'll reject the rename by setting args.Succeeded=false.
// - If there aren't any other windows with this name, then we'll set
// args.Succeeded=true, allowing the window to keep this name.
// Arguments:
// - args: Contains the requested window name and a boolean (Succeeded)
// indicating if the request was successful.
// Return Value:
// - <none>
void Monarch::_renameRequested(const winrt::Windows::Foundation::IInspectable& /*sender*/,
const winrt::Microsoft::Terminal::Remoting::RenameRequestArgs& args)
{
bool successfullyRenamed = false;

try
{
args.Succeeded(false);
const auto name{ args.NewName() };
// Try to find a peasant that currently has this name
const auto id = _lookupPeasantIdForName(name);
if (_getPeasant(id) == nullptr)
{
// If there is one, then oh no! The requestor is not allowed to
// be renamed.
args.Succeeded(true);
successfullyRenamed = true;
}

TraceLoggingWrite(g_hRemotingProvider,
"Monarch_renameRequested",
TraceLoggingWideString(name.c_str(), "name", "The newly proposed name"),
TraceLoggingInt64(successfullyRenamed, "successfullyRenamed", "true if the peasant is allowed to rename themselves to that name."),
TraceLoggingLevel(WINEVENT_LEVEL_VERBOSE));
}
catch (...)
{
LOG_CAUGHT_EXCEPTION();
// If this fails, we don't _really_ care. The peasant died, but
// they're the only one who cares about the result.
TraceLoggingWrite(g_hRemotingProvider,
"Monarch_renameRequested_Failed",
TraceLoggingLevel(WINEVENT_LEVEL_VERBOSE));
}
}
}
8 changes: 6 additions & 2 deletions src/cascadia/Remoting/Monarch.h
Original file line number Diff line number Diff line change
Expand Up @@ -74,11 +74,15 @@ namespace winrt::Microsoft::Terminal::Remoting::implementation
void _doHandleActivatePeasant(const winrt::com_ptr<winrt::Microsoft::Terminal::Remoting::implementation::WindowActivatedArgs>& args);
void _clearOldMruEntries(const uint64_t peasantID);

void _forAllPeasantsIgnoringTheDead(std::function<void(const winrt::Microsoft::Terminal::Remoting::IPeasant&, const uint64_t)> callback,
std::function<void(const uint64_t)> errorCallback);

void _identifyWindows(const winrt::Windows::Foundation::IInspectable& sender,
const winrt::Windows::Foundation::IInspectable& args);

void _forAllPeasantsIgnoringTheDead(std::function<void(const winrt::Microsoft::Terminal::Remoting::IPeasant&, const uint64_t)> callback,
std::function<void(const uint64_t)> errorCallback);
void _renameRequested(const winrt::Windows::Foundation::IInspectable& sender,
const winrt::Microsoft::Terminal::Remoting::RenameRequestArgs& args);

friend class RemotingUnitTests::RemotingTests;
};
}
Expand Down
30 changes: 30 additions & 0 deletions src/cascadia/Remoting/Peasant.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -164,4 +164,34 @@ namespace winrt::Microsoft::Terminal::Remoting::implementation
TraceLoggingLevel(WINEVENT_LEVEL_VERBOSE));
}

void Peasant::RequestRename(const winrt::Microsoft::Terminal::Remoting::RenameRequestArgs& args)
{
bool successfullyNotified = false;
const auto oldName{ _WindowName };
try
{
// Try/catch this, because the other side of this event is handled
// by the monarch. The monarch might have died. If they have, this
// will throw an exception. Just eat it, the election thread will
// handle hooking up the new one.
_RenameRequestedHandlers(*this, args);
if (args.Succeeded())
{
_WindowName = args.NewName();
}
successfullyNotified = true;
}
catch (...)
{
LOG_CAUGHT_EXCEPTION();
}
TraceLoggingWrite(g_hRemotingProvider,
"Peasant_RequestRename",
TraceLoggingUInt64(GetID(), "peasantID", "Our ID"),
TraceLoggingWideString(oldName.c_str(), "oldName", "Our old name"),
TraceLoggingWideString(args.NewName().c_str(), "newName", "The proposed name"),
TraceLoggingBoolean(args.Succeeded(), "succeeded", "true if the monarch ok'd this new name for us."),
TraceLoggingBoolean(successfullyNotified, "successfullyNotified", "true if we successfully notified the monarch"),
TraceLoggingLevel(WINEVENT_LEVEL_VERBOSE));
}
}
3 changes: 3 additions & 0 deletions src/cascadia/Remoting/Peasant.h
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@

#include "Peasant.g.h"
#include "../cascadia/inc/cppwinrt_utils.h"
#include "RenameRequestArgs.h"

namespace RemotingUnitTests
{
Expand All @@ -24,6 +25,7 @@ namespace winrt::Microsoft::Terminal::Remoting::implementation
void ActivateWindow(const winrt::Microsoft::Terminal::Remoting::WindowActivatedArgs& args);
void RequestIdentifyWindows();
void DisplayWindowId();
void RequestRename(const winrt::Microsoft::Terminal::Remoting::RenameRequestArgs& args);

winrt::Microsoft::Terminal::Remoting::WindowActivatedArgs GetLastActivatedArgs();

Expand All @@ -34,6 +36,7 @@ namespace winrt::Microsoft::Terminal::Remoting::implementation
TYPED_EVENT(ExecuteCommandlineRequested, winrt::Windows::Foundation::IInspectable, winrt::Microsoft::Terminal::Remoting::CommandlineArgs);
TYPED_EVENT(IdentifyWindowsRequested, winrt::Windows::Foundation::IInspectable, winrt::Windows::Foundation::IInspectable);
TYPED_EVENT(DisplayWindowIdRequested, winrt::Windows::Foundation::IInspectable, winrt::Windows::Foundation::IInspectable);
TYPED_EVENT(RenameRequested, winrt::Windows::Foundation::IInspectable, winrt::Microsoft::Terminal::Remoting::RenameRequestArgs);

private:
Peasant(const uint64_t testPID);
Expand Down
10 changes: 10 additions & 0 deletions src/cascadia/Remoting/Peasant.idl
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,13 @@ namespace Microsoft.Terminal.Remoting
String CurrentDirectory();
};

runtimeclass RenameRequestArgs
{
RenameRequestArgs(String newName);
String NewName { get; };
Boolean Succeeded;
};

runtimeclass WindowActivatedArgs
{
WindowActivatedArgs(UInt64 peasantID, Guid desktopID, Windows.Foundation.DateTime activatedTime);
Expand All @@ -37,10 +44,13 @@ namespace Microsoft.Terminal.Remoting
void RequestIdentifyWindows(); // Tells us to raise a IdentifyWindowsRequested
void DisplayWindowId(); // Tells us to display its own ID (which causes a DisplayWindowIdRequested to be raised)

void RequestRename(RenameRequestArgs args); // Tells us to raise a RenameRequested

event Windows.Foundation.TypedEventHandler<Object, WindowActivatedArgs> WindowActivated;
event Windows.Foundation.TypedEventHandler<Object, CommandlineArgs> ExecuteCommandlineRequested;
event Windows.Foundation.TypedEventHandler<Object, Object> IdentifyWindowsRequested;
event Windows.Foundation.TypedEventHandler<Object, Object> DisplayWindowIdRequested;
event Windows.Foundation.TypedEventHandler<Object, RenameRequestArgs> RenameRequested;
};

[default_interface] runtimeclass Peasant : IPeasant
Expand Down
5 changes: 5 additions & 0 deletions src/cascadia/Remoting/RenameRequestArgs.cpp
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
// Copyright (c) Microsoft Corporation.
// Licensed under the MIT license.
#include "pch.h"
#include "RenameRequestArgs.h"
#include "RenameRequestArgs.g.cpp"
30 changes: 30 additions & 0 deletions src/cascadia/Remoting/RenameRequestArgs.h
Original file line number Diff line number Diff line change
@@ -0,0 +1,30 @@
/*++
Copyright (c) Microsoft Corporation
Licensed under the MIT license.
Class Name:
- RenameRequestArgs.h
--*/
#pragma once

#include "RenameRequestArgs.g.h"
#include "../cascadia/inc/cppwinrt_utils.h"

namespace winrt::Microsoft::Terminal::Remoting::implementation
{
struct RenameRequestArgs : public RenameRequestArgsT<RenameRequestArgs>
{
WINRT_PROPERTY(winrt::hstring, NewName);
WINRT_PROPERTY(bool, Succeeded, false);

public:
RenameRequestArgs(winrt::hstring newName) :
_NewName{ newName } {};
};
}

namespace winrt::Microsoft::Terminal::Remoting::factory_implementation
{
BASIC_FACTORY(RenameRequestArgs);
}
Loading

0 comments on commit fb597ed

Please sign in to comment.