Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Replace New Tab Menu Match Profiles functionality with regex support #18654

Draft
wants to merge 1 commit into
base: main
Choose a base branch
from
Draft
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 3 additions & 0 deletions src/cascadia/TerminalApp/Resources/en-US/Resources.resw
Original file line number Diff line number Diff line change
Expand Up @@ -944,4 +944,7 @@
<data name="TabMoveRight" xml:space="preserve">
<value>Move right</value>
</data>
<data name="InvalidRegex" xml:space="preserve">
<value>An invalid regex was found.</value>
</data>
</root>
1 change: 1 addition & 0 deletions src/cascadia/TerminalApp/TerminalWindow.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -55,6 +55,7 @@ static const std::array settingsLoadWarningsLabels{
USES_RESOURCE(L"UnknownTheme"),
USES_RESOURCE(L"DuplicateRemainingProfilesEntry"),
USES_RESOURCE(L"InvalidUseOfContent"),
USES_RESOURCE(L"InvalidRegex"),
};

static_assert(settingsLoadWarningsLabels.size() == static_cast<size_t>(SettingsLoadWarnings::WARNINGS_SIZE));
Expand Down
2 changes: 2 additions & 0 deletions src/cascadia/TerminalSettingsEditor/NewTabMenu.xaml
Original file line number Diff line number Diff line change
Expand Up @@ -449,6 +449,8 @@
FontIconGlyph="&#xE748;"
Style="{StaticResource ExpanderSettingContainerStyleWithIcon}">
<StackPanel Spacing="10">
<HyperlinkButton x:Uid="NewTabMenu_AddMatchProfiles_Help"
NavigateUri="https://learn.microsoft.com/en-us/dotnet/standard/base-types/regular-expression-language-quick-reference" />
<TextBox x:Uid="NewTabMenu_AddMatchProfiles_Name"
Text="{x:Bind ViewModel.ProfileMatcherName, Mode=TwoWay}" />
<TextBox x:Uid="NewTabMenu_AddMatchProfiles_Source"
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -2105,7 +2105,7 @@
<comment>Header for a control that adds any remaining profiles to the new tab menu.</comment>
</data>
<data name="NewTabMenu_AddMatchProfiles.HelpText" xml:space="preserve">
<value>Add a group of profiles that match at least one of the defined properties</value>
<value>Add a group of profiles that match at least one of the defined regex properties</value>
<comment>Additional information for a control that adds a terminal profile matcher to the new tab menu. Presented near "NewTabMenu_AddMatchProfiles".</comment>
</data>
<data name="NewTabMenu_AddRemainingProfiles.HelpText" xml:space="preserve">
Expand All @@ -2121,15 +2121,15 @@
<comment>Header for a control that adds a folder to the new tab menu.</comment>
</data>
<data name="NewTabMenu_AddMatchProfiles_Name.Header" xml:space="preserve">
<value>Profile name</value>
<value>Profile name (Regex)</value>
<comment>Header for a text box used to define a regex for the names of profiles to add.</comment>
</data>
<data name="NewTabMenu_AddMatchProfiles_Source.Header" xml:space="preserve">
<value>Profile source</value>
<value>Profile source (Regex)</value>
<comment>Header for a text box used to define a regex for the sources of profiles to add.</comment>
</data>
<data name="NewTabMenu_AddMatchProfiles_Commandline.Header" xml:space="preserve">
<value>Commandline</value>
<value>Commandline (Regex)</value>
<comment>Header for a text box used to define a regex for the commandlines of profiles to add.</comment>
</data>
<data name="NewTabMenu_AddMatchProfilesTextBlock.Text" xml:space="preserve">
Expand Down Expand Up @@ -2340,4 +2340,7 @@
<value>This option is managed by enterprise policy and cannot be changed here.</value>
<comment>This is displayed in concordance with Globals_StartOnUserLogin if the enterprise administrator has taken control of this setting.</comment>
</data>
<data name="NewTabMenu_AddMatchProfiles_Help.Content" xml:space="preserve">
<value>Learn more about regular expressions</value>
</data>
</root>
44 changes: 44 additions & 0 deletions src/cascadia/TerminalSettingsModel/CascadiaSettings.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -429,6 +429,7 @@ void CascadiaSettings::_validateSettings()
_validateColorSchemesInCommands();
_validateThemeExists();
_validateProfileEnvironmentVariables();
_validateRegexes();
}

// Method Description:
Expand Down Expand Up @@ -583,6 +584,49 @@ void CascadiaSettings::_validateProfileEnvironmentVariables()
}
}

