Skip to content

Commit 4d67453

Browse files
Replace New Tab Menu Match Profiles functionality with regex support (#18654)
## Summary of the Pull Request Updates the New Tab Menu's Match Profiles entry to support regex instead of doing a direct match. Also adds validation to ensure the regex is valid. Updated the UI to help make it more clear that this supports regexes and even added a link to some helpful docs. ## Validation Steps Performed ✅ Invalid regex displays a warning ✅ Valid regex works nicely ✅ profile matcher with source=`Windows.Terminal.VisualStudio` still works as expected ## PR Checklist Closes #18553
1 parent 4d094df commit 4d67453

File tree

14 files changed

+161
-47
lines changed

14 files changed

+161
-47
lines changed

src/buffer/out/UTextAdapter.cpp

Lines changed: 0 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -400,17 +400,6 @@ Microsoft::Console::ICU::unique_utext Microsoft::Console::ICU::UTextFromTextBuff
400400
return ut;
401401
}
402402

403-
Microsoft::Console::ICU::unique_uregex Microsoft::Console::ICU::CreateRegex(const std::wstring_view& pattern, uint32_t flags, UErrorCode* status) noexcept
404-
{
405-
#pragma warning(suppress : 26490) // Don't use reinterpret_cast (type.1).
406-
const auto re = uregex_open(reinterpret_cast<const char16_t*>(pattern.data()), gsl::narrow_cast<int32_t>(pattern.size()), flags, nullptr, status);
407-
// ICU describes the time unit as being dependent on CPU performance and "typically [in] the order of milliseconds",
408-
// but this claim seems highly outdated already. On my CPU from 2021, a limit of 4096 equals roughly 600ms.
409-
uregex_setTimeLimit(re, 4096, status);
410-
uregex_setStackLimit(re, 4 * 1024 * 1024, status);
411-
return unique_uregex{ re };
412-
}
413-
414403
// Returns a half-open [beg,end) range given a text start and end position.
415404
// This function is designed to be used with uregex_start64/uregex_end64.
416405
til::point_span Microsoft::Console::ICU::BufferRangeFromMatch(UText* ut, URegularExpression* re)

src/buffer/out/UTextAdapter.h

Lines changed: 0 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -9,10 +9,8 @@ class TextBuffer;
99

1010
namespace Microsoft::Console::ICU
1111
{
12-
using unique_uregex = wistd::unique_ptr<URegularExpression, wil::function_deleter<decltype(&uregex_close), &uregex_close>>;
1312
using unique_utext = wil::unique_struct<UText, decltype(&utext_close), &utext_close>;
1413

1514
unique_utext UTextFromTextBuffer(const TextBuffer& textBuffer, til::CoordType rowBeg, til::CoordType rowEnd) noexcept;
16-
unique_uregex CreateRegex(const std::wstring_view& pattern, uint32_t flags, UErrorCode* status) noexcept;
1715
til::point_span BufferRangeFromMatch(UText* ut, URegularExpression* re);
1816
}

src/buffer/out/textBuffer.cpp

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -10,6 +10,7 @@
1010
#include "../../types/inc/CodepointWidthDetector.hpp"
1111
#include "../renderer/base/renderer.hpp"
1212
#include "../types/inc/utils.hpp"
13+
#include <til/regex.h>
1314
#include "search.h"
1415

1516
// BODGY: Misdiagnosis in MSVC 17.11: Referencing global constants in the member
@@ -3353,7 +3354,7 @@ std::optional<std::vector<til::point_span>> TextBuffer::SearchText(const std::ws
33533354
}
33543355

