feedback from review

microsoft · Mar 4, 2021 · c202257 · c202257
1 parent 27d12a2
commit c202257
Show file tree

Hide file tree

Showing 4 changed files with 11 additions and 14 deletions.
diff --git a/src/cascadia/Remoting/WindowManager.cpp b/src/cascadia/Remoting/WindowManager.cpp
@@ -71,7 +71,7 @@ namespace winrt::Microsoft::Terminal::Remoting::implementation
         // Otherwise, the King will tell us if we should make a new window
         _shouldCreateWindow = _isKing;
         std::optional<uint64_t> givenID;
-        winrt::hstring givenName = L""; // TODO:MG If we're the king, we might STILL WANT TO GET THE NAME. How do we get the name?
+        winrt::hstring givenName = L"";
         if (!_isKing)
         {
             // The monarch may respond back "you should be a new
@@ -123,7 +123,7 @@ namespace winrt::Microsoft::Terminal::Remoting::implementation
             auto findWindowArgs{ winrt::make_self<Remoting::implementation::FindTargetWindowArgs>(args) };
             _raiseFindTargetWindowRequested(nullptr, *findWindowArgs);
 
-            auto responseId = findWindowArgs->ResultTargetWindow();
+            const auto responseId = findWindowArgs->ResultTargetWindow();
             if (responseId > 0)
             {
                 givenID = ::base::saturated_cast<uint64_t>(responseId);

diff --git a/src/cascadia/TerminalApp/AppCommandlineArgs.cpp b/src/cascadia/TerminalApp/AppCommandlineArgs.cpp
@@ -859,11 +859,5 @@ void AppCommandlineArgs::FullResetState()
 
 std::string_view AppCommandlineArgs::GetTargetWindow() const noexcept
 {
-    // // If the user provides _any_ negative number, then treat it as -1, for "use a new window".
-    // if (_windowTarget.has_value() && *_windowTarget < 0)
-    // {
-    //     return { -1 };
-    // }
-
     return _windowTarget;
 }
diff --git a/src/cascadia/TerminalApp/AppLogic.cpp b/src/cascadia/TerminalApp/AppLogic.cpp
@@ -1240,7 +1240,7 @@ namespace winrt::TerminalApp::implementation
         {
             if (!appArgs.GetExitMessage().empty())
             {
-                return winrt::make<FindTargetWindowResult>(WindowingBehaviorUseNew, L"");
+                return winrt::make<FindTargetWindowResult>(WindowingBehaviorUseNew);
             }
 
             const std::string parsedTarget{ appArgs.GetTargetWindow() };
@@ -1263,7 +1263,7 @@ namespace winrt::TerminalApp::implementation
                     windowId = WindowingBehaviorUseAnyExisting;
                     break;
                 }
-                return winrt::make<FindTargetWindowResult>(windowId, L"");
+                return winrt::make<FindTargetWindowResult>(windowId);
             }
 
             // Here, the user _has_ provided a window-id on the commandline.
@@ -1282,7 +1282,7 @@ namespace winrt::TerminalApp::implementation
 
                 // Hooray! This is a valid integer. The set of possible values
                 // here is {-1, 0, ℤ+}. Let's return that window ID.
-                return winrt::make<TerminalApp::implementation::FindTargetWindowResult>(windowId, L"");
+                return winrt::make<FindTargetWindowResult>(windowId);
             }
             catch (...)
             {
@@ -1292,11 +1292,11 @@ namespace winrt::TerminalApp::implementation
                 // First, check the reserved keywords:
                 if (parsedTarget == "new")
                 {
-                    return winrt::make<FindTargetWindowResult>(WindowingBehaviorUseNew, L"");
+                    return winrt::make<FindTargetWindowResult>(WindowingBehaviorUseNew);
                 }
                 else if (parsedTarget == "last")
                 {
-                    return winrt::make<FindTargetWindowResult>(WindowingBehaviorUseExisting, L"");
+                    return winrt::make<FindTargetWindowResult>(WindowingBehaviorUseExisting);
                 }
                 else
                 {
@@ -1319,7 +1319,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<FindTargetWindowResult>(WindowingBehaviorUseNew, L"");
+        return winrt::make<FindTargetWindowResult>(WindowingBehaviorUseNew);
     }
 
     // Method Description:

diff --git a/src/cascadia/TerminalApp/AppLogic.h b/src/cascadia/TerminalApp/AppLogic.h
@@ -27,6 +27,9 @@ namespace winrt::TerminalApp::implementation
     public:
         FindTargetWindowResult(const int32_t id, const winrt::hstring& name) :
             _WindowId{ id }, _WindowName{ name } {};
+
+        FindTargetWindowResult(const int32_t id) :
+            FindTargetWindowResult(id, L""){};
     };
 
     struct AppLogic : AppLogicT<AppLogic, IInitializeWithWindow>