Unify sheet name validators and apply to properties

Properties panel and properties dialog now share a validator with shared
strings

Fixes https://gitlab.com/kicad/code/kicad/issues/21689

common/validators.cpp

@@ -289,13 +289,14 @@ bool FIELD_VALIDATOR::Validate( wxWindow* aParent )
 }
 
 
-bool FIELD_VALIDATOR::DoValidate( const wxString& aValue, wxWindow* aParent )
+wxString GetFieldValidationErrorMessage( FIELD_T aFieldId, const wxString& aValue )
 {
-    wxString msg;
+    FIELD_VALIDATOR validator( aFieldId );
+    wxString        msg;
 
-    if( HasFlag( wxFILTER_EMPTY ) && aValue.empty() )
+    if( validator.HasFlag( wxFILTER_EMPTY ) && aValue.empty() )
     {
-        switch( m_fieldId )
+        switch( aFieldId )
         {
         case FIELD_T::SHEET_NAME:     msg = _( "A sheet must have a name." );               break;
         case FIELD_T::SHEET_FILENAME: msg = _( "A sheet must have a file specified." );     break;
@@ -303,11 +304,11 @@ bool FIELD_VALIDATOR::DoValidate( const wxString& aValue, wxWindow* aParent )
         }
     }
 
-    if( HasFlag( wxFILTER_EXCLUDE_CHAR_LIST ) && ContainsExcludedCharacters( aValue ) )
+    if( msg.empty() && validator.HasFlag( wxFILTER_EXCLUDE_CHAR_LIST ) )
     {
         wxArrayString badCharsFound;
 
-        for( const wxUniCharRef& excludeChar : GetCharExcludes() )
+        for( const wxUniCharRef& excludeChar : validator.GetCharExcludes() )
         {
             if( aValue.Find( excludeChar ) != wxNOT_FOUND )
             {
@@ -324,68 +325,83 @@ bool FIELD_VALIDATOR::DoValidate( const wxString& aValue, wxWindow* aParent )
             }
         }
 
-        wxString badChars;
+        if( !badCharsFound.IsEmpty() )
-
-        for( size_t i = 0; i < badCharsFound.GetCount(); i++ )
         {
-            if( !badChars.IsEmpty() )
+            wxString badChars;
+
+            for( size_t i = 0; i < badCharsFound.GetCount(); i++ )
             {
-                if( badCharsFound.GetCount() == 2 )
+                if( !badChars.IsEmpty() )
                 {
-                    badChars += _( " or " );
+                    if( badCharsFound.GetCount() == 2 )
-                }
+                    {
-                else
+                        badChars += _( " or " );
-                {
+                    }
-                    if( i < badCharsFound.GetCount() - 2 )
-                        badChars += _( ", or " );
                     else
-                        badChars += wxT( ", " );
+                    {
+                        if( i < badCharsFound.GetCount() - 2 )
+                            badChars += _( ", or " );
+                        else
+                            badChars += wxT( ", " );
+                    }
                 }
+
+                badChars += badCharsFound.Item( i );
             }
 
-            badChars += badCharsFound.Item( i );
+            switch( aFieldId )
+            {
+            case FIELD_T::REFERENCE:
+                msg.Printf( _( "The reference designator cannot contain %s character(s)." ), badChars );
+                break;
+
+            case FIELD_T::VALUE:
+                msg.Printf( _( "The value field cannot contain %s character(s)." ), badChars );
+                break;
+
+            case FIELD_T::FOOTPRINT:
+                msg.Printf( _( "The footprint field cannot contain %s character(s)." ), badChars );
+                break;
+
+            case FIELD_T::DATASHEET:
+                msg.Printf( _( "The datasheet field cannot contain %s character(s)." ), badChars );
+                break;
+
+            case FIELD_T::SHEET_NAME:
+                msg.Printf( _( "The sheet name cannot contain %s character(s)." ), badChars );
+                break;
+
+            case FIELD_T::SHEET_FILENAME:
+                msg.Printf( _( "The sheet filename cannot contain %s character(s)." ), badChars );
+                break;
+
+            default:
+                msg.Printf( _( "The field cannot contain %s character(s)." ), badChars );
+                break;
+            };
         }
+    }
 
-        switch( m_fieldId )
+    if( msg.empty() )
+    {
+        if( aFieldId == FIELD_T::REFERENCE && aValue.Contains( wxT( "${" ) ) )
         {
-        case FIELD_T::REFERENCE:
+            msg.Printf( _( "The reference designator cannot contain text variable references" ) );
-            msg.Printf( _( "The reference designator cannot contain %s character(s)." ), badChars );
+        }
-            break;
+        else if( aFieldId == FIELD_T::REFERENCE && UTIL::GetRefDesPrefix( aValue ).IsEmpty() )
-
+        {
-        case FIELD_T::VALUE:
+            msg.Printf( _( "References must start with a letter." ) );
-            msg.Printf( _( "The value field cannot contain %s character(s)." ), badChars );
+        }
-            break;
-
-        case FIELD_T::FOOTPRINT:
-            msg.Printf( _( "The footprint field cannot contain %s character(s)." ), badChars );
-            break;
-
-        case FIELD_T::DATASHEET:
-            msg.Printf( _( "The datasheet field cannot contain %s character(s)." ), badChars );
-            break;
-
-        case FIELD_T::SHEET_NAME:
-            msg.Printf( _( "The sheet name cannot contain %s character(s)." ), badChars );
-            break;
-
-        case FIELD_T::SHEET_FILENAME:
-            msg.Printf( _( "The sheet filename cannot contain %s character(s)." ), badChars );
-            break;
-
-        default:
-            msg.Printf( _( "The field cannot contain %s character(s)." ), badChars );
-            break;
-        };
-    }
-    else if( m_fieldId == FIELD_T::REFERENCE && aValue.Contains( wxT( "${" ) ) )
-    {
-        msg.Printf( _( "The reference designator cannot contain text variable references" ) );
-    }
-    else if( m_fieldId == FIELD_T::REFERENCE && UTIL::GetRefDesPrefix( aValue ).IsEmpty() )
-    {
-        msg.Printf( _( "References must start with a letter." ) );
     }
 
