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

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

carlos-zamora
Copy link
Member

@carlos-zamora carlos-zamora commented Mar 5, 2025

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

PR Checklist

Closes #18553

@microsoft-github-policy-service microsoft-github-policy-service bot added Issue-Feature Complex enough to require an in depth planning process and actual budgeted, scheduled work. Area-Settings Issues related to settings and customizability, for console or terminal Product-Terminal The new Windows Terminal. labels Mar 5, 2025
@carlos-zamora
Copy link
Member Author

Demo

image

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;

{
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;

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Area-Settings Issues related to settings and customizability, for console or terminal Issue-Feature Complex enough to require an in depth planning process and actual budgeted, scheduled work. Product-Terminal The new Windows Terminal.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Allow newTabMenu's matchProfile to work with regex
2 participants