Final cleanup for review

This commit is contained in:
Mike Griese 2020-12-17 14:26:06 -06:00
parent 0103331987
commit b4fe1bffbf
9 changed files with 57 additions and 17 deletions

View file

@ -16,6 +16,9 @@ namespace winrt::Microsoft::Terminal::Remoting::implementation
_ourPID{ GetCurrentProcessId() }
{
}
// This is a private constructor to be used in unit tests, where we don't
// want each Monarch to necessarily use the current PID.
Monarch::Monarch(const uint64_t testPID) :
_ourPID{ testPID }
{
@ -30,6 +33,12 @@ namespace winrt::Microsoft::Terminal::Remoting::implementation
return _ourPID;
}
// Method Description:
// - Add the given peasant to the list of peasants we're tracking. This Peasant may have already been assigned an ID. If it hasn't, then give it an ID.
// Arguments:
// - peasant: the new Peasant to track.
// Return Value:
// - the ID assigned to the peasant.
uint64_t Monarch::AddPeasant(Remoting::IPeasant peasant)
{
// TODO:projects/5 This is terrible. There's gotta be a better way
@ -55,11 +64,20 @@ namespace winrt::Microsoft::Terminal::Remoting::implementation
peasant.WindowActivated({ this, &Monarch::_peasantWindowActivated });
// TODO:projects/5 Wait on the peasant's PID, and remove them from the
// map if they die.
// map if they die. This won't work great in tests though, with fake
// PIDs.
return newPeasantsId;
}
// Method Description:
// - Event handler for the Peasant::WindowActivated event. Used as an
// opportunity for us to update our internal stack of the "most recent
// window".
// Arguments:
// - sender: the Peasant that raised this event. This might be out-of-proc!
// Return Value:
// - <none>
void Monarch::_peasantWindowActivated(const winrt::Windows::Foundation::IInspectable& sender,
const winrt::Windows::Foundation::IInspectable& /*args*/)
{
@ -94,8 +112,15 @@ namespace winrt::Microsoft::Terminal::Remoting::implementation
_mostRecentPeasant = peasantID;
}
bool Monarch::ProposeCommandline(array_view<const winrt::hstring> /*args*/,
winrt::hstring /*cwd*/)
// Method Description:
// - Try to handle a commandline from a new WT invocation. We might need to
// hand the commandline to an existing window, or we might need to tell
// the caller that they need to become a new window to handle it themself.
// Arguments:
// - <none>
// Return Value:
// - <none>
bool Monarch::ProposeCommandline(const Remoting::CommandlineArgs& /*args*/)
{
// TODO:projects/5
// The branch dev/migrie/f/remote-commandlines has a more complete

View file

@ -34,7 +34,7 @@ namespace winrt::Microsoft::Terminal::Remoting::implementation
uint64_t AddPeasant(winrt::Microsoft::Terminal::Remoting::IPeasant peasant);
bool ProposeCommandline(array_view<const winrt::hstring> args, winrt::hstring cwd);
bool ProposeCommandline(const winrt::Microsoft::Terminal::Remoting::CommandlineArgs& args);
private:
Monarch(const uint64_t testPID);

View file

@ -7,6 +7,6 @@ namespace Microsoft.Terminal.Remoting
UInt64 GetPID();
UInt64 AddPeasant(IPeasant peasant);
Boolean ProposeCommandline(String[] args, String cwd);
Boolean ProposeCommandline(CommandlineArgs args);
};
}

View file

