cache regex; rework validation system

Author: carlos-zamora

Diff for: src/cascadia/TerminalSettingsModel/CascadiaSettings.cpp

@@ -4,6 +4,7 @@
 #include "pch.h"
 #include "CascadiaSettings.h"
 #include "CascadiaSettings.g.cpp"
+#include "MatchProfilesEntry.h"
 
 #include "DefaultTerminal.h"
 #include "FileUtils.h"
@@ -584,47 +585,35 @@ void CascadiaSettings::_validateProfileEnvironmentVariables()
     }
 }
 
-static void _validateRegex(const winrt::hstring& regex, IVector<Model::SettingsLoadWarnings>& warnings)
-{
-    try
-    {
-        std::wregex{ regex.cbegin(), regex.cend() };
-    }
-    catch (std::regex_error)
-    {
-        warnings.Append(Model::SettingsLoadWarnings::InvalidRegex);
-    }
-}
-
-static void _validateNTMEntries(const IVector<Model::NewTabMenuEntry>& entries, IVector<Model::SettingsLoadWarnings>& warnings)
+// Returns true if all regexes in the new tab menu are valid, false otherwise
+static bool _validateNTMEntries(const IVector<Model::NewTabMenuEntry>& entries)
 {
     for (const auto& ntmEntry : entries)
     {
         if (const auto& folderEntry = ntmEntry.try_as<Model::FolderEntry>())
         {
-            _validateNTMEntries(folderEntry.RawEntries(), warnings);
+            if (!_validateNTMEntries(folderEntry.RawEntries()))
+            {
+                return false;
+            }
         }
         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())
+            if (!winrt::get_self<Model::implementation::MatchProfilesEntry>(matchProfilesEntry)->ValidateRegexes())
             {
-                _validateRegex(commandlineRegex, warnings);
-            }
-            if (const auto sourceRegex = matchProfilesEntry.Source(); !sourceRegex.empty())
-            {
-                _validateRegex(sourceRegex, warnings);
+                return false;
             }
         }
     }
+    return true;
 }
 
 void CascadiaSettings::_validateRegexes()
 {
-    _validateNTMEntries(_globals->NewTabMenu(), _warnings);
+    if (!_validateNTMEntries(_globals->NewTabMenu()))
+    {
+        _warnings.Append(SettingsLoadWarnings::InvalidRegex);
+    }
 }
 
 // Method Description:

Diff for: src/cascadia/TerminalSettingsModel/MatchProfilesEntry.cpp

@@ -36,61 +36,46 @@ namespace winrt::Microsoft::Terminal::Settings::Model::implementation
         auto entry = winrt::make_self<MatchProfilesEntry>();
 
         JsonUtils::GetValueForKey(json, NameKey, entry->_Name);
+        entry->_validateName();
+
         JsonUtils::GetValueForKey(json, CommandlineKey, entry->_Commandline);
+        entry->_validateCommandline();
+
         JsonUtils::GetValueForKey(json, SourceKey, entry->_Source);
+        entry->_validateSource();
 
         return entry;
     }
 
+    // Returns true if all regexes are valid, false otherwise
+    bool MatchProfilesEntry::ValidateRegexes() const
+    {
+        return !(_invalidName || _invalidCommandline || _invalidSource);
+    }
+
     bool MatchProfilesEntry::MatchesProfile(const Model::Profile& profile)
     {
-        // We use an optional here instead of a simple bool directly, since there is no
-        // sensible default value for the desired semantics: the first property we want
-        // to match on should always be applied (so one would set "true" as a default),
-        // but if none of the properties are set, the default return value should be false
-        // since this entry type is expected to behave like a positive match/whitelist.
-        //
-        // The easiest way to deal with this neatly is to use an optional, then for any
-        // property to match we consider a null value to be "true", and for the return
-        // 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) {
+        auto isMatch = [](const std::wregex& 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);
+            return std::regex_match(text.cbegin(), text.cend(), regex);
         };
 
-        if (!_Name.empty())
+        if (!_Name.empty() && isMatch(_NameRegex, profile.Name()))
         {
-            isMatching = { isMatching.value_or(true) && isMatch(_Name, profile.Name()) };
+            return true;
         }
-
-        if (!_Source.empty())
+        else if (!_Source.empty() && isMatch(_SourceRegex, profile.Source()))
         {
-            isMatching = { isMatching.value_or(true) && isMatch(_Source, profile.Source()) };
+            return true;
         }
-
-        if (!_Commandline.empty())
+        else if (!_Commandline.empty() && isMatch(_CommandlineRegex, profile.Commandline()))
         {
-            isMatching = { isMatching.value_or(true) && isMatch(_Commandline, profile.Commandline()) };
+            return true;
         }
-
-        return isMatching.value_or(false);
+        return false;
     }
 
     Model::NewTabMenuEntry MatchProfilesEntry::Copy() const

Diff for: src/cascadia/TerminalSettingsModel/MatchProfilesEntry.h

@@ -18,6 +18,40 @@ Author(s):
 #include "ProfileCollectionEntry.h"
 #include "MatchProfilesEntry.g.h"
 
+// This macro defines the getter and setter for a regex property.
+// The setter tries to instantiate the regex immediately and caches
+// it if successful. If it fails, it sets a boolean flag to track that
+// it failed.
+#define MATCH_PROFILE_REGEX_PROPERTY(name)                         \
+public:                                                            \
+    hstring name() const noexcept                                  \
+    {                                                              \
+        return _##name;                                            \
+    }                                                              \
+    void name(const hstring& value) noexcept                       \
+    {                                                              \
+        _##name = value;                                           \
+        _validate##name();                                         \
+    }                                                              \
+                                                                   \
+private:                                                           \
+    void _validate##name() noexcept                                \
+    {                                                              \
+        _invalid##name = false;                                    \
+        try                                                        \
+        {                                                          \
+            _##name##Regex = { _##name.cbegin(), _##name.cend() }; \
+        }                                                          \
+        catch (std::regex_error)                                   \
+        {                                                          \
+            _invalid##name = true;                                 \
+        }                                                          \
+    }                                                              \
+                                                                   \
+    hstring _##name;                                               \
+    std::wregex _##name##Regex;                                    \
+    bool _invalid##name{ false };
+
 namespace winrt::Microsoft::Terminal::Settings::Model::implementation
 {
     struct MatchProfilesEntry : MatchProfilesEntryT<MatchProfilesEntry, ProfileCollectionEntry>
@@ -30,11 +64,12 @@ namespace winrt::Microsoft::Terminal::Settings::Model::implementation
         Json::Value ToJson() const override;
         static com_ptr<NewTabMenuEntry> FromJson(const Json::Value& json);
 
+        bool ValidateRegexes() const;
         bool MatchesProfile(const Model::Profile& profile);
 
-        WINRT_PROPERTY(winrt::hstring, Name);
-        WINRT_PROPERTY(winrt::hstring, Commandline);
-        WINRT_PROPERTY(winrt::hstring, Source);
+        MATCH_PROFILE_REGEX_PROPERTY(Name);
+        MATCH_PROFILE_REGEX_PROPERTY(Commandline);
+        MATCH_PROFILE_REGEX_PROPERTY(Source);
     };
 }