From f7bef5e09b396fb4bcb9efd5a39c809baea0d239 Mon Sep 17 00:00:00 2001
From: Jeff Young <jeff@rokeby.ie>
Date: Thu, 16 May 2024 15:50:57 +0100
Subject: [PATCH] Generalize EnhanceAttr() function.

Fixes https://gitlab.com/kicad/code/kicad/-/issues/17735
---
 common/widgets/wx_grid.cpp                    | 30 +++++++++++++
 eeschema/dialogs/dialog_symbol_properties.cpp | 39 +++--------------
 eeschema/fields_grid_table.cpp                | 42 +++++++++----------
 eeschema/fields_grid_table.h                  |  4 +-
 include/widgets/wx_grid.h                     |  8 ++++
 .../dialog_footprint_properties_fp_editor.cpp |  4 +-
 .../dialog_footprint_properties_fp_editor.h   |  4 +-
 pcbnew/pcb_fields_grid_table.cpp              | 27 ++++++------
 pcbnew/pcb_fields_grid_table.h                |  4 +-
 9 files changed, 86 insertions(+), 76 deletions(-)

diff --git a/common/widgets/wx_grid.cpp b/common/widgets/wx_grid.cpp
index 0c475d839b..d2d43309f0 100644
--- a/common/widgets/wx_grid.cpp
+++ b/common/widgets/wx_grid.cpp
@@ -38,6 +38,36 @@
 #include <pgm_base.h>
 #include <settings/common_settings.h>
 
+
+wxGridCellAttr* WX_GRID_TABLE_BASE::enhanceAttr( wxGridCellAttr* aInputAttr, int aRow, int aCol,
+                                                 wxGridCellAttr::wxAttrKind aKind  )
+{
+    wxGridCellAttr* attr = aInputAttr;
+
+    if( wxGridCellAttrProvider* provider = GetAttrProvider() )
+    {
+        wxGridCellAttr* providerAttr = provider->GetAttr( aRow, aCol, aKind );
+
+        if( providerAttr )
+        {
+            attr = new wxGridCellAttr;
+            attr->SetKind( wxGridCellAttr::Merged );
+
+            if( aInputAttr )
+            {
+                attr->MergeWith( aInputAttr );
+                aInputAttr->DecRef();
+            }
+
+            attr->MergeWith( providerAttr );
+            providerAttr->DecRef();
+        }
+    }
+
+    return attr;
+}
+
+
 #define MIN_GRIDCELL_MARGIN FromDIP( 3 )
 
 
diff --git a/eeschema/dialogs/dialog_symbol_properties.cpp b/eeschema/dialogs/dialog_symbol_properties.cpp
index 29ed159621..c68c714b52 100644
--- a/eeschema/dialogs/dialog_symbol_properties.cpp
+++ b/eeschema/dialogs/dialog_symbol_properties.cpp
@@ -65,7 +65,7 @@ enum PIN_TABLE_COL_ORDER
 };
 
 