@ -15,6 +15,9 @@ namespace winrt::Microsoft::Terminal::Remoting::implementation
_ourPID{ GetCurrentProcessId() }
{
}
// This is a private constructor to be used in unit tests, where we don't
// want each Peasant to necessarily use the current PID.
Peasant::Peasant(const uint64_t testPID) :
_ourPID{ testPID }
{
@ -36,11 +39,17 @@ namespace winrt::Microsoft::Terminal::Remoting::implementation
bool Peasant::ExecuteCommandline(const Remoting::CommandlineArgs& args)
{
// If this is the first set of args we were ever told about, stash them
// away. We'll need to get at them later, when we setup the startup
// actions for the window.
if (_initialArgs == nullptr)
{
_initialArgs = args;
}
// Raise an event with these args. The AppHost will listen for this
// event to know when to take these args and dispatch them to a
// currently-running window.
_ExecuteCommandlineRequestedHandlers(*this, args);
return true;

View file

@ -31,8 +31,7 @@ namespace winrt::Microsoft::Terminal::Remoting::implementation
_registrationHostClass = 0;
}
void WindowManager::ProposeCommandline(array_view<const winrt::hstring> args,
const winrt::hstring cwd)
void WindowManager::ProposeCommandline(const Remoting::CommandlineArgs& args)
{
const bool isKing = _areWeTheKing();
// If we're the king, we _definitely_ want to process the arguments, we were
@ -40,7 +39,7 @@ namespace winrt::Microsoft::Terminal::Remoting::implementation
//
// Otherwise, the King will tell us if we should make a new window
_shouldCreateWindow = isKing ||
_monarch.ProposeCommandline(args, cwd);
_monarch.ProposeCommandline(args);
if (_shouldCreateWindow)
{
@ -48,8 +47,7 @@ namespace winrt::Microsoft::Terminal::Remoting::implementation
// instance, and tell that peasant to handle that commandline.
_createOurPeasant();
auto eventArgs = winrt::make_self<implementation::CommandlineArgs>(args, cwd);
_peasant.ExecuteCommandline(*eventArgs);
_peasant.ExecuteCommandline(args);
}
// Othersize, we'll do _nothing_.
}

View file

@ -12,7 +12,7 @@ namespace winrt::Microsoft::Terminal::Remoting::implementation
WindowManager();
~WindowManager();
void ProposeCommandline(array_view<const winrt::hstring> args, const winrt::hstring cwd);
void ProposeCommandline(const winrt::Microsoft::Terminal::Remoting::CommandlineArgs& args);
bool ShouldCreateWindow();
winrt::Microsoft::Terminal::Remoting::Peasant CurrentWindow();

View file

@ -3,12 +3,11 @@ import "Peasant.idl";
namespace Microsoft.Terminal.Remoting
{
[default_interface] runtimeclass WindowManager
{
WindowManager();
void ProposeCommandline(String[] commands, String cwd);
void ProposeCommandline(CommandlineArgs args);
Boolean ShouldCreateWindow { get; };
Peasant CurrentWindow();
IPeasant CurrentWindow();
};
}

View file

@ -12,17 +12,23 @@ using namespace WEX::Common;
using namespace winrt;
using namespace winrt::Microsoft::Terminal;
// These are some gross macros that let us call a private ctor for
// Monarch/Peasant. We can't just use make_self, because that doesn't let us
// call a private ctor. We can use com_ptr::attach, but since we're allocating
// the thing on the stack, we need to make sure to call detach before the object
// is destructed.
#define MAKE_MONARCH(name, pid) \
Remoting::implementation::Monarch _local_##name##{ pid }; \
com_ptr<Remoting::implementation::Monarch> name; \
name.attach(&_local_##name##); \
auto cleanup = wil::scope_exit([&]() { name.detach(); });
auto cleanup_##name## = wil::scope_exit([&]() { name.detach(); });
#define MAKE_PEASANT(name, pid) \
Remoting::implementation::Peasant _local_##name##{ pid }; \
com_ptr<Remoting::implementation::Peasant> name; \
name.attach(&_local_##name##); \
auto cleanup = wil::scope_exit([&]() { name.detach(); });
auto cleanup_##name## = wil::scope_exit([&]() { name.detach(); });
namespace RemotingUnitTests
{

View file

@ -14,6 +14,7 @@ using namespace winrt::Windows::UI::Composition;
using namespace winrt::Windows::UI::Xaml;
using namespace winrt::Windows::UI::Xaml::Hosting;
using namespace winrt::Windows::Foundation::Numerics;
using namespace winrt::Microsoft::Terminal;
using namespace winrt::Microsoft::Terminal::Settings::Model;
using namespace ::Microsoft::Console;
using namespace ::Microsoft::Console::Types;
@ -147,7 +148,9 @@ void AppHost::_HandleCommandlineArgs()
std::vector<winrt::hstring> args;
_buildArgsFromCommandline(args);
std::wstring cwd{ wil::GetCurrentDirectoryW<std::wstring>() };
_windowManager.ProposeCommandline({ args }, { cwd });
Remoting::CommandlineArgs eventArgs{ { args }, { cwd } };
_windowManager.ProposeCommandline(eventArgs);
_shouldCreateWindow = _windowManager.ShouldCreateWindow();
if (!_shouldCreateWindow)