Replace New Tab Menu Match Profiles functionality with regex support #18654
Conversation
microsoft-github-policy-service
bot
added
Issue-Feature
Demo
|
This comment was marked as off-topic.
This comment was marked as off-topic.
Sorry, something went wrong.
|
Don't spam this repo with LLM responses. We have access to those ourselves as well. Thanks! |
| UErrorCode status = U_ZERO_ERROR; | ||
| uregex_setText(regex.get(), reinterpret_cast<const UChar*>(text.data()), static_cast<int32_t>(text.size()), &status); | ||
| const auto match = uregex_matches(regex.get(), 0, &status); | ||
| return status == U_ZERO_ERROR && match; |
There was a problem hiding this comment.
To make it easier on ourselves, we could write our own unique_uregex struct. This would allow us to define helper functions on it like matches(std::wstring_view) that does all of this under the hood for you.
Not sure if it's worth doing as part of this PR, but if you're feeling eager, I think it may be worth it.
There was a problem hiding this comment.
FWIW, if we do that, I think it's worth keeping the current approach of not throwing when the regex is invalid (and instead return a bool or std::optional, etc.). I feel like throwing would be overkill and just splatter try/catch everywhere we handle user input...
| return; \ | ||
| } \ | ||
| UErrorCode status = U_ZERO_ERROR; \ | ||
| _##name##Regex = til::ICU::CreateRegex(_##name, 0, &status); \ |
There was a problem hiding this comment.
i wish "validate" didn't also mean "populate".
There was a problem hiding this comment.
what i mean to say is, "i wish this was clearer about what it did"
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.VisualStudiostill works as expectedPR Checklist
Closes #18553