Skip to content

Commit fa65a29

Browse files
lheckerDHowett
authored andcommitted
New tokenization & integer parsing helpers (microsoft#17962)
Reading through our existing patterns for integer parsing, I noticed that we'd be better off returning them as optionals. This also allowed me to improve the implementation to support integers all the way up to their absolute maximum/minimum. Furthermore, I noticed that `prefix_split` was unsound: If the last needle character was the last character in the remaining text, the remaining text would be updated to an empty string view. The caller would then have no idea if there's 1 more token left or if the string is truly empty. To solve this, this PR introduces an iterator class. This will allow it to be used in our VT parser code. (cherry picked from commit fa82730) Service-Card-Id: PVTI_lADOAF3p4s4AmhmQzgZtkTk Service-Version: 1.22
1 parent 0892689 commit fa65a29

File tree

9 files changed

+302
-248
lines changed

9 files changed

+302
-248
lines changed

src/cascadia/TerminalApp/Jumplist.cpp

Lines changed: 2 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -162,10 +162,9 @@ winrt::com_ptr<IShellLinkW> Jumplist::_createShellLink(const std::wstring_view n
162162
const std::wstring iconPath{ path.substr(0, commaPosition) };
163163

164164
// We dont want the comma included so add 1 to its position
165-
int iconIndex = til::to_int(path.substr(commaPosition + 1));
166-
if (iconIndex != til::to_int_error)
165+
if (const auto iconIndex = til::parse_signed<int>(path.substr(commaPosition + 1)))
167166
{
168-
THROW_IF_FAILED(sh->SetIconLocation(iconPath.data(), iconIndex));
167+
THROW_IF_FAILED(sh->SetIconLocation(iconPath.data(), *iconIndex));
169168
}
170169
}
171170
else if (til::ends_with(path, L"exe") || til::ends_with(path, L"dll"))

src/cascadia/TerminalSettingsModel/KeyChordSerialization.cpp

Lines changed: 4 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -101,10 +101,10 @@ static int32_t parseNumericCode(const std::wstring_view& str, const std::wstring
101101
return 0;
102102
}
103103

104-
const auto value = til::to_ulong({ str.data() + prefix.size(), str.size() - prefix.size() - suffix.size() });
105-
if (value > 0 && value < 256)
104+
const auto value = til::parse_unsigned<uint8_t>({ str.data() + prefix.size(), str.size() - prefix.size() - suffix.size() });
105+
if (value)
106106
{
107-
return gsl::narrow_cast<int32_t>(value);
107+
return gsl::narrow_cast<int32_t>(*value);
108108
}
109109

110110
throw winrt::hresult_invalid_argument(L"Invalid numeric argument to vk() or sc()");
@@ -151,10 +151,8 @@ static KeyChord _fromString(std::wstring_view wstr)
151151
auto vkey = 0;
152152
auto scanCode = 0;
153153

154-
while (!wstr.empty())
154+
for (const auto& part : til::split_iterator{ wstr, L'+' })
155155
{
156-
const auto part = til::prefix_split(wstr, L'+');
157-
158156
if (til::equals_insensitive_ascii(part, CTRL_KEY))
159157
{
160158
modifiers |= VirtualKeyModifiers::Control;

src/cascadia/TerminalSettingsModel/TerminalSettingsSerializationHelpers.h

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -719,8 +719,8 @@ struct ::Microsoft::Terminal::Settings::Model::JsonUtils::ConversionTrait<::winr
719719
if (isIndexed16)
720720
{
721721
const auto indexStr = string.substr(1);
722-
const auto idx = til::to_ulong(indexStr, 16);
723-
color.r = gsl::narrow_cast<uint8_t>(std::min(idx, 15ul));
722+
const auto idx = til::parse_unsigned<uint8_t>(indexStr, 16);
723+
color.r = std::min<uint8_t>(idx.value_or(255), 15);
724724
}
725725
else
726726
{

src/cascadia/UIHelpers/Converters.cpp

Lines changed: 16 additions & 19 deletions
Original file line numberDiff line numberDiff line change
@@ -73,35 +73,32 @@ namespace winrt::Microsoft::Terminal::UI::implementation
7373

7474
double Converters::MaxValueFromPaddingString(const winrt::hstring& paddingString)
7575
{
76-
std::wstring_view remaining{ paddingString };
76+
std::wstring buffer;
7777
double maxVal = 0;
7878

79+
auto& errnoRef = errno; // Nonzero cost, pay it once
80+
7981
// Get padding values till we run out of delimiter separated values in the stream
8082
// Non-numeral values detected will default to 0
8183
// std::stod will throw invalid_argument exception if the input is an invalid double value
8284
// std::stod will throw out_of_range exception if the input value is more than DBL_MAX
83-
try
85+
for (const auto& part : til::split_iterator{ std::wstring_view{ paddingString }, L',' })
8486
{
85-
while (!remaining.empty())
87+
buffer.assign(part);
88+
89+
// wcstod handles whitespace prefix (which is ignored) & stops the
90+
// scan when first char outside the range of radix is encountered.
91+
// We'll be permissive till the extent that stod function allows us to be by default
92+
// Ex. a value like 100.3#535w2 will be read as 100.3, but ;df25 will fail
93+
errnoRef = 0;
94+
wchar_t* end;
95+
const double val = wcstod(buffer.c_str(), &end);
96+
97+
if (end != buffer.c_str() && errnoRef != ERANGE)
8698
{
87-
const std::wstring token{ til::prefix_split(remaining, L',') };
88-
// std::stod internally calls wcstod which handles whitespace prefix (which is ignored)
89-
// & stops the scan when first char outside the range of radix is encountered
90-
// We'll be permissive till the extent that stod function allows us to be by default
91-
// Ex. a value like 100.3#535w2 will be read as 100.3, but ;df25 will fail
92-
const auto curVal = std::stod(token);
93-
if (curVal > maxVal)
94-
{
95-
maxVal = curVal;
96-
}
99+
maxVal = std::max(maxVal, val);
97100
}
98101
}
99-
catch (...)
100-
{
101-
// If something goes wrong, even if due to a single bad padding value, we'll return default 0 padding
102-
maxVal = 0;
103-
LOG_CAUGHT_EXCEPTION();
104-
}
105102

106103
return maxVal;
107104
}

src/cascadia/UIHelpers/IconPathConverter.cpp

Lines changed: 1 addition & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -271,12 +271,7 @@ namespace winrt::Microsoft::Terminal::UI::implementation
271271
if (commaIndex != std::wstring::npos)
272272
{
273273
// Convert the string iconIndex to a signed int to support negative numbers which represent an Icon's ID.
274-
const auto index{ til::to_int(pathView.substr(commaIndex + 1)) };
275-
if (index == til::to_int_error)
276-
{
277-
return std::nullopt;
278-
}
279-
return static_cast<int>(index);
274+
return til::parse_signed<int>(pathView.substr(commaIndex + 1));
280275
}
281276

282277
// We had a binary path, but no index. Default to 0.

src/common.build.pre.props

Lines changed: 11 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -150,7 +150,17 @@
150150
<RemoveUnreferencedCodeData>true</RemoveUnreferencedCodeData>
151151
<LanguageStandard>stdcpp20</LanguageStandard>
152152
<LanguageStandard_C>stdc17</LanguageStandard_C>
153-
<AdditionalOptions>%(AdditionalOptions) /utf-8 /Zc:__cplusplus /Zc:__STDC__ /Zc:enumTypes /Zc:externConstexpr /Zc:templateScope /Zc:throwingNew</AdditionalOptions>
153+
<!--
154+
Conformance options that are off by default at the time of writing:
155+
/Zc:__cplusplus Enable the __cplusplus macro to report the supported standard.
156+
/Zc:__STDC__ Enable the __STDC__ macro to report the C standard is supported.
157+
/Zc:enumTypes Enable Standard C++ rules for enum type deduction.
158+
/Zc:inline Remove unreferenced functions or data if they're COMDAT or have internal linkage only.
159+
/Zc:templateScope Enforce Standard C++ template parameter shadowing rules.
160+
/Zc:throwingNew Assume operator new throws on failure.
161+
/diagnostics:caret Places ^^^^^ under the exact ___location of the issue. The default only shows the line number.
162+
-->
163+
<AdditionalOptions>%(AdditionalOptions) /utf-8 /Zc:__cplusplus /Zc:__STDC__ /Zc:enumTypes /Zc:inline /Zc:templateScope /Zc:throwingNew /diagnostics:caret</AdditionalOptions>
154164
<ControlFlowGuard>Guard</ControlFlowGuard>
155165
</ClCompile>
156166
<ResourceCompile>

0 commit comments

Comments
 (0)