Skip to content

Commit

Permalink
use std::move() on a few more strings, other general code tidying (mi…
Browse files Browse the repository at this point in the history
…crosoft#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 microsoft#1844
  • Loading branch information
yves-dolce authored and DHowett committed Aug 6, 2019
1 parent ff7fdbe commit dfb8536
Show file tree
Hide file tree
Showing 7 changed files with 25 additions and 20 deletions.
15 changes: 8 additions & 7 deletions src/cascadia/TerminalApp/App.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -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{};
Expand Down Expand Up @@ -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;
}
Expand Down Expand Up @@ -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);
}
Expand Down Expand Up @@ -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;
}
Expand Down
2 changes: 1 addition & 1 deletion src/cascadia/TerminalApp/CascadiaSettings.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -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" });

Expand Down
3 changes: 2 additions & 1 deletion src/cascadia/TerminalApp/ColorScheme.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -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]);
}
Expand Down
18 changes: 10 additions & 8 deletions src/cascadia/TerminalApp/Profile.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -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]);
}
Expand Down Expand Up @@ -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
Expand All @@ -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
Expand Down Expand Up @@ -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);
}

Expand Down
2 changes: 1 addition & 1 deletion src/cascadia/TerminalApp/Profile.h
Original file line number Diff line number Diff line change
Expand Up @@ -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;

Expand Down
3 changes: 2 additions & 1 deletion src/cascadia/TerminalSettings/TerminalSettings.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -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;
}

Expand Down
2 changes: 1 addition & 1 deletion src/cascadia/WindowsTerminal/WindowsTerminal.manifest
Original file line number Diff line number Diff line change
Expand Up @@ -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>
Expand Down

0 comments on commit dfb8536

Please sign in to comment.