mirror of
https://github.com/microsoft/terminal.git
synced 2025-12-10 00:48:23 -06:00
GetGuid noexcept Fix (#3806)
<!-- Enter a brief description/summary of your PR here. What does it fix/what does it change/how was it tested (even manually, if necessary)? --> ## Summary of the Pull Request Fixed the noexcept specifier on `GetGuid`, and corrected `FindProfile` and `FindGuid` so they don't throw. Also, adjusted `SettingsTests` to reflect these changes. <!-- Other than the issue solved, is this relevant to any other issues/existing PRs? --> ## References <!-- Please review the items on the PR checklist before submitting--> ## PR Checklist * [x] Closes #3763 * [x] CLA signed. If not, go over [here](https://cla.opensource.microsoft.com/microsoft/Terminal) and sign the CLA * [x] Tests added/passed updated a test group in `SettingsTests` * [ ] Requires documentation to be updated * [x] I've discussed this with core contributors already. If not checked, I'm ready to accept this work might be rejected in favor of a different grand plan. Issue number where discussion took place: #3763 <!-- Provide a more detailed description of the PR, other things fixed or any additional comments/features here --> ## Detailed Description of the Pull Request / Additional comments The `noexcept` specifier on `GetGuid` was not removed when `Profile` was [updated](https://github.com/microsoft/terminal/issues/3763#issuecomment-559497094) to `std::optional<GUID>`. This PR fixes that and modifies two helper functions `FindProfile` and `FindGuid` in `CascadiaSettings` to work correctly if `GetGuid` does throw. <!-- Describe how you validated the behavior. Add automated tests wherever possible, but list manual validation steps taken as well --> ## Validation Steps Performed Updated the `TestHelperFunctions` test group in `SettingsTests` and made sure the tests pass.
This commit is contained in:
parent
b4673bd475
commit
a47a53c893
@ -1280,11 +1280,11 @@ namespace TerminalAppLocalTests
|
|||||||
"guid": "{6239a42c-6666-49a3-80bd-e8fdd045185c}"
|
"guid": "{6239a42c-6666-49a3-80bd-e8fdd045185c}"
|
||||||
},
|
},
|
||||||
{
|
{
|
||||||
"name" : "Ubuntu",
|
"name" : "ThisProfileShouldNotThrow"
|
||||||
"guid" : "{2C4DE342-38B7-51CF-B940-2309A097F518}"
|
|
||||||
},
|
},
|
||||||
{
|
{
|
||||||
"name" : "ThisProfileShouldNotCrash"
|
"name" : "Ubuntu",
|
||||||
|
"guid" : "{2C4DE342-38B7-51CF-B940-2309A097F518}"
|
||||||
}
|
}
|
||||||
]
|
]
|
||||||
})" };
|
})" };
|
||||||
@ -1292,11 +1292,13 @@ namespace TerminalAppLocalTests
|
|||||||
auto name0{ L"profile0" };
|
auto name0{ L"profile0" };
|
||||||
auto name1{ L"profile1" };
|
auto name1{ L"profile1" };
|
||||||
auto name2{ L"Ubuntu" };
|
auto name2{ L"Ubuntu" };
|
||||||
|
auto name3{ L"ThisProfileShouldNotThrow" };
|
||||||
auto badName{ L"DoesNotExist" };
|
auto badName{ L"DoesNotExist" };
|
||||||
|
|
||||||
auto guid0{ Microsoft::Console::Utils::GuidFromString(L"{6239a42c-5555-49a3-80bd-e8fdd045185c}") };
|
auto guid0{ Microsoft::Console::Utils::GuidFromString(L"{6239a42c-5555-49a3-80bd-e8fdd045185c}") };
|
||||||
auto guid1{ Microsoft::Console::Utils::GuidFromString(L"{6239a42c-6666-49a3-80bd-e8fdd045185c}") };
|
auto guid1{ Microsoft::Console::Utils::GuidFromString(L"{6239a42c-6666-49a3-80bd-e8fdd045185c}") };
|
||||||
auto guid2{ Microsoft::Console::Utils::GuidFromString(L"{2C4DE342-38B7-51CF-B940-2309A097F518}") };
|
auto guid2{ Microsoft::Console::Utils::GuidFromString(L"{2C4DE342-38B7-51CF-B940-2309A097F518}") };
|
||||||
|
auto fakeGuid{ Microsoft::Console::Utils::GuidFromString(L"{FFFFFFFF-FFFF-FFFF-FFFF-FFFFFFFFFFFF}") };
|
||||||
std::optional<GUID> badGuid{};
|
std::optional<GUID> badGuid{};
|
||||||
|
|
||||||
VerifyParseSucceeded(settings0String);
|
VerifyParseSucceeded(settings0String);
|
||||||
@ -1308,22 +1310,19 @@ namespace TerminalAppLocalTests
|
|||||||
VERIFY_ARE_EQUAL(guid0, settings.FindGuid(name0));
|
VERIFY_ARE_EQUAL(guid0, settings.FindGuid(name0));
|
||||||
VERIFY_ARE_EQUAL(guid1, settings.FindGuid(name1));
|
VERIFY_ARE_EQUAL(guid1, settings.FindGuid(name1));
|
||||||
VERIFY_ARE_EQUAL(guid2, settings.FindGuid(name2));
|
VERIFY_ARE_EQUAL(guid2, settings.FindGuid(name2));
|
||||||
|
VERIFY_ARE_EQUAL(badGuid, settings.FindGuid(name3));
|
||||||
VERIFY_ARE_EQUAL(badGuid, settings.FindGuid(badName));
|
VERIFY_ARE_EQUAL(badGuid, settings.FindGuid(badName));
|
||||||
// Following test will fail because GetGuid throws (despite being noexcept)
|
|
||||||
// VERIFY_ARE_EQUAL(badGuid, settings.FindGuid(L"ThisProfileShouldNotCrash"));
|
|
||||||
|
|
||||||
// Following lines only work because the GUID-less profile is last
|
|
||||||
auto prof0{ settings.FindProfile(guid0) };
|
auto prof0{ settings.FindProfile(guid0) };
|
||||||
auto prof1{ settings.FindProfile(guid1) };
|
auto prof1{ settings.FindProfile(guid1) };
|
||||||
auto prof2{ settings.FindProfile(guid2) };
|
auto prof2{ settings.FindProfile(guid2) };
|
||||||
// Following line will fail because GetGuid throws (despite being noexcept)
|
|
||||||
// auto badProf{ settings.FindProfile(badGuid) };
|
auto badProf{ settings.FindProfile(fakeGuid) };
|
||||||
|
VERIFY_ARE_EQUAL(badProf, nullptr);
|
||||||
|
|
||||||
VERIFY_ARE_EQUAL(name0, prof0->GetName());
|
VERIFY_ARE_EQUAL(name0, prof0->GetName());
|
||||||
VERIFY_ARE_EQUAL(name1, prof1->GetName());
|
VERIFY_ARE_EQUAL(name1, prof1->GetName());
|
||||||
VERIFY_ARE_EQUAL(name2, prof2->GetName());
|
VERIFY_ARE_EQUAL(name2, prof2->GetName());
|
||||||
// Following test will fail because GetGuid throws (despite being noexcept)
|
|
||||||
// VERIFY_ARE_EQUAL(badName, badProf->GetName());
|
|
||||||
}
|
}
|
||||||
|
|
||||||
void SettingsTests::TestLayerGlobalsOnRoot()
|
void SettingsTests::TestLayerGlobalsOnRoot()
|
||||||
|
|||||||
@ -78,7 +78,12 @@ std::optional<GUID> CascadiaSettings::FindGuid(const std::wstring& profileName)
|
|||||||
{
|
{
|
||||||
if (profileName == profile.GetName())
|
if (profileName == profile.GetName())
|
||||||
{
|
{
|
||||||
profileGuid = profile.GetGuid();
|
try
|
||||||
|
{
|
||||||
|
profileGuid = profile.GetGuid();
|
||||||
|
}
|
||||||
|
CATCH_LOG();
|
||||||
|
|
||||||
break;
|
break;
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
@ -98,10 +103,14 @@ const Profile* CascadiaSettings::FindProfile(GUID profileGuid) const noexcept
|
|||||||
{
|
{
|
||||||
for (auto& profile : _profiles)
|
for (auto& profile : _profiles)
|
||||||
{
|
{
|
||||||
if (profile.GetGuid() == profileGuid)
|
try
|
||||||
{
|
{
|
||||||
return &profile;
|
if (profile.GetGuid() == profileGuid)
|
||||||
|
{
|
||||||
|
return &profile;
|
||||||
|
}
|
||||||
}
|
}
|
||||||
|
CATCH_LOG();
|
||||||
}
|
}
|
||||||
return nullptr;
|
return nullptr;
|
||||||
}
|
}
|
||||||
|
|||||||
@ -138,7 +138,7 @@ bool Profile::HasSource() const noexcept
|
|||||||
return _source.has_value();
|
return _source.has_value();
|
||||||
}
|
}
|
||||||
|
|
||||||
GUID Profile::GetGuid() const noexcept
|
GUID Profile::GetGuid() const
|
||||||
{
|
{
|
||||||
// This can throw if we never had our guid set to a legitimate value.
|
// This can throw if we never had our guid set to a legitimate value.
|
||||||
THROW_HR_IF_MSG(E_FAIL, !_guid.has_value(), "Profile._guid always expected to have a value");
|
THROW_HR_IF_MSG(E_FAIL, !_guid.has_value(), "Profile._guid always expected to have a value");
|
||||||
|
|||||||
@ -64,7 +64,7 @@ public:
|
|||||||
|
|
||||||
bool HasGuid() const noexcept;
|
bool HasGuid() const noexcept;
|
||||||
bool HasSource() const noexcept;
|
bool HasSource() const noexcept;
|
||||||
GUID GetGuid() const noexcept;
|
GUID GetGuid() const;
|
||||||
void SetSource(std::wstring_view sourceNamespace) noexcept;
|
void SetSource(std::wstring_view sourceNamespace) noexcept;
|
||||||
std::wstring_view GetName() const noexcept;
|
std::wstring_view GetName() const noexcept;
|
||||||
bool HasConnectionType() const noexcept;
|
bool HasConnectionType() const noexcept;
|
||||||
|
|||||||
Loading…
x
Reference in New Issue
Block a user