+    return msg;
+}
+
+
+bool FIELD_VALIDATOR::DoValidate( const wxString& aValue, wxWindow* aParent )
+{
+    wxString msg = GetFieldValidationErrorMessage( m_fieldId, aValue );
+
     if( !msg.empty() )
     {
         if( m_validatorWindow )

eeschema/sch_sheet.cpp

@@ -46,6 +46,8 @@
 #include <settings/color_settings.h>
 #include <settings/settings_manager.h>
 #include <trace_helpers.h>
+#include <validators.h>
+#include <properties/property_validators.h>
 #include <pgm_base.h>
 #include <wx/log.h>
 
@@ -1657,7 +1659,21 @@ static struct SCH_SHEET_DESC
         propMgr.InheritsAfter( TYPE_HASH( SCH_SHEET ), TYPE_HASH( SCH_ITEM ) );
 
         propMgr.AddProperty( new PROPERTY<SCH_SHEET, wxString>( _HKI( "Sheet Name" ),
-                             &SCH_SHEET::SetName, &SCH_SHEET::GetName ) );
+                             &SCH_SHEET::SetName, &SCH_SHEET::GetName ) )
+                .SetValidator( []( const wxAny&& aValue, EDA_ITEM* ) -> VALIDATOR_RESULT
+                                {
+                                    wxString value;
+
+                                    if( !aValue.GetAs( &value ) )
+                                        return {};
+
+                                    wxString msg = GetFieldValidationErrorMessage( FIELD_T::SHEET_NAME, value );
+
+                                    if( msg.empty() )
+                                        return {};
+
+                                    return std::make_unique<VALIDATION_ERROR_MSG>( msg );
+                                } );
 
         propMgr.AddProperty( new PROPERTY<SCH_SHEET, int>( _HKI( "Border Width" ),
                              &SCH_SHEET::SetBorderWidth, &SCH_SHEET::GetBorderWidth,

include/validators.h

@@ -162,5 +162,11 @@ private:
     FIELD_T m_fieldId;
 };
 
+/**
+ * Return the error message if @a aValue is invalid for @a aFieldId.
+ * Returns an empty string when the value is valid.
+ */
+wxString GetFieldValidationErrorMessage( FIELD_T aFieldId, const wxString& aValue );
+
 
 #endif  // #ifndef VALIDATORS_H