-class SCH_PIN_TABLE_DATA_MODEL : public wxGridTableBase, public std::vector<SCH_PIN>
+class SCH_PIN_TABLE_DATA_MODEL : public WX_GRID_TABLE_BASE, public std::vector<SCH_PIN>
 {
 public:
     SCH_PIN_TABLE_DATA_MODEL() :
@@ -197,53 +197,24 @@ public:
 
     wxGridCellAttr* GetAttr( int aRow, int aCol, wxGridCellAttr::wxAttrKind aKind ) override
     {
-        // This is needed to support alternating row colors
-        auto enhanceAttr = [this, &aRow, &aCol,
-                            &aKind]( wxGridCellAttr* aInputAttr ) -> wxGridCellAttr*
-        {
-            if( aInputAttr == nullptr )
-                return nullptr;
-
-            wxGridCellAttr* attr = aInputAttr;
-
-            if( wxGridCellAttrProvider* provider = GetAttrProvider() )
-            {
-                wxGridCellAttr* providerAttr = provider->GetAttr( aRow, aCol, aKind );
-
-                if( providerAttr )
-                {
-                    attr = new wxGridCellAttr;
-                    attr->SetKind( wxGridCellAttr::Merged );
-
-                    attr->MergeWith( aInputAttr );
-                    aInputAttr->DecRef();
-
-                    attr->MergeWith( providerAttr );
-                    providerAttr->DecRef();
-                }
-            }
-
-            return attr;
-        };
-
         switch( aCol )
         {
         case COL_NUMBER:
         case COL_BASE_NAME:
             m_readOnlyAttr->IncRef();
-            return enhanceAttr( m_readOnlyAttr );
+            return enhanceAttr( m_readOnlyAttr, aRow, aCol, aKind );
 
         case COL_ALT_NAME:
             m_nameAttrs[ aRow ]->IncRef();
-            return enhanceAttr( m_nameAttrs[ aRow ] );
+            return enhanceAttr( m_nameAttrs[ aRow ], aRow, aCol, aKind );
 
         case COL_TYPE:
             m_typeAttr->IncRef();
-            return enhanceAttr( m_typeAttr );
+            return enhanceAttr( m_typeAttr, aRow, aCol, aKind );
 
         case COL_SHAPE:
             m_shapeAttr->IncRef();
-            return enhanceAttr( m_shapeAttr );
+            return enhanceAttr( m_shapeAttr, aRow, aCol, aKind );
 
         default:
             wxFAIL;
diff --git a/eeschema/fields_grid_table.cpp b/eeschema/fields_grid_table.cpp
index 0f16e3faea..17583d1fda 100644
--- a/eeschema/fields_grid_table.cpp
+++ b/eeschema/fields_grid_table.cpp
@@ -422,7 +422,7 @@ bool FIELDS_GRID_TABLE::CanSetValueAs( int aRow, int aCol, const wxString& aType
 }
 
 
-wxGridCellAttr* FIELDS_GRID_TABLE::GetAttr( int aRow, int aCol, wxGridCellAttr::wxAttrKind  )
+wxGridCellAttr* FIELDS_GRID_TABLE::GetAttr( int aRow, int aCol, wxGridCellAttr::wxAttrKind aKind  )
 {
     wxGridCellAttr* tmp;
 
@@ -433,24 +433,24 @@ wxGridCellAttr* FIELDS_GRID_TABLE::GetAttr( int aRow, int aCol, wxGridCellAttr::
         {
             tmp = m_fieldNameAttr->Clone();
             tmp->SetReadOnly( true );
-            return tmp;
+            return enhanceAttr( tmp, aRow, aCol, aKind );
         }
         else
         {
             m_fieldNameAttr->IncRef();
-            return m_fieldNameAttr;
+            return enhanceAttr( m_fieldNameAttr, aRow, aCol, aKind );
         }
 
     case FDC_VALUE:
         if( m_parentType == SCH_SYMBOL_T && aRow == REFERENCE_FIELD )
         {
             m_referenceAttr->IncRef();
-            return m_referenceAttr;
+            return enhanceAttr( m_referenceAttr, aRow, aCol, aKind );
         }
         else if( m_parentType == SCH_SYMBOL_T && aRow == VALUE_FIELD )
         {
             m_valueAttr->IncRef();
-            return m_valueAttr;
+            return enhanceAttr( m_valueAttr, aRow, aCol, aKind );
         }
         else if( m_parentType == SCH_SYMBOL_T && aRow == FOOTPRINT_FIELD )
         {
@@ -460,34 +460,34 @@ wxGridCellAttr* FIELDS_GRID_TABLE::GetAttr( int aRow, int aCol, wxGridCellAttr::
             if( m_part && m_part->IsPower() )
             {
                 m_readOnlyAttr->IncRef();
-                return m_readOnlyAttr;
+                return enhanceAttr( m_readOnlyAttr, aRow, aCol, aKind );
             }
             else
             {
                 m_footprintAttr->IncRef();
-                return m_footprintAttr;
+                return enhanceAttr( m_footprintAttr, aRow, aCol, aKind );
             }
         }
         else if( m_parentType == SCH_SYMBOL_T && aRow == DATASHEET_FIELD )
         {
             m_urlAttr->IncRef();
-            return m_urlAttr;
+            return enhanceAttr( m_urlAttr, aRow, aCol, aKind );
         }
         else if( m_parentType == SCH_SHEET_T && aRow == SHEETNAME )
         {
             m_referenceAttr->IncRef();
-            return m_referenceAttr;
+            return enhanceAttr( m_referenceAttr, aRow, aCol, aKind );
         }
         else if( m_parentType == SCH_SHEET_T && aRow == SHEETFILENAME )
         {
             m_filepathAttr->IncRef();
-            return m_filepathAttr;
+            return enhanceAttr( m_filepathAttr, aRow, aCol, aKind );
         }
         else if( ( m_parentType == SCH_LABEL_LOCATE_ANY_T )
                 && this->at( (size_t) aRow ).GetCanonicalName() == wxT( "Netclass" ) )
         {
             m_netclassAttr->IncRef();
-            return m_netclassAttr;
+            return enhanceAttr( m_netclassAttr, aRow, aCol, aKind );
         }
         else
         {
@@ -501,31 +501,31 @@ wxGridCellAttr* FIELDS_GRID_TABLE::GetAttr( int aRow, int aCol, wxGridCellAttr::
             if( templateFn && templateFn->m_URL )
             {
                 m_urlAttr->IncRef();
-                return m_urlAttr;
+                return enhanceAttr( m_urlAttr, aRow, aCol, aKind );
             }
             else
             {
                 m_nonUrlAttr->IncRef();
-                return m_nonUrlAttr;
+                return enhanceAttr( m_nonUrlAttr, aRow, aCol, aKind );
             }
         }
 
     case FDC_TEXT_SIZE:
     case FDC_POSX:
     case FDC_POSY:
-        return nullptr;
+        return enhanceAttr( nullptr, aRow, aCol, aKind );
 
     case FDC_H_ALIGN:
         m_hAlignAttr->IncRef();
-        return m_hAlignAttr;
+        return enhanceAttr( m_hAlignAttr, aRow, aCol, aKind );
 
     case FDC_V_ALIGN:
         m_vAlignAttr->IncRef();
-        return m_vAlignAttr;
+        return enhanceAttr( m_vAlignAttr, aRow, aCol, aKind );
 
     case FDC_ORIENTATION:
         m_orientationAttr->IncRef();
-        return m_orientationAttr;
+        return enhanceAttr( m_orientationAttr, aRow, aCol, aKind );
 
     case FDC_SHOWN:
     case FDC_SHOW_NAME:
@@ -533,19 +533,19 @@ wxGridCellAttr* FIELDS_GRID_TABLE::GetAttr( int aRow, int aCol, wxGridCellAttr::
     case FDC_BOLD:
     case FDC_ALLOW_AUTOPLACE:
         m_boolAttr->IncRef();
-        return m_boolAttr;
+        return enhanceAttr( m_boolAttr, aRow, aCol, aKind );
 
     case FDC_FONT:
         m_fontAttr->IncRef();
-        return m_fontAttr;
+        return enhanceAttr( m_fontAttr, aRow, aCol, aKind );
 
     case FDC_COLOR:
         m_colorAttr->IncRef();
-        return m_colorAttr;
+        return enhanceAttr( m_colorAttr, aRow, aCol, aKind );
 
     default:
         wxFAIL;
-        return nullptr;
+        return enhanceAttr( nullptr, aRow, aCol, aKind );
     }
 }
 
diff --git a/eeschema/fields_grid_table.h b/eeschema/fields_grid_table.h
index a294862ed1..4a0a00661c 100644
--- a/eeschema/fields_grid_table.h
+++ b/eeschema/fields_grid_table.h
@@ -72,7 +72,7 @@ enum FIELDS_DATA_COL_ORDER
 };
 
 
-class FIELDS_GRID_TABLE : public wxGridTableBase, public std::vector<SCH_FIELD>
+class FIELDS_GRID_TABLE : public WX_GRID_TABLE_BASE, public std::vector<SCH_FIELD>
 {
 public:
     FIELDS_GRID_TABLE( DIALOG_SHIM* aDialog, SCH_BASE_FRAME* aFrame, WX_GRID* aGrid,
@@ -97,7 +97,7 @@ public:
 
     bool CanGetValueAs( int aRow, int aCol, const wxString& aTypeName ) override;
     bool CanSetValueAs( int aRow, int aCol, const wxString& aTypeName ) override;
-    wxGridCellAttr* GetAttr( int row, int col, wxGridCellAttr::wxAttrKind kind ) override;
+    wxGridCellAttr* GetAttr( int aRow, int aCol, wxGridCellAttr::wxAttrKind aKind ) override;
 
     wxString GetValue( int aRow, int aCol ) override;
     bool GetValueAsBool( int aRow, int aCol ) override;
diff --git a/include/widgets/wx_grid.h b/include/widgets/wx_grid.h
index b79d448019..d225d565b0 100644
--- a/include/widgets/wx_grid.h
+++ b/include/widgets/wx_grid.h
@@ -37,6 +37,14 @@
 class wxTextEntryBase;
 
 
+class WX_GRID_TABLE_BASE : public wxGridTableBase
+{
+protected:
+    wxGridCellAttr* enhanceAttr( wxGridCellAttr* aInputAttr, int aRow, int aCol,
+                                 wxGridCellAttr::wxAttrKind aKind  );
+};
+
+
 class WX_GRID : public wxGrid
 {
 public:
diff --git a/pcbnew/dialogs/dialog_footprint_properties_fp_editor.cpp b/pcbnew/dialogs/dialog_footprint_properties_fp_editor.cpp
index 88d841ca36..e16480534e 100644
--- a/pcbnew/dialogs/dialog_footprint_properties_fp_editor.cpp
+++ b/pcbnew/dialogs/dialog_footprint_properties_fp_editor.cpp
@@ -85,10 +85,10 @@ bool PRIVATE_LAYERS_GRID_TABLE::CanSetValueAs( int aRow, int aCol, const wxStrin
 
 
 wxGridCellAttr* PRIVATE_LAYERS_GRID_TABLE::GetAttr( int aRow, int aCol,
-                                                    wxGridCellAttr::wxAttrKind  )
+                                                    wxGridCellAttr::wxAttrKind aKind  )
 {
     m_layerColAttr->IncRef();
-    return m_layerColAttr;
+    return enhanceAttr( m_layerColAttr, aRow, aCol, aKind );
 }
 
 
diff --git a/pcbnew/dialogs/dialog_footprint_properties_fp_editor.h b/pcbnew/dialogs/dialog_footprint_properties_fp_editor.h
index eff5029c69..7bf6e00f52 100644
--- a/pcbnew/dialogs/dialog_footprint_properties_fp_editor.h
+++ b/pcbnew/dialogs/dialog_footprint_properties_fp_editor.h
@@ -36,7 +36,7 @@ class FOOTPRINT_EDIT_FRAME;
 class PANEL_FP_PROPERTIES_3D_MODEL;
 
 
-class PRIVATE_LAYERS_GRID_TABLE : public wxGridTableBase, public std::vector<PCB_LAYER_ID>
+class PRIVATE_LAYERS_GRID_TABLE : public WX_GRID_TABLE_BASE, public std::vector<PCB_LAYER_ID>
 {
 public:
     PRIVATE_LAYERS_GRID_TABLE( PCB_BASE_FRAME* aFrame );
@@ -47,7 +47,7 @@ public:
 
     bool CanGetValueAs( int aRow, int aCol, const wxString& aTypeName ) override;
     bool CanSetValueAs( int aRow, int aCol, const wxString& aTypeName ) override;
-    wxGridCellAttr* GetAttr( int row, int col, wxGridCellAttr::wxAttrKind kind ) override;
+    wxGridCellAttr* GetAttr( int aRow, int aCol, wxGridCellAttr::wxAttrKind aKind ) override;
 
     wxString GetValue( int aRow, int aCol ) override;
     long GetValueAsLong( int aRow, int aCol ) override;
diff --git a/pcbnew/pcb_fields_grid_table.cpp b/pcbnew/pcb_fields_grid_table.cpp
index 5576018588..e3a8d1615d 100644
--- a/pcbnew/pcb_fields_grid_table.cpp
+++ b/pcbnew/pcb_fields_grid_table.cpp
@@ -190,7 +190,8 @@ bool PCB_FIELDS_GRID_TABLE::CanSetValueAs( int aRow, int aCol, const wxString& a
 }
 
 
-wxGridCellAttr* PCB_FIELDS_GRID_TABLE::GetAttr( int aRow, int aCol, wxGridCellAttr::wxAttrKind  )
+wxGridCellAttr* PCB_FIELDS_GRID_TABLE::GetAttr( int aRow, int aCol,
+                                                wxGridCellAttr::wxAttrKind aKind  )
 {
     switch( aCol )
     {
@@ -198,41 +199,41 @@ wxGridCellAttr* PCB_FIELDS_GRID_TABLE::GetAttr( int aRow, int aCol, wxGridCellAt
         if( aRow < MANDATORY_FIELDS )
         {
             m_readOnlyAttr->IncRef();
-            return m_readOnlyAttr;
+            return enhanceAttr( m_readOnlyAttr, aRow, aCol, aKind );
         }
 
-        return nullptr;
+        return enhanceAttr( nullptr, aRow, aCol, aKind );
 
     case PFC_VALUE:
         if( aRow == REFERENCE_FIELD )
         {
             m_referenceAttr->IncRef();
-            return m_referenceAttr;
+            return enhanceAttr( m_referenceAttr, aRow, aCol, aKind );
         }
         else if( aRow == VALUE_FIELD )
         {
             m_valueAttr->IncRef();
-            return m_valueAttr;
+            return enhanceAttr( m_valueAttr, aRow, aCol, aKind );
         }
         else if( aRow == FOOTPRINT_FIELD )
         {
             m_footprintAttr->IncRef();
-            return m_footprintAttr;
+            return enhanceAttr( m_footprintAttr, aRow, aCol, aKind );
         }
         else if( aRow == DATASHEET_FIELD )
         {
             m_urlAttr->IncRef();
-            return m_urlAttr;
+            return enhanceAttr( m_urlAttr, aRow, aCol, aKind );
         }
 
-        return nullptr;
+        return enhanceAttr( nullptr, aRow, aCol, aKind );
 
     case PFC_WIDTH:
     case PFC_HEIGHT:
     case PFC_THICKNESS:
     case PFC_XOFFSET:
     case PFC_YOFFSET:
-        return nullptr;
+        return enhanceAttr( nullptr, aRow, aCol, aKind );
 
     case PFC_SHOWN:
     case PFC_ITALIC:
@@ -240,19 +241,19 @@ wxGridCellAttr* PCB_FIELDS_GRID_TABLE::GetAttr( int aRow, int aCol, wxGridCellAt
     case PFC_KNOCKOUT:
     case PFC_MIRRORED:
         m_boolColAttr->IncRef();
-        return m_boolColAttr;
+        return enhanceAttr( m_boolColAttr, aRow, aCol, aKind );
 
     case PFC_LAYER:
         m_layerColAttr->IncRef();
-        return m_layerColAttr;
+        return enhanceAttr( m_layerColAttr, aRow, aCol, aKind );
 
     case PFC_ORIENTATION:
         m_orientationColAttr->IncRef();
-        return m_orientationColAttr;
+        return enhanceAttr( m_orientationColAttr, aRow, aCol, aKind );
 
     default:
         wxFAIL;
-        return nullptr;
+        return enhanceAttr( nullptr, aRow, aCol, aKind );
     }
 }
 
diff --git a/pcbnew/pcb_fields_grid_table.h b/pcbnew/pcb_fields_grid_table.h
index 74505e4daf..e100bcd923 100644
--- a/pcbnew/pcb_fields_grid_table.h
+++ b/pcbnew/pcb_fields_grid_table.h
@@ -54,7 +54,7 @@ enum PCB_FIELDS_COL_ORDER
 };
 
 
-class PCB_FIELDS_GRID_TABLE : public wxGridTableBase, public std::vector<PCB_FIELD>
+class PCB_FIELDS_GRID_TABLE : public WX_GRID_TABLE_BASE, public std::vector<PCB_FIELD>
 {
 public:
     PCB_FIELDS_GRID_TABLE( PCB_BASE_FRAME* aFrame, DIALOG_SHIM* aDialog );
@@ -72,7 +72,7 @@ public:
 
     bool CanGetValueAs( int aRow, int aCol, const wxString& aTypeName ) override;
     bool CanSetValueAs( int aRow, int aCol, const wxString& aTypeName ) override;
-    wxGridCellAttr* GetAttr( int row, int col, wxGridCellAttr::wxAttrKind kind ) override;
+    wxGridCellAttr* GetAttr( int aRow, int aCol, wxGridCellAttr::wxAttrKind aKind ) override;
 
     wxString GetValue( int aRow, int aCol ) override;
     bool GetValueAsBool( int aRow, int aCol ) override;