33553356
UErrorCode status = U_ZERO_ERROR;
3356-
const auto re = ICU::CreateRegex(needle, icuFlags, &status);
3357+
const auto re = til::ICU::CreateRegex(needle, icuFlags, &status);
33573358
if (status > U_ZERO_ERROR)
33583359
{
33593360
return std::nullopt;

src/cascadia/TerminalApp/Resources/en-US/Resources.resw

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -944,4 +944,7 @@
944944
<data name="TabMoveRight" xml:space="preserve">
945945
<value>Move right</value>
946946
</data>
947+
<data name="InvalidRegex" xml:space="preserve">
948+
<value>An invalid regex was found.</value>
949+
</data>
947950
</root>

src/cascadia/TerminalApp/TerminalWindow.cpp

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -55,6 +55,7 @@ static const std::array settingsLoadWarningsLabels{
5555
USES_RESOURCE(L"UnknownTheme"),
5656
USES_RESOURCE(L"DuplicateRemainingProfilesEntry"),
5757
USES_RESOURCE(L"InvalidUseOfContent"),
58+
USES_RESOURCE(L"InvalidRegex"),
5859
};
5960

6061
static_assert(settingsLoadWarningsLabels.size() == static_cast<size_t>(SettingsLoadWarnings::WARNINGS_SIZE));

src/cascadia/TerminalCore/Terminal.cpp

Lines changed: 6 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -12,6 +12,7 @@
1212
#include "../../buffer/out/UTextAdapter.h"
1313

1414
#include <til/hash.h>
15+
#include <til/regex.h>
1516
#include <winrt/Microsoft.Terminal.Core.h>
1617

1718
using namespace winrt::Microsoft::Terminal::Core;
@@ -1375,22 +1376,22 @@ struct URegularExpressionInterner
13751376
//
13761377
// An alternative approach would be to not make this method thread-safe and give each
13771378
// Terminal instance its own cache. I'm not sure which approach would have been better.
1378-
ICU::unique_uregex Intern(const std::wstring_view& pattern)
1379+
til::ICU::unique_uregex Intern(const std::wstring_view& pattern)
13791380
{
13801381
UErrorCode status = U_ZERO_ERROR;
13811382

13821383
{
13831384
const auto guard = _lock.lock_shared();
13841385
if (const auto it = _cache.find(pattern); it != _cache.end())
13851386
{
1386-
return ICU::unique_uregex{ uregex_clone(it->second.re.get(), &status) };
1387+
return til::ICU::unique_uregex{ uregex_clone(it->second.re.get(), &status) };
13871388
}
13881389
}
13891390

13901391
// Even if the URegularExpression creation failed, we'll insert it into the cache, because there's no point in retrying.
13911392
// (Apart from OOM but in that case this application will crash anyways in 3.. 2.. 1..)
1392-
auto re = ICU::CreateRegex(pattern, 0, &status);
1393-
ICU::unique_uregex clone{ uregex_clone(re.get(), &status) };
1393+
auto re = til::ICU::CreateRegex(pattern, 0, &status);
1394+
til::ICU::unique_uregex clone{ uregex_clone(re.get(), &status) };
13941395
std::wstring key{ pattern };
13951396

13961397
const auto guard = _lock.lock_exclusive();
@@ -1412,7 +1413,7 @@ struct URegularExpressionInterner
14121413
private:
14131414
struct CacheValue
14141415
{
1415-
ICU::unique_uregex re;
1416+
til::ICU::unique_uregex re;
14161417
size_t generation = 0;
14171418
};
14181419

src/cascadia/TerminalSettingsEditor/NewTabMenu.xaml

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -449,6 +449,8 @@
449449
FontIconGlyph="&#xE748;"
450450
Style="{StaticResource ExpanderSettingContainerStyleWithIcon}">
451451
<StackPanel Spacing="10">
452+
<HyperlinkButton x:Uid="NewTabMenu_AddMatchProfiles_Help"
453+
NavigateUri="https://learn.microsoft.com/en-us/dotnet/standard/base-types/regular-expression-language-quick-reference" />
452454
<TextBox x:Uid="NewTabMenu_AddMatchProfiles_Name"
453455
Text="{x:Bind ViewModel.ProfileMatcherName, Mode=TwoWay}" />
454456
<TextBox x:Uid="NewTabMenu_AddMatchProfiles_Source"

src/cascadia/TerminalSettingsEditor/Resources/en-US/Resources.resw

Lines changed: 7 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -2105,7 +2105,7 @@
21052105
<comment>Header for a control that adds any remaining profiles to the new tab menu.</comment>
21062106
</data>
21072107
<data name="NewTabMenu_AddMatchProfiles.HelpText" xml:space="preserve">
2108-
<value>Add a group of profiles that match at least one of the defined properties</value>
2108+
<value>Add a group of profiles that match at least one of the defined regex properties</value>
21092109
<comment>Additional information for a control that adds a terminal profile matcher to the new tab menu. Presented near "NewTabMenu_AddMatchProfiles".</comment>
21102110
</data>
21112111
<data name="NewTabMenu_AddRemainingProfiles.HelpText" xml:space="preserve">
@@ -2121,15 +2121,15 @@
21212121
<comment>Header for a control that adds a folder to the new tab menu.</comment>
21222122
</data>
21232123
<data name="NewTabMenu_AddMatchProfiles_Name.Header" xml:space="preserve">
2124-
<value>Profile name</value>
2124+
<value>Profile name (Regex)</value>
21252125
<comment>Header for a text box used to define a regex for the names of profiles to add.</comment>
21262126
</data>
21272127
<data name="NewTabMenu_AddMatchProfiles_Source.Header" xml:space="preserve">
2128-
<value>Profile source</value>
2128+
<value>Profile source (Regex)</value>
21292129
<comment>Header for a text box used to define a regex for the sources of profiles to add.</comment>
21302130
</data>
21312131
<data name="NewTabMenu_AddMatchProfiles_Commandline.Header" xml:space="preserve">
2132-
<value>Commandline</value>
2132+
<value>Commandline (Regex)</value>
21332133
<comment>Header for a text box used to define a regex for the commandlines of profiles to add.</comment>
21342134
</data>
21352135
<data name="NewTabMenu_AddMatchProfilesTextBlock.Text" xml:space="preserve">
@@ -2344,6 +2344,9 @@
23442344
<value>This option is managed by enterprise policy and cannot be changed here.</value>
23452345
<comment>This is displayed in concordance with Globals_StartOnUserLogin if the enterprise administrator has taken control of this setting.</comment>
23462346
</data>
2347+
<data name="NewTabMenu_AddMatchProfiles_Help.Content" xml:space="preserve">
2348+
<value>Learn more about regular expressions</value>
2349+
</data>
23472350
<data name="Appearance_BackgroundImageNone" xml:space="preserve">
23482351
<value>None</value>
23492352
<comment>Text displayed when the background image path is not defined.</comment>

src/cascadia/TerminalSettingsModel/CascadiaSettings.cpp

Lines changed: 37 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -4,6 +4,7 @@
44
#include "pch.h"
55
#include "CascadiaSettings.h"
66
#include "CascadiaSettings.g.cpp"
7+
#include "MatchProfilesEntry.h"
78

89
#include "DefaultTerminal.h"
910
#include "FileUtils.h"
@@ -429,6 +430,7 @@ void CascadiaSettings::_validateSettings()
429430
_validateColorSchemesInCommands();
430431
_validateThemeExists();
431432
_validateProfileEnvironmentVariables();
433+
_validateRegexes();
432434
}
433435

434436
// Method Description:
@@ -583,6 +585,41 @@ void CascadiaSettings::_validateProfileEnvironmentVariables()
583585
}
584586
}
585587

