diff --git a/src/cascadia/Remoting/Monarch.cpp b/src/cascadia/Remoting/Monarch.cpp index 4f31609ebbb..f45247b2696 100644 --- a/src/cascadia/Remoting/Monarch.cpp +++ b/src/cascadia/Remoting/Monarch.cpp @@ -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(); @@ -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(); 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(targetWindow); @@ -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 }` + // + } } - // 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; } diff --git a/src/cascadia/Remoting/WindowManager.cpp b/src/cascadia/Remoting/WindowManager.cpp index 2780c6f03a1..f0b34d04e75 100644 --- a/src/cascadia/Remoting/WindowManager.cpp +++ b/src/cascadia/Remoting/WindowManager.cpp @@ -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(); @@ -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; diff --git a/src/cascadia/TerminalApp/AppLogic.cpp b/src/cascadia/TerminalApp/AppLogic.cpp index d611ba27c70..7393b248a80 100644 --- a/src/cascadia/TerminalApp/AppLogic.cpp +++ b/src/cascadia/TerminalApp/AppLogic.cpp @@ -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 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; }