Add support for new panes with specifc profiles and other settings overrides

doc/cascadia/profiles.schema.json

@@ -104,6 +104,10 @@
         "profile": {
           "description": "Either the GUID or name of a profile to use, instead of launching the default",
           "type": "string"
+        },
+        "index": {
+          "type": "integer",
+          "description": "The index of the profile in the new tab dropdown to open"
         }
       },
       "type": "object"
@@ -143,11 +147,7 @@
         { "$ref": "#/definitions/NewTerminalArgs" },
         {
           "properties": {
-            "action": { "type":"string", "pattern": "newTab" },
-            "index": {
-              "type": "integer",
-              "description": "The index in the new tab dropdown to open in a new tab"
-            }
+            "action": { "type":"string", "pattern": "newTab" }
           }
         }
       ]

src/cascadia/LocalTests_TerminalApp/KeyBindingsTests.cpp

@@ -245,7 +245,8 @@ namespace TerminalAppLocalTests
             const auto& realArgs = actionAndArgs.Args().try_as<NewTabArgs>();
             VERIFY_IS_NOT_NULL(realArgs);
             // Verify the args have the expected value
-            VERIFY_IS_NULL(realArgs.ProfileIndex());
+            VERIFY_IS_NOT_NULL(realArgs.TerminalArgs());
+            VERIFY_IS_NULL(realArgs.TerminalArgs().ProfileIndex());
         }
         {
             Log::Comment(NoThrowString().Format(
@@ -256,8 +257,9 @@ namespace TerminalAppLocalTests
             const auto& realArgs = actionAndArgs.Args().try_as<NewTabArgs>();
             VERIFY_IS_NOT_NULL(realArgs);
             // Verify the args have the expected value
-            VERIFY_IS_NOT_NULL(realArgs.ProfileIndex());
-            VERIFY_ARE_EQUAL(0, realArgs.ProfileIndex().Value());
+            VERIFY_IS_NOT_NULL(realArgs.TerminalArgs());
+            VERIFY_IS_NOT_NULL(realArgs.TerminalArgs().ProfileIndex());
+            VERIFY_ARE_EQUAL(0, realArgs.TerminalArgs().ProfileIndex().Value());
         }
         {
             Log::Comment(NoThrowString().Format(
@@ -268,8 +270,9 @@ namespace TerminalAppLocalTests
             const auto& realArgs = actionAndArgs.Args().try_as<NewTabArgs>();
             VERIFY_IS_NOT_NULL(realArgs);
             // Verify the args have the expected value
-            VERIFY_IS_NOT_NULL(realArgs.ProfileIndex());
-            VERIFY_ARE_EQUAL(0, realArgs.ProfileIndex().Value());
+            VERIFY_IS_NOT_NULL(realArgs.TerminalArgs());
+            VERIFY_IS_NOT_NULL(realArgs.TerminalArgs().ProfileIndex());
+            VERIFY_ARE_EQUAL(0, realArgs.TerminalArgs().ProfileIndex().Value());
         }
         {
             Log::Comment(NoThrowString().Format(
@@ -281,8 +284,9 @@ namespace TerminalAppLocalTests
             const auto& realArgs = actionAndArgs.Args().try_as<NewTabArgs>();
             VERIFY_IS_NOT_NULL(realArgs);
             // Verify the args have the expected value
-            VERIFY_IS_NOT_NULL(realArgs.ProfileIndex());
-            VERIFY_ARE_EQUAL(11, realArgs.ProfileIndex().Value());
+            VERIFY_IS_NOT_NULL(realArgs.TerminalArgs());
+            VERIFY_IS_NOT_NULL(realArgs.TerminalArgs().ProfileIndex());
+            VERIFY_ARE_EQUAL(11, realArgs.TerminalArgs().ProfileIndex().Value());
         }
         {
             Log::Comment(NoThrowString().Format(
@@ -293,8 +297,9 @@ namespace TerminalAppLocalTests
             const auto& realArgs = actionAndArgs.Args().try_as<NewTabArgs>();
             VERIFY_IS_NOT_NULL(realArgs);
             // Verify the args have the expected value
-            VERIFY_IS_NOT_NULL(realArgs.ProfileIndex());
-            VERIFY_ARE_EQUAL(8, realArgs.ProfileIndex().Value());
+            VERIFY_IS_NOT_NULL(realArgs.TerminalArgs());
+            VERIFY_IS_NOT_NULL(realArgs.TerminalArgs().ProfileIndex());
+            VERIFY_ARE_EQUAL(8, realArgs.TerminalArgs().ProfileIndex().Value());
         }
 
         {

src/cascadia/TerminalApp/ActionArgs.h

@@ -80,7 +80,7 @@ namespace winrt::TerminalApp::implementation
             {
                 args->_TabTitle = winrt::to_hstring(tabTitle.asString());
             }
-            if (auto index{ json[JsonKey(TabTitleKey)] })
+            if (auto index{ json[JsonKey(ProfileIndexKey)] })
             {
                 args->_ProfileIndex = index.asInt();
             }

src/cascadia/TerminalApp/AppLogic.cpp

@@ -343,7 +343,7 @@ namespace winrt::TerminalApp::implementation
         }
 
         // Use the default profile to determine how big of a window we need.
-        TerminalSettings settings = _settings->MakeSettings(std::nullopt);
+        const auto [_, settings] = _settings->BuildSettings(nullptr);
 
         // TODO MSFT:21150597 - If the global setting "Always show tab bar" is
         // set or if "Show tabs in title bar" is set, then we'll need to add

src/cascadia/TerminalApp/CascadiaSettings.cpp

@@ -115,35 +115,6 @@ const Profile* CascadiaSettings::FindProfile(GUID profileGuid) const noexcept
     return nullptr;
 }
 
-// Method Description:
-// - Create a TerminalSettings object from the given profile.
-//      If the profileGuidArg is not provided, this method will use the default
-//      profile.
-//   The TerminalSettings object that is created can be used to initialize both
-//      the Control's settings, and the Core settings of the terminal.
-// Arguments:
-// - profileGuidArg: an optional GUID to use to lookup the profile to create the
-//      settings from. If this arg is not provided, or the GUID does not match a
-//       profile, then this method will use the default profile.
-// Return Value:
-// - <none>
-TerminalSettings CascadiaSettings::MakeSettings(std::optional<GUID> profileGuidArg) const
-{
-    GUID profileGuid = profileGuidArg ? profileGuidArg.value() : _globals.GetDefaultProfile();
-    const Profile* const profile = FindProfile(profileGuid);
-    if (profile == nullptr)
-    {
-        throw E_INVALIDARG;
-    }
-
-    TerminalSettings result = profile->CreateTerminalSettings(_globals.GetColorSchemes());
-
-    // Place our appropriate global settings into the Terminal Settings
-    _globals.ApplyToSettings(result);
-
-    return result;
-}
-
 // Method Description:
 // - Returns an iterable collection of all of our Profiles.
 // Arguments:
@@ -454,15 +425,12 @@ void CascadiaSettings::_ValidateAllSchemesExist()
 }
 
 // Method Description:
-// - Create a TerminalSettings object for the provided combination of
-//   profileIndex and newTerminalArgs. We'll use the index and newTerminalArgs
-//   to look up the profile that should be used to create these
-//   TerminalSettings. Then, we'll apply settings contained in the
+// - Create a TerminalSettings object for the provided newTerminalArgs. We'll
+//   use the newTerminalArgs to look up the profile that should be used to
+//   create these TerminalSettings. Then, we'll apply settings contained in the
 //   newTerminalArgs to the profile's settings, to enable customization on top
 //   of the profile's default values.
 // Arguments:
-// - index: if provided, the index in the list of profiles to get the GUID for.
-//   If omitted, instead use the default profile's GUID
 // - newTerminalArgs: An object that may contain a profile name or GUID to
 //   actually use. If the Profile value is not a guid, we'll treat it as a name,
 //   and attempt to look the profile up by name instead.
@@ -474,7 +442,7 @@ void CascadiaSettings::_ValidateAllSchemesExist()
 std::tuple<GUID, TerminalSettings> CascadiaSettings::BuildSettings(const NewTerminalArgs& newTerminalArgs) const
 {
     const GUID profileGuid = _GetProfileForArgs(newTerminalArgs);
-    auto settings = MakeSettings(profileGuid);
+    auto settings = BuildSettings(profileGuid);
 
     if (newTerminalArgs)
     {
@@ -496,6 +464,30 @@ std::tuple<GUID, TerminalSettings> CascadiaSettings::BuildSettings(const NewTerm
     return { profileGuid, settings };
 }
 
+// Method Description:
+// - Create a TerminalSettings object for the profile with a GUID matching the
+//   provided GUID. If no profile matches this GUID, then this method will
+//   throw.
+// Arguments:
+// - profileGuid: The GUID of a profile to use to create a settings object for.
+// Return Value:
+// - the GUID of the created profile, and a fully initialized TerminalSettings object
+TerminalSettings CascadiaSettings::BuildSettings(GUID profileGuid) const
+{
+    const Profile* const profile = FindProfile(profileGuid);
+    if (profile == nullptr)
+    {
+        throw E_INVALIDARG;
+    }
+
+    TerminalSettings result = profile->CreateTerminalSettings(_globals.GetColorSchemes());
+
+    // Place our appropriate global settings into the Terminal Settings
+    _globals.ApplyToSettings(result);
+
+    return result;
+}
+
 // Method Description:
 // - Helper to get the GUID of a profile, given an optional index and a possible
 //   "profile" value to override that.
@@ -532,33 +524,37 @@ GUID CascadiaSettings::_GetProfileForArgs(const NewTerminalArgs& newTerminalArgs
         {
             bool wasGuid = false;
 
-            try
+            // Do a quick heuristic check - is the profile 38 chars long (the
+            // length of a GUID string), and does it start with '{'? Because if
+            // it doesn't, it's _definitely_ not a GUID.
+            if (newTerminalArgs.Profile().size() == 38 && newTerminalArgs.Profile()[0] == L'{')
             {
-                const auto newGUID = Utils::GuidFromString(newTerminalArgs.Profile().c_str());
-
-                for (const auto& p : _profiles)
+                try
                 {
-                    if (p.GetGuid() == newGUID)
+                    const auto newGUID = Utils::GuidFromString(newTerminalArgs.Profile().c_str());
+
+                    for (const auto& p : _profiles)
                     {
-                        profileGuid = newGUID;
-                        wasGuid = true;
-                        break;
+                        if (p.GetGuid() == newGUID)
+                        {
+                            profileGuid = newGUID;
+                            wasGuid = true;
+                            break;
+                        }
                     }
                 }
+                CATCH_LOG();
             }
-            CATCH_LOG();
 
             // Here, we were unable to use the profile string as a GUID to
             // lookup a profile. Instead, try using the string to look the
             // Profile up by name.
             if (!wasGuid)
             {
-                for (const auto& p : _profiles)
+                const auto guidFromName = FindGuid(newTerminalArgs.Profile().c_str());
+                if (guidFromName.has_value())
                 {
-                    if (p.GetName() == newTerminalArgs.Profile())
-                    {
-                        profileGuid = p.GetGuid();
-                    }
+                    profileGuid = guidFromName.value();
                 }
             }
         }
@@ -584,7 +580,7 @@ GUID CascadiaSettings::_GetProfileForIndex(std::optional<int> index) const
     {
         const auto realIndex = index.value();
         // If we don't have that many profiles, then do nothing.
-        if (realIndex >= gsl::narrow<decltype(realIndex)>(_profiles.size()))
+        if (realIndex >= gsl::narrow_cast<decltype(realIndex)>(_profiles.size()))
         {
             return _globals.GetDefaultProfile();
         }

src/cascadia/TerminalApp/CascadiaSettings.h

@@ -51,8 +51,8 @@ class TerminalApp::CascadiaSettings final
 
     static const CascadiaSettings& GetCurrentAppSettings();
 
-    winrt::Microsoft::Terminal::Settings::TerminalSettings MakeSettings(std::optional<GUID> profileGuid) const;
     std::tuple<GUID, winrt::Microsoft::Terminal::Settings::TerminalSettings> BuildSettings(const winrt::TerminalApp::NewTerminalArgs& newTerminalArgs) const;
+    winrt::Microsoft::Terminal::Settings::TerminalSettings BuildSettings(GUID profileGuid) const;
 
     GlobalAppSettings& GlobalSettings();
 

src/cascadia/TerminalApp/TerminalPage.cpp

@@ -115,8 +115,11 @@ namespace winrt::TerminalApp::implementation
         _RegisterActionCallbacks();
 
         //Event Bindings (Early)
-        _newTabButton.Click([this](auto&&, auto&&) {
-            this->_OpenNewTab(nullptr);
+        _newTabButton.Click([weakThis](auto&&, auto&&) {
+            if (auto page{ weakThis.get() })
+            {
+                page->_OpenNewTab(nullptr);
+            }
         });
         _tabView.SelectionChanged({ this, &TerminalPage::_OnTabSelectionChanged });
         _tabView.TabCloseRequested({ this, &TerminalPage::_OnTabCloseRequested });
@@ -318,10 +321,15 @@ namespace winrt::TerminalApp::implementation
                 profileMenuItem.FontWeight(FontWeights::Bold());
             }
 
-            profileMenuItem.Click([this, profileIndex](auto&&, auto&&) {
-                auto newTerminalArgs = winrt::make_self<winrt::TerminalApp::implementation::NewTerminalArgs>();
-                newTerminalArgs->ProfileIndex(profileIndex);
-                this->_OpenNewTab(*newTerminalArgs);
+            auto weakThis{ get_weak() };
+
+            profileMenuItem.Click([profileIndex, weakThis](auto&&, auto&&) {
+                if (auto page{ weakThis.get() })
+                {
+                    auto newTerminalArgs = winrt::make_self<winrt::TerminalApp::implementation::NewTerminalArgs>();
+                    newTerminalArgs->ProfileIndex(profileIndex);
+                    page->_OpenNewTab(*newTerminalArgs);
+                }
             });
             newTabFlyout.Items().Append(profileMenuItem);
         }
@@ -701,9 +709,11 @@ namespace winrt::TerminalApp::implementation
         const auto& _tab = _tabs.at(focusedTabIndex);
 
         const auto& profileGuid = _tab->GetFocusedProfile();
-        const auto& settings = _settings->MakeSettings(profileGuid);
-
-        _CreateNewTabFromSettings(profileGuid.value(), settings);
+        if (profileGuid.has_value())
+        {
+            const auto settings = _settings->BuildSettings(profileGuid.value());
+            _CreateNewTabFromSettings(profileGuid.value(), settings);
+        }
     }
 
     // Method Description:
@@ -936,8 +946,9 @@ namespace winrt::TerminalApp::implementation
     // Arguments:
     // - splitType: one value from the TerminalApp::SplitState enum, indicating how the
     //   new pane should be split from its parent.
-    // - profile: The profile GUID to associate with the newly created pane. If
-    //   this is nullopt, use the default profile.
+    // - newTerminalArgs: An object that may contain a blob of parameters to
+    //   control which profile is created and with possible other
+    //   configurations. See CascadiaSettings::BuildSettings for more details.
     void TerminalPage::_SplitPane(const TerminalApp::SplitState splitType,
                                   const winrt::TerminalApp::NewTerminalArgs& newTerminalArgs)
     {
@@ -1350,7 +1361,7 @@ namespace winrt::TerminalApp::implementation
         for (auto& profile : profiles)
         {
             const GUID profileGuid = profile.GetGuid();
-            TerminalSettings settings = _settings->MakeSettings(profileGuid);
+            const auto settings = _settings->BuildSettings(profileGuid);
 
             for (auto& tab : _tabs)
             {