Skip to content

Commit

Permalink
notes, comments, cleanup
Browse files Browse the repository at this point in the history
  • Loading branch information
zadjii-msft committed Jan 6, 2021
1 parent bcbef34 commit 813dbc6
Show file tree
Hide file tree
Showing 3 changed files with 67 additions and 22 deletions.
37 changes: 26 additions & 11 deletions src/cascadia/Remoting/Monarch.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -139,7 +139,7 @@ namespace winrt::Microsoft::Terminal::Remoting::implementation
if (_mostRecentPeasant == 0)
{
// We haven't yet been told the MRU peasant. Just use the first one.
// TODO: GOD this is just gonna be a random one. Hacks on hacks on hacks
// TODO:MG GOD this is just gonna be a random one. Hacks on hacks on hacks
if (_peasants.size() > 0)
{
return _peasants.begin()->second.GetID();
Expand All @@ -164,18 +164,21 @@ namespace winrt::Microsoft::Terminal::Remoting::implementation
// to another window in this case.
bool Monarch::ProposeCommandline(const Remoting::CommandlineArgs& args)
{
// TODO:projects/5
// The branch dev/migrie/f/remote-commandlines has a more complete
// version of this function, with a naive implementation. For now, we
// always want to create a new window, so we'll just return true. This
// will tell the caller that we didn't handle the commandline, and they
// should open a new window to deal with it themselves.
// Raise an event, to ask how to handle this commandline. We can't ask
// the app ourselves - we exist isolated from that knowledge (and
// dependency hell). The WindowManager will raise this up to the app
// host, which will then ask the AppLogic, who will then parse the
// commandline and determine the provided ID of the window.
auto findWindowArgs = winrt::make_self<Remoting::implementation::FindTargetWindowArgs>();
findWindowArgs->Args(args);

_FindTargetWindowRequestedHandlers(*this, *findWindowArgs);

// After the event was handled, ResultTargetWindow() will be filled with
// the parsed result.
const auto targetWindow = findWindowArgs->ResultTargetWindow();

// TODO:projects/5 targetWindow==0 -> use the currently active window
// If there's a valid ID returned, then let's try and find the peasant that goes with it.
if (targetWindow >= 0)
{
uint64_t windowID = ::base::saturated_cast<uint64_t>(targetWindow);
Expand All @@ -188,12 +191,24 @@ namespace winrt::Microsoft::Terminal::Remoting::implementation
if (auto targetPeasant{ _getPeasant(windowID) })
{
targetPeasant.ExecuteCommandline(args);
// TODO:MG if the targeted peasant fails to execute the
// commandline, we should create our own window to display the
// message box.
return false;
}
else
{
// TODO:MG in this case, an ID was provided, but there's no
// peasant with that ID. Instead, we should tell the caller that
// they should make a new window, but _with that ID_.
//
// `Monarch::ProposeCommandline` needs to return a structure of
// `{ shouldCreateWindow: bool, givenID: optional<uint> }`
//
}
}
// TEMPORARY: if the target window is -1, then we want a new window. All
// other cases, just do it in this window (for now).
// return targetWindow == -1;

// TODO:MG in this case, no usable ID was provided. Return { true, nullopt }
return true;
}

Expand Down
18 changes: 14 additions & 4 deletions src/cascadia/Remoting/WindowManager.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -52,13 +52,22 @@ namespace winrt::Microsoft::Terminal::Remoting::implementation
_shouldCreateWindow = isKing ||
_monarch.ProposeCommandline(args);

// TODO:projects/5 The monarch may respond back "you should be a new
// window, with ID,name of (id, name)". Really the responses are:
// * You should not create a new window
// * Create a new window (but without a given ID or name). The Monarch
// will assign your ID/name later
// * Create a new window, and you'll have this ID or name
// - This is the case where the user provides `wt -w 1`, and there's
// no existing window 1

if (_shouldCreateWindow)
{
// If we should create a new window, then instantiate our Peasant
// instance, and tell that peasant to handle that commandline.
_createOurPeasant();

// TODO:MG Spawn a thread to wait on the monarch, and handle the election
// Spawn a thread to wait on the monarch, and handle the election
if (!isKing)
{
_createPeasantThread();
Expand Down Expand Up @@ -150,9 +159,10 @@ namespace winrt::Microsoft::Terminal::Remoting::implementation

if (_areWeTheKing())
{
// This is only called when a _new_ monarch is elected. We need to
// do this _always_, even on the first instance, which won't have an
// election
// This is only called when a _new_ monarch is elected. So don't do
// anything here that needs to be done for all monarch windows. This
// should only be for work that's done when a window _becomes_ a
// monarch, after the death of the previous monarch.
return true;
}
return false;
Expand Down
34 changes: 27 additions & 7 deletions src/cascadia/TerminalApp/AppLogic.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -1125,25 +1125,45 @@ namespace winrt::TerminalApp::implementation
return result; // TODO:MG does a return value make sense
}

// Method Description:
// - Parse the given commandline args in an attempt to find the specified
// window. The rest of the args are ignored for now (they'll be handled
// whenever the commandline fets to the window it was intended for).
// - Note that this function will only ever be called by the monarch. A
// return value of `0` in this case does not mean "run the commandline in
// _this_ process", rather it means "run the commandline in the current
// process", whoever that may be.
// Arguments:
// - args: an array of strings to process as a commandline. These args can contain spaces
// Return Value:
// - 0: We should handle the args "in the current window".
// - -1: We should handle the args in a new window
// - anything else: We should handle the commandline in the window with the given ID.
int32_t AppLogic::FindTargetWindow(array_view<const winrt::hstring> args)
{
::TerminalApp::AppCommandlineArgs appArgs;
auto result = appArgs.ParseArgs(args);
if (result == 0)
{
// TODO:MG Right now, any successful parse will end up in the same window
// return 0;
return appArgs.GetTargetWindow();

// TODO:projects/5 We'll want to use the windowingBehavior setting to determine
// well
// Maybe that'd be a special return value out of here, to tell the monarch to do something special
// TODO:projects/5
//
// In the future, we'll want to use the windowingBehavior setting to
// determine what happens when a window ID wasn't manually provided.
//
// Maybe that'd be a special return value out of here, to tell the
// monarch to do something special:
//
// -1 -> create a new window
// -2 -> find the mru, this desktop
// -3 -> MRU, any desktop
// -3 -> MRU, any desktop (is this not just 0?)
}

// Any unsuccessful parse will be a new window.
// Any unsuccessful parse will be a new window. That new window will try
// to handle the commandline iteslf, and find that the commandline
// failed to parse. When that happens, the new window will display the
// message box.
return -1;
}

Expand Down

1 comment on commit 813dbc6

@github-actions

This comment was marked as outdated.

Please sign in to comment.