static void _validateRegex(const winrt::hstring& regex, IVector<Model::SettingsLoadWarnings>& warnings)
{
try
{
std::wregex{ regex.cbegin(), regex.cend() };
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

AGH! STD REGEX MY MORTAL ENEMY! 😄

That said, regex construction can be fairly costly in general, and we should not treat this as some kind of cheap validation. It involves constructing DFAs, etc., after all. I'm not entirely sure anymore where MatchProfilesEntry fits into the callstack, but seeing that it's also part of TerminalSettingsModel, I'm sure that we can make it cache every regex instance after load and store any warnings into a list. We can then splice its list into the overall SettingsLoadWarnings list (or store it directly there, or something like that). Basically, a way to only construct these once.

FWIW if you want to give icu.h a try:

#include "path/to/buffer/out/UTextAdapter.h"

// Cache this
UErrorCode status = U_ZERO_ERROR;
const auto re = Microsoft::Console::ICU::CreateRegex(pattern, 0, &status);
if (status > U_ZERO_ERROR) {
    // bad regex
}

// Run this
UErrorCode status = U_ZERO_ERROR;
uregex_setText(re.get(), hstring.data(), hstring.size(), &status);
const auto match = uregex_matches(re.get(), 0, &status);
return status == U_ZERO_ERROR && match;

}
catch (std::regex_error)
{
warnings.Append(Model::SettingsLoadWarnings::InvalidRegex);
}
}

static void _validateNTMEntries(const IVector<Model::NewTabMenuEntry>& entries, IVector<Model::SettingsLoadWarnings>& warnings)
{
for (const auto& ntmEntry : entries)
{
if (const auto& folderEntry = ntmEntry.try_as<Model::FolderEntry>())
{
_validateNTMEntries(folderEntry.RawEntries(), warnings);
}
if (const auto& matchProfilesEntry = ntmEntry.try_as<Model::MatchProfilesEntry>())
{
if (const auto nameRegex = matchProfilesEntry.Name(); !nameRegex.empty())
{
_validateRegex(nameRegex, warnings);
}
if (const auto commandlineRegex = matchProfilesEntry.Commandline(); !commandlineRegex.empty())
{
_validateRegex(commandlineRegex, warnings);
}
if (const auto sourceRegex = matchProfilesEntry.Source(); !sourceRegex.empty())
{
_validateRegex(sourceRegex, warnings);
}
}
}
}

void CascadiaSettings::_validateRegexes()
{
_validateNTMEntries(_globals->NewTabMenu(), _warnings);
}

// Method Description:
// - Helper to get the GUID of a profile, given an optional index and a possible
// "profile" value to override that.
Expand Down
1 change: 1 addition & 0 deletions src/cascadia/TerminalSettingsModel/CascadiaSettings.h
Original file line number Diff line number Diff line change
Expand Up @@ -175,6 +175,7 @@ namespace winrt::Microsoft::Terminal::Settings::Model::implementation
void _validateColorSchemesInCommands() const;
bool _hasInvalidColorScheme(const Model::Command& command) const;
void _validateThemeExists();
void _validateRegexes();

void _researchOnLoad();

Expand Down
26 changes: 23 additions & 3 deletions src/cascadia/TerminalSettingsModel/MatchProfilesEntry.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -55,19 +55,39 @@ namespace winrt::Microsoft::Terminal::Settings::Model::implementation
// value of the function we consider the null value to be "false".
auto isMatching = std::optional<bool>{};

auto isMatch = [](std::wstring_view regex, std::wstring_view text) {
if (text.empty())
{
return false;
}

std::wregex re;
try
{
re = { regex.cbegin(), regex.cend() };
}
catch (std::regex_error)
{
// invalid regex
return false;
}

return std::regex_match(text.cbegin(), text.cend(), re);
};

if (!_Name.empty())
{
isMatching = { isMatching.value_or(true) && _Name == profile.Name() };
isMatching = { isMatching.value_or(true) && isMatch(_Name, profile.Name()) };
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This optional-bool code seems like a roundabout way to say "return early if true". 😅

if (!field.empty() && isMatch(field, ...)) {
    return true;
}
if (!field.empty() && isMatch(field, ...)) {
    return true;
}
if (!field.empty() && isMatch(field, ...)) {
    return true;
}
return false;

}

if (!_Source.empty())
{
isMatching = { isMatching.value_or(true) && _Source == profile.Source() };
isMatching = { isMatching.value_or(true) && isMatch(_Source, profile.Source()) };
}

if (!_Commandline.empty())
{
isMatching = { isMatching.value_or(true) && _Commandline == profile.Commandline() };
isMatching = { isMatching.value_or(true) && isMatch(_Commandline, profile.Commandline()) };
}

return isMatching.value_or(false);
Expand Down
1 change: 1 addition & 0 deletions src/cascadia/TerminalSettingsModel/TerminalWarnings.idl
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,7 @@ namespace Microsoft.Terminal.Settings.Model
UnknownTheme,
DuplicateRemainingProfilesEntry,
InvalidUseOfContent,
InvalidRegex,
WARNINGS_SIZE // IMPORTANT: This MUST be the last value in this enum. It's an unused placeholder.
};

Expand Down
Loading