From 76609371396ba67b485ac62b70a6526c5942cf7f Mon Sep 17 00:00:00 2001 From: Mike Griese Date: Fri, 10 Feb 2023 13:04:59 -0600 Subject: [PATCH] and with that, we're ready to cleanup for review. 2 TODOs left --- src/cascadia/Remoting/Monarch.cpp | 8 ++++++ src/cascadia/TerminalApp/AppLogic.cpp | 11 ++++++-- src/cascadia/TerminalApp/AppLogic.h | 2 ++ src/cascadia/TerminalApp/AppLogic.idl | 11 +++++++- src/cascadia/WindowsTerminal/AppHost.cpp | 25 +++++++++++-------- src/cascadia/WindowsTerminal/AppHost.h | 2 ++ .../WindowsTerminal/WindowEmperor.cpp | 13 ++++++++-- 7 files changed, 57 insertions(+), 15 deletions(-) diff --git a/src/cascadia/Remoting/Monarch.cpp b/src/cascadia/Remoting/Monarch.cpp index f828fd05dbd..726882a8989 100644 --- a/src/cascadia/Remoting/Monarch.cpp +++ b/src/cascadia/Remoting/Monarch.cpp @@ -768,6 +768,14 @@ namespace winrt::Microsoft::Terminal::Remoting::implementation return *result; } } + else if (targetWindow == WindowingBehaviorUseNone) + { + // In this case, the targetWindow was UseNone, which means that we + // want to make a message box, but otherwise not make a Terminal + // window. + auto result = winrt::make_self(false); + return *result; + } // If we get here, we couldn't find an existing window. Make a new one. TraceLoggingWrite(g_hRemotingProvider, diff --git a/src/cascadia/TerminalApp/AppLogic.cpp b/src/cascadia/TerminalApp/AppLogic.cpp index 2105af24279..2717cec205d 100644 --- a/src/cascadia/TerminalApp/AppLogic.cpp +++ b/src/cascadia/TerminalApp/AppLogic.cpp @@ -565,7 +565,7 @@ namespace winrt::TerminalApp::implementation { if (!appArgs.GetExitMessage().empty()) { - return winrt::make(WindowingBehaviorUseNew); + return winrt::make(WindowingBehaviorUseNone); } const std::string parsedTarget{ appArgs.GetTargetWindow() }; @@ -644,7 +644,7 @@ namespace winrt::TerminalApp::implementation // create a new window. Then, in that new window, we'll try to set the // StartupActions, which will again fail, returning the correct error // message. - return winrt::make(WindowingBehaviorUseNew); + return winrt::make(WindowingBehaviorUseNone); } Windows::Foundation::Collections::IMapView AppLogic::GlobalHotkeys() @@ -699,4 +699,11 @@ namespace winrt::TerminalApp::implementation ApplicationState::SharedInstance().PersistedWindowLayouts(winrt::single_threaded_vector(std::move(converted))); } + + TerminalApp::ParseCommandlineResult AppLogic::GetParseCommandlineMessage(array_view args) + { + ::TerminalApp::AppCommandlineArgs _appArgs; + const auto r = _appArgs.ParseArgs(args); + return TerminalApp::ParseCommandlineResult{ winrt::to_hstring(_appArgs.GetExitMessage()), r}; + } } diff --git a/src/cascadia/TerminalApp/AppLogic.h b/src/cascadia/TerminalApp/AppLogic.h index 759cfb09500..fe2b2435c4a 100644 --- a/src/cascadia/TerminalApp/AppLogic.h +++ b/src/cascadia/TerminalApp/AppLogic.h @@ -67,6 +67,8 @@ namespace winrt::TerminalApp::implementation TerminalApp::TerminalWindow CreateNewWindow(); + TerminalApp::ParseCommandlineResult GetParseCommandlineMessage(array_view args); + TYPED_EVENT(SettingsChanged, winrt::Windows::Foundation::IInspectable, winrt::TerminalApp::SettingsLoadEventArgs); private: diff --git a/src/cascadia/TerminalApp/AppLogic.idl b/src/cascadia/TerminalApp/AppLogic.idl index a5db001de67..8499d53a52c 100644 --- a/src/cascadia/TerminalApp/AppLogic.idl +++ b/src/cascadia/TerminalApp/AppLogic.idl @@ -10,6 +10,13 @@ namespace TerminalApp String WindowName { get; }; }; + struct ParseCommandlineResult + { + String Message; + Int32 ExitCode; + }; + + // See IDialogPresenter and TerminalPage's DialogPresenter for more // information. [default_interface] runtimeclass AppLogic @@ -40,7 +47,9 @@ namespace TerminalApp TerminalWindow CreateNewWindow(); - Windows.Foundation.Collections.IMapView GlobalHotkeys(); + ParseCommandlineResult GetParseCommandlineMessage(String[] args); + + IMapView GlobalHotkeys(); event Windows.Foundation.TypedEventHandler SettingsChanged; diff --git a/src/cascadia/WindowsTerminal/AppHost.cpp b/src/cascadia/WindowsTerminal/AppHost.cpp index 8916ab85255..6ba6b8c3bfe 100644 --- a/src/cascadia/WindowsTerminal/AppHost.cpp +++ b/src/cascadia/WindowsTerminal/AppHost.cpp @@ -139,6 +139,19 @@ void AppHost::SetTaskbarProgress(const winrt::Windows::Foundation::IInspectable& } } +void AppHost::s_DisplayMessageBox(const winrt::TerminalApp::ParseCommandlineResult& result) +{ + const auto displayHelp = result.ExitCode == 0; + const auto messageTitle = displayHelp ? IDS_HELP_DIALOG_TITLE : IDS_ERROR_DIALOG_TITLE; + const auto messageIcon = displayHelp ? MB_ICONWARNING : MB_ICONERROR; + // TODO:GH#4134: polish this dialog more, to make the text more + // like msiexec /? + MessageBoxW(nullptr, + result.Message.data(), + GetStringResource(messageTitle).data(), + MB_OK | messageIcon); +} + // Method Description: // - Retrieve any commandline args passed on the commandline, and pass them to // the WindowManager, to ask if we should become a window process. @@ -171,19 +184,11 @@ void AppHost::_HandleCommandlineArgs() const auto message = _windowLogic.ParseCommandlineMessage(); if (!message.empty()) { - const auto displayHelp = result == 0; - const auto messageTitle = displayHelp ? IDS_HELP_DIALOG_TITLE : IDS_ERROR_DIALOG_TITLE; - const auto messageIcon = displayHelp ? MB_ICONWARNING : MB_ICONERROR; - // TODO:GH#4134: polish this dialog more, to make the text more - // like msiexec /? - MessageBoxW(nullptr, - message.data(), - GetStringResource(messageTitle).data(), - MB_OK | messageIcon); + AppHost::s_DisplayMessageBox({ message, result}); if (_windowLogic.ShouldExitEarly()) { - ExitProcess(result); + ExitThread(result); } } } diff --git a/src/cascadia/WindowsTerminal/AppHost.h b/src/cascadia/WindowsTerminal/AppHost.h index 7d576fc36e3..dc307261cd9 100644 --- a/src/cascadia/WindowsTerminal/AppHost.h +++ b/src/cascadia/WindowsTerminal/AppHost.h @@ -24,6 +24,8 @@ class AppHost bool HasWindow(); + static void s_DisplayMessageBox(const winrt::TerminalApp::ParseCommandlineResult& message); + winrt::TerminalApp::TerminalWindow Logic(); private: diff --git a/src/cascadia/WindowsTerminal/WindowEmperor.cpp b/src/cascadia/WindowsTerminal/WindowEmperor.cpp index 91cf84d3a2a..fcb9ecae594 100644 --- a/src/cascadia/WindowsTerminal/WindowEmperor.cpp +++ b/src/cascadia/WindowsTerminal/WindowEmperor.cpp @@ -60,6 +60,8 @@ WindowEmperor::~WindowEmperor() // hosts, xaml sources, everything, seem to get closed and dtor'd before // this point. Theoretically, we shouldn't event have leak reporting enabled // for this thread. It's a real thinker. + // + // I need someone to help take a look at this with me. _app.Close(); _app = nullptr; @@ -97,8 +99,6 @@ bool WindowEmperor::HandleCommandlineArgs() const auto result = _manager.ProposeCommandline2(eventArgs); - // TODO! createWindow is false in cases like wt --help. Figure that out. - if (result.ShouldCreateWindow()) { CreateNewWindowThread(Remoting::WindowRequestedArgs{ result, eventArgs }, true); @@ -109,6 +109,15 @@ bool WindowEmperor::HandleCommandlineArgs() _becomeMonarch(); } + else + { + const auto res = _app.Logic().GetParseCommandlineMessage(eventArgs.Commandline()); + if (!res.Message.empty()) + { + AppHost::s_DisplayMessageBox(res); + ExitThread(res.ExitCode); + } + } return result.ShouldCreateWindow(); }