588+
// Returns true if all regexes in the new tab menu are valid, false otherwise
589+
static bool _validateNTMEntries(const IVector<Model::NewTabMenuEntry>& entries)
590+
{
591+
if (!entries)
592+
{
593+
return true;
594+
}
595+
for (const auto& ntmEntry : entries)
596+
{
597+
if (const auto& folderEntry = ntmEntry.try_as<Model::FolderEntry>())
598+
{
599+
if (!_validateNTMEntries(folderEntry.RawEntries()))
600+
{
601+
return false;
602+
}
603+
}
604+
if (const auto& matchProfilesEntry = ntmEntry.try_as<Model::MatchProfilesEntry>())
605+
{
606+
if (!winrt::get_self<Model::implementation::MatchProfilesEntry>(matchProfilesEntry)->ValidateRegexes())
607+
{
608+
return false;
609+
}
610+
}
611+
}
612+
return true;
613+
}
614+
615+
void CascadiaSettings::_validateRegexes()
616+
{
617+
if (!_validateNTMEntries(_globals->NewTabMenu()))
618+
{
619+
_warnings.Append(SettingsLoadWarnings::InvalidRegex);
620+
}
621+
}
622+
586623
// Method Description:
587624
// - Helper to get the GUID of a profile, given an optional index and a possible
588625
// "profile" value to override that.

src/cascadia/TerminalSettingsModel/CascadiaSettings.h

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -175,6 +175,7 @@ namespace winrt::Microsoft::Terminal::Settings::Model::implementation
175175
void _validateColorSchemesInCommands() const;
176176
bool _hasInvalidColorScheme(const Model::Command& command) const;
177177
void _validateThemeExists();
178+
void _validateRegexes();
178179

179180
void _researchOnLoad();
180181

src/cascadia/TerminalSettingsModel/MatchProfilesEntry.cpp

Lines changed: 51 additions & 21 deletions
Original file line numberDiff line numberDiff line change
@@ -36,41 +36,71 @@ namespace winrt::Microsoft::Terminal::Settings::Model::implementation
3636
auto entry = winrt::make_self<MatchProfilesEntry>();
3737

3838
JsonUtils::GetValueForKey(json, NameKey, entry->_Name);
39+
entry->_validateName();
40+
3941
JsonUtils::GetValueForKey(json, CommandlineKey, entry->_Commandline);
42+
entry->_validateCommandline();
43+
4044
JsonUtils::GetValueForKey(json, SourceKey, entry->_Source);
45+
entry->_validateSource();
4146

4247
return entry;
4348
}
4449

50+
// Returns true if all regexes are valid, false otherwise
51+
bool MatchProfilesEntry::ValidateRegexes() const
52+
{
53+
return !(_invalidName || _invalidCommandline || _invalidSource);
54+
}
55+
56+
#define DEFINE_VALIDATE_FUNCTION(name) \
57+
void MatchProfilesEntry::_validate##name() noexcept \
58+
{ \
59+
_invalid##name = false; \
60+
if (_##name.empty()) \
61+
{ \
62+
/* empty field is valid*/ \
63+
return; \
64+
} \
65+
UErrorCode status = U_ZERO_ERROR; \
66+
_##name##Regex = til::ICU::CreateRegex(_##name, 0, &status); \
67+
if (U_FAILURE(status)) \
68+
{ \
69+
_invalid##name = true; \
70+
_##name##Regex.reset(); \
71+
} \
72+
}
73+
74+
DEFINE_VALIDATE_FUNCTION(Name);
75+
DEFINE_VALIDATE_FUNCTION(Commandline);
76+
DEFINE_VALIDATE_FUNCTION(Source);
77+
4578
bool MatchProfilesEntry::MatchesProfile(const Model::Profile& profile)
4679
{
47-
// We use an optional here instead of a simple bool directly, since there is no
48-
// sensible default value for the desired semantics: the first property we want
49-
// to match on should always be applied (so one would set "true" as a default),
50-
// but if none of the properties are set, the default return value should be false
51-
// since this entry type is expected to behave like a positive match/whitelist.
52-
//
53-
// The easiest way to deal with this neatly is to use an optional, then for any
54-
// property to match we consider a null value to be "true", and for the return
55-
// value of the function we consider the null value to be "false".
56-
auto isMatching = std::optional<bool>{};
57-
58-
if (!_Name.empty())
80+
auto isMatch = [](const til::ICU::unique_uregex& regex, std::wstring_view text) {
81+
if (text.empty())
82+
{
83+
return false;
84+
}
85+
UErrorCode status = U_ZERO_ERROR;
86+
uregex_setText(regex.get(), reinterpret_cast<const UChar*>(text.data()), static_cast<int32_t>(text.size()), &status);
87+
const auto match = uregex_matches(regex.get(), 0, &status);
88+
return status == U_ZERO_ERROR && match;
89+
};
90+
91+
if (!_Name.empty() && isMatch(_NameRegex, profile.Name()))
5992
{
60-
isMatching = { isMatching.value_or(true) && _Name == profile.Name() };
93+
return true;
6194
}
62-
63-
if (!_Source.empty())
95+
else if (!_Source.empty() && isMatch(_SourceRegex, profile.Source()))
6496
{
65-
isMatching = { isMatching.value_or(true) && _Source == profile.Source() };
97+
return true;
6698
}
67-
68-
if (!_Commandline.empty())
99+
else if (!_Commandline.empty() && isMatch(_CommandlineRegex, profile.Commandline()))
69100
{
70-
isMatching = { isMatching.value_or(true) && _Commandline == profile.Commandline() };
101+
return true;
71102
}
72-
73-
return isMatching.value_or(false);
103+
return false;
74104
}
75105

76106
Model::NewTabMenuEntry MatchProfilesEntry::Copy() const

src/cascadia/TerminalSettingsModel/MatchProfilesEntry.h

Lines changed: 28 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -17,6 +17,30 @@ Author(s):
1717

1818
#include "ProfileCollectionEntry.h"
1919
#include "MatchProfilesEntry.g.h"
20+
#include <til/regex.h>
21+
22+
// This macro defines the getter and setter for a regex property.
23+
// The setter tries to instantiate the regex immediately and caches
24+
// it if successful. If it fails, it sets a boolean flag to track that
25+
// it failed.
26+
#define DEFINE_MATCH_PROFILE_REGEX_PROPERTY(name) \
27+
public: \
28+
hstring name() const noexcept \
29+
{ \
30+
return _##name; \
31+
} \
32+
void name(const hstring& value) noexcept \
33+
{ \
34+
_##name = value; \
35+
_validate##name(); \
36+
} \
37+
\
38+
private: \
39+
void _validate##name() noexcept; \
40+
\
41+
hstring _##name; \
42+
til::ICU::unique_uregex _##name##Regex; \
43+
bool _invalid##name{ false };
2044

2145
namespace winrt::Microsoft::Terminal::Settings::Model::implementation
2246
{
@@ -30,11 +54,12 @@ namespace winrt::Microsoft::Terminal::Settings::Model::implementation
3054
Json::Value ToJson() const override;
3155
static com_ptr<NewTabMenuEntry> FromJson(const Json::Value& json);
3256

57+
bool ValidateRegexes() const;
3358
bool MatchesProfile(const Model::Profile& profile);
3459

35-
WINRT_PROPERTY(winrt::hstring, Name);
36-
WINRT_PROPERTY(winrt::hstring, Commandline);
37-
WINRT_PROPERTY(winrt::hstring, Source);
60+
DEFINE_MATCH_PROFILE_REGEX_PROPERTY(Name)
61+
DEFINE_MATCH_PROFILE_REGEX_PROPERTY(Commandline)
62+
DEFINE_MATCH_PROFILE_REGEX_PROPERTY(Source)
3863
};
3964
}
4065

src/cascadia/TerminalSettingsModel/TerminalWarnings.idl

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -25,6 +25,7 @@ namespace Microsoft.Terminal.Settings.Model
2525
UnknownTheme,
2626
DuplicateRemainingProfilesEntry,
2727
InvalidUseOfContent,
28+
InvalidRegex,
2829
WARNINGS_SIZE // IMPORTANT: This MUST be the last value in this enum. It's an unused placeholder.
2930
};
3031

src/inc/til/regex.h

Lines changed: 22 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,22 @@
1+
// Copyright (c) Microsoft Corporation.
2+
// Licensed under the MIT license.
3+
4+
#pragma once
5+
6+
#include <icu.h>
7+
8+
namespace til::ICU // Terminal Implementation Library. Also: "Today I Learned"
9+
{
10+
using unique_uregex = wistd::unique_ptr<URegularExpression, wil::function_deleter<decltype(&uregex_close), &uregex_close>>;
11+
12+
_TIL_INLINEPREFIX unique_uregex CreateRegex(const std::wstring_view& pattern, uint32_t flags, UErrorCode* status) noexcept
13+
{
14+
#pragma warning(suppress : 26490) // Don't use reinterpret_cast (type.1).
15+
const auto re = uregex_open(reinterpret_cast<const char16_t*>(pattern.data()), gsl::narrow_cast<int32_t>(pattern.size()), flags, nullptr, status);
16+
// ICU describes the time unit as being dependent on CPU performance and "typically [in] the order of milliseconds",
17+
// but this claim seems highly outdated already. On my CPU from 2021, a limit of 4096 equals roughly 600ms.
18+
uregex_setTimeLimit(re, 4096, status);
19+
uregex_setStackLimit(re, 4 * 1024 * 1024, status);
20+
return unique_uregex{ re };
21+
}
22+
}

0 commit comments

Comments
 (0)