Skip to content

Commit

Permalink
cleanup Peasant for review
Browse files Browse the repository at this point in the history
  • Loading branch information
zadjii-msft committed Dec 17, 2020
1 parent 9c6eac4 commit 0103331
Show file tree
Hide file tree
Showing 5 changed files with 44 additions and 40 deletions.
21 changes: 3 additions & 18 deletions src/cascadia/Remoting/Monarch.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -54,6 +54,9 @@ namespace winrt::Microsoft::Terminal::Remoting::implementation
// Add an event listener to the peasant's WindowActivated event.
peasant.WindowActivated({ this, &Monarch::_peasantWindowActivated });

// TODO:projects/5 Wait on the peasant's PID, and remove them from the
// map if they die.

return newPeasantsId;
}

Expand Down Expand Up @@ -91,24 +94,6 @@ namespace winrt::Microsoft::Terminal::Remoting::implementation
_mostRecentPeasant = peasantID;
}

void Monarch::SetSelfID(const uint64_t selfID)
{
this->_thisPeasantID = selfID;
// Right now, the monarch assumes the role of the most recent
// window. If the monarch dies, and a new monarch takes over, then the
// entire stack of MRU windows will go with it. That's not what you
// want!
//
// In the real app, we'll have each window also track the timestamp it
// was activated at, and the monarch will cache these. So a new monarch
// could re-query these last activated timestamps, and reconstruct the
// MRU stack.
//
// This is a sample though, and we're not too worried about complete
// correctness here.
_setMostRecentPeasant(_thisPeasantID);
}

bool Monarch::ProposeCommandline(array_view<const winrt::hstring> /*args*/,
winrt::hstring /*cwd*/)
{
Expand Down
2 changes: 0 additions & 2 deletions src/cascadia/Remoting/Monarch.h
Original file line number Diff line number Diff line change
Expand Up @@ -34,8 +34,6 @@ namespace winrt::Microsoft::Terminal::Remoting::implementation

uint64_t AddPeasant(winrt::Microsoft::Terminal::Remoting::IPeasant peasant);

void SetSelfID(const uint64_t selfID);

bool ProposeCommandline(array_view<const winrt::hstring> args, winrt::hstring cwd);

private:
Expand Down
24 changes: 7 additions & 17 deletions src/cascadia/Remoting/Peasant.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,12 @@ using namespace ::Microsoft::Console;

namespace winrt::Microsoft::Terminal::Remoting::implementation
{
Peasant::Peasant()
Peasant::Peasant() :
_ourPID{ GetCurrentProcessId() }
{
}
Peasant::Peasant(const uint64_t testPID) :
_ourPID{ testPID }
{
}

Expand All @@ -26,7 +31,7 @@ namespace winrt::Microsoft::Terminal::Remoting::implementation

uint64_t Peasant::GetPID()
{
return GetCurrentProcessId();
return _ourPID;
}

bool Peasant::ExecuteCommandline(const Remoting::CommandlineArgs& args)
Expand All @@ -38,24 +43,9 @@ namespace winrt::Microsoft::Terminal::Remoting::implementation

_ExecuteCommandlineRequestedHandlers(*this, args);

// auto argsProcessed = 0;
// std::wstring fullCmdline;
// for (const auto& arg : args)
// {
// fullCmdline += argsProcessed++ == 0 ? L"sample.exe" : arg;
// fullCmdline += L" ";
// }
// wprintf(L"\x1b[32mExecuted Commandline\x1b[m: \"");
// wprintf(fullCmdline.c_str());
// wprintf(L"\"\n");
return true;
}

void Peasant::raiseActivatedEvent()
{
_WindowActivatedHandlers(*this, nullptr);
}

Remoting::CommandlineArgs Peasant::InitialArgs()
{
return _initialArgs;
Expand Down
11 changes: 8 additions & 3 deletions src/cascadia/Remoting/Peasant.h
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,10 @@
#include "Peasant.g.h"
#include "../cascadia/inc/cppwinrt_utils.h"

namespace RemotingUnitTests
{
class RemotingTests;
};
namespace winrt::Microsoft::Terminal::Remoting::implementation
{
struct Peasant : public PeasantT<Peasant>
Expand All @@ -15,18 +19,19 @@ namespace winrt::Microsoft::Terminal::Remoting::implementation

bool ExecuteCommandline(const winrt::Microsoft::Terminal::Remoting::CommandlineArgs& args);

void raiseActivatedEvent();
winrt::Microsoft::Terminal::Remoting::CommandlineArgs InitialArgs();
TYPED_EVENT(WindowActivated, winrt::Windows::Foundation::IInspectable, winrt::Windows::Foundation::IInspectable);
TYPED_EVENT(ExecuteCommandlineRequested, winrt::Windows::Foundation::IInspectable, winrt::Microsoft::Terminal::Remoting::CommandlineArgs);

private:
Peasant(const uint64_t testPID);
uint64_t _ourPID;

uint64_t _id{ 0 };

winrt::Microsoft::Terminal::Remoting::CommandlineArgs _initialArgs{ nullptr };

// array_view<const winrt::hstring> _args;
// winrt::hstring _cwd;
friend class RemotingUnitTests::RemotingTests;
};
}

Expand Down
26 changes: 26 additions & 0 deletions src/cascadia/UnitTests_Remoting/RemotingTests.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,12 @@ using namespace winrt::Microsoft::Terminal;
name.attach(&_local_##name##); \
auto cleanup = 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(); });

namespace RemotingUnitTests
{
class RemotingTests
Expand All @@ -26,6 +32,7 @@ namespace RemotingUnitTests
END_TEST_CLASS()

TEST_METHOD(CreateMonarch);
TEST_METHOD(CreatePeasant);

TEST_CLASS_SETUP(ClassSetup)
{
Expand All @@ -52,4 +59,23 @@ namespace RemotingUnitTests
L"A Monarch with an explicit PID should use the one we provided");
}

void RemotingTests::CreatePeasant()
{
auto p1 = winrt::make_self<Remoting::implementation::Peasant>();
VERIFY_IS_NOT_NULL(p1);
VERIFY_ARE_EQUAL(GetCurrentProcessId(),
p1->GetPID(),
L"A Peasant without an explicit PID should use the current PID");

Log::Comment(L"That's what we need for window process management, but for tests, it'll be more useful to fake the PIDs.");

auto expectedFakePID = 2345u;
MAKE_PEASANT(p2, expectedFakePID);

VERIFY_IS_NOT_NULL(p2);
VERIFY_ARE_EQUAL(expectedFakePID,
p2->GetPID(),
L"A Peasant with an explicit PID should use the one we provided");
}

}

0 comments on commit 0103331

Please sign in to comment.