use std::move() on a few more strings, other general code tidying (#1899

)

* -  moving string parameter into data member instead of copying it.
-  removing noexcept from methods where an exception could be raised.
   If std::terminate() call is desired instead, I guess those should be
   left and std::move_if_noexcept() used to document the fact that it's
   on purpose.
- std::moving local variable into argument when possible.
- change maxversiontested XML element to maxVersionTested.
- used of gsl::narrow_cast where appropriate to prevent warnings.
- fixed bug in TerminalSettings::SetColorTableEntry()

Fixes #1844

src/cascadia/TerminalApp/App.cpp

@@ -313,7 +313,8 @@ namespace winrt::TerminalApp::implementation
         auto keyBindings = _settings->GetKeybindings();
 
         const GUID defaultProfileGuid = _settings->GlobalSettings().GetDefaultProfile();
-        for (int profileIndex = 0; profileIndex < _settings->GetProfiles().size(); profileIndex++)
+        auto const profileCount = gsl::narrow_cast<int>(_settings->GetProfiles().size()); // the number of profiles should not change in the loop for this to work
+        for (int profileIndex = 0; profileIndex < profileCount; profileIndex++)
         {
             const auto& profile = _settings->GetProfiles()[profileIndex];
             auto profileMenuItem = Controls::MenuFlyoutItem{};
@@ -794,7 +795,7 @@ namespace winrt::TerminalApp::implementation
             const auto profiles = _settings->GetProfiles();
 
             // If we don't have that many profiles, then do nothing.
-            if (realIndex >= profiles.size())
+            if (realIndex >= gsl::narrow<decltype(realIndex)>(profiles.size()))
             {
                 return;
             }
@@ -1082,7 +1083,7 @@ namespace winrt::TerminalApp::implementation
     // - Sets focus to the desired tab.
     void App::_SelectTab(const int tabIndex)
     {
-        if (tabIndex >= 0 && tabIndex < _tabs.size())
+        if (tabIndex >= 0 && tabIndex < gsl::narrow_cast<decltype(tabIndex)>(_tabs.size()))
         {
             _SetFocusedTabIndex(tabIndex);
         }
@@ -1224,12 +1225,12 @@ namespace winrt::TerminalApp::implementation
 
         if (tabIndexFromControl == focusedTabIndex)
         {
-            if (focusedTabIndex >= _tabs.size())
+            auto const tabCount = gsl::narrow_cast<decltype(focusedTabIndex)>(_tabs.size());
+            if (focusedTabIndex >= tabCount)
             {
-                focusedTabIndex = static_cast<int>(_tabs.size()) - 1;
+                focusedTabIndex = tabCount - 1;
             }
-
-            if (focusedTabIndex < 0)
+            else if (focusedTabIndex < 0)
             {
                 focusedTabIndex = 0;
             }

src/cascadia/TerminalApp/CascadiaSettings.cpp

@@ -246,7 +246,7 @@ void CascadiaSettings::_CreateDefaultProfiles()
     if (_isPowerShellCoreInstalled(psCoreCmdline))
     {
         auto pwshProfile{ _CreateDefaultProfile(L"PowerShell Core") };
-        pwshProfile.SetCommandline(psCoreCmdline);
+        pwshProfile.SetCommandline(std::move(psCoreCmdline));
         pwshProfile.SetStartingDirectory(DEFAULT_STARTING_DIRECTORY);
         pwshProfile.SetColorScheme({ L"Campbell" });
 

src/cascadia/TerminalApp/ColorScheme.cpp

@@ -66,7 +66,8 @@ void ColorScheme::ApplyScheme(TerminalSettings terminalSettings) const
     terminalSettings.DefaultForeground(_defaultForeground);
     terminalSettings.DefaultBackground(_defaultBackground);
 
-    for (int i = 0; i < _table.size(); i++)
+    auto const tableCount = gsl::narrow_cast<int>(_table.size());
+    for (int i = 0; i < tableCount; i++)
     {
         terminalSettings.SetColorTableEntry(i, _table[i]);
     }

src/cascadia/TerminalApp/Profile.cpp

@@ -150,7 +150,8 @@ TerminalSettings Profile::CreateTerminalSettings(const std::vector<ColorScheme>&
     TerminalSettings terminalSettings{};
 
     // Fill in the Terminal Setting's CoreSettings from the profile
-    for (int i = 0; i < _colorTable.size(); i++)
+    auto const colorTableCount = gsl::narrow_cast<int>(_colorTable.size());
+    for (int i = 0; i < colorTableCount; i++)
     {
         terminalSettings.SetColorTableEntry(i, _colorTable[i]);
     }
@@ -489,12 +490,12 @@ Profile Profile::FromJson(const Json::Value& json)
 
 void Profile::SetFontFace(std::wstring fontFace) noexcept
 {
-    _fontFace = fontFace;
+    _fontFace = std::move(fontFace);
 }
 
 void Profile::SetColorScheme(std::optional<std::wstring> schemeName) noexcept
 {
-    _schemeName = schemeName;
+    _schemeName = std::move(schemeName);
 }
 
 void Profile::SetAcrylicOpacity(double opacity) noexcept
@@ -504,17 +505,17 @@ void Profile::SetAcrylicOpacity(double opacity) noexcept
 
 void Profile::SetCommandline(std::wstring cmdline) noexcept
 {
-    _commandline = cmdline;
+    _commandline = std::move(cmdline);
 }
 
 void Profile::SetStartingDirectory(std::wstring startingDirectory) noexcept
 {
-    _startingDirectory = startingDirectory;
+    _startingDirectory = std::move(startingDirectory);
 }
 
 void Profile::SetName(std::wstring name) noexcept
 {
-    _name = name;
+    _name = std::move(name);
 }
 
 void Profile::SetUseAcrylic(bool useAcrylic) noexcept
@@ -553,15 +554,16 @@ bool Profile::HasIcon() const noexcept
 // - tabTitle: the tab title
 void Profile::SetTabTitle(std::wstring tabTitle) noexcept
 {
-    _tabTitle = tabTitle;
+    _tabTitle = std::move(tabTitle);
 }
 
 // Method Description:
 // - Sets this profile's icon path.
 // Arguments:
 // - path: the path
-void Profile::SetIconPath(std::wstring_view path) noexcept
+void Profile::SetIconPath(std::wstring_view path)
 {
+    static_assert(!noexcept(_icon.emplace(path)));
     _icon.emplace(path);
 }
 

src/cascadia/TerminalApp/Profile.h

@@ -56,7 +56,7 @@ class TerminalApp::Profile final
 
     bool HasIcon() const noexcept;
     std::wstring_view GetIconPath() const noexcept;
-    void SetIconPath(std::wstring_view path) noexcept;
+    void SetIconPath(std::wstring_view path);
 
     bool GetCloseOnExit() const noexcept;
 

src/cascadia/TerminalSettings/TerminalSettings.cpp

@@ -64,7 +64,8 @@ namespace winrt::Microsoft::Terminal::Settings::implementation
 
     void TerminalSettings::SetColorTableEntry(int32_t index, uint32_t value)
     {
-        THROW_HR_IF(E_INVALIDARG, index > _colorTable.size());
+        auto const colorTableCount = gsl::narrow_cast<decltype(index)>(_colorTable.size());
+        THROW_HR_IF(E_INVALIDARG, index >= colorTableCount);
         _colorTable[index] = value;
     }
 

src/cascadia/WindowsTerminal/WindowsTerminal.manifest

@@ -9,7 +9,7 @@
     <application>
         <!-- Windows 10 1903 -->
         <!-- See https://docs.microsoft.com/en-us/windows/apps/desktop/modernize/xaml-islands -->
-        <maxversiontested Id="10.0.18362.0"/>
+        <maxVersionTested Id="10.0.18362.0"/>
         <supportedOS Id="{8e0f7a12-bfb3-4fe8-b9a5-48fd50a15a9a}" />
     </application>
   </compatibility>