Properties: refactoring

common/widgets/properties_panel.cpp

@@ -225,43 +225,11 @@ void PROPERTIES_PANEL::rebuildProperties( const SELECTION& aSelection )
             continue;
 
         // Either determine the common value for a property or "<...>" to indicate multiple values
-        bool available = true;
+        bool available;
-        bool writeable = true;
+        bool writeable;
-        bool different = false;
         wxVariant commonVal;
 
-        for( EDA_ITEM* item : aSelection )
+        available = extractValueAndWritability( aSelection, property, commonVal, writeable );
-        {
-            if( !propMgr.IsAvailableFor( TYPE_HASH( *item ), property, item ) )
-            {
-                available = false;
-                break; // there is an item that does not have this property, so do not display it
-            }
-
-            // If read-only for any of the selection, read-only for the whole selection.
-            if( !property->Writeable( item ) )
-                writeable = false;
-
-            wxVariant value;
-
-            if( getItemValue( item, property, value ) )
-            {
-                // Null value indicates different property values between items
-                if( !different && !commonVal.IsNull() && value != commonVal )
-                {
-                    different = true;
-                    commonVal.MakeNull();
-                }
-                else if( !different )
-                {
-                    commonVal = value;
-                }
-            }
-            else
-            {
-                available = false;
-            }
-        }
 
         if( available )
         {
@@ -334,6 +302,54 @@ bool PROPERTIES_PANEL::getItemValue( EDA_ITEM* aItem, PROPERTY_BASE* aProperty,
 }
 
 
+bool PROPERTIES_PANEL::extractValueAndWritability( const SELECTION& aSelection,
+                                                   PROPERTY_BASE* aProperty,
+                                                   wxVariant& aValue, bool& aWritable )
+{
+    PROPERTY_MANAGER& propMgr = PROPERTY_MANAGER::Instance();
+    propMgr.SetUnits( m_frame->GetUserUnits() );
+    propMgr.SetTransforms( &m_frame->GetOriginTransforms() );
+
+    bool different = false;
+    wxVariant commonVal;
+
+    aWritable = true;
+
+    for( EDA_ITEM* item : aSelection )
+    {
+        if( !propMgr.IsAvailableFor( TYPE_HASH( *item ), aProperty, item ) )
+            return false;
+
+        // If read-only for any of the selection, read-only for the whole selection.
+        if( !aProperty->Writeable( item ) )
+            aWritable = false;
+
+        wxVariant value;
+
+        if( getItemValue( item, aProperty, value ) )
+        {
+            // Null value indicates different property values between items
+            if( !different && !aValue.IsNull() && value != aValue )
+            {
+                different = true;
+                aValue.MakeNull();
+            }
+            else if( !different )
+            {
+                aValue = value;
+            }
+        }
+        else
+        {
+            // getItemValue returned false -- not available for this item
+            return false;
+        }
+    }
+
+    return true;
+}
+
+
 void PROPERTIES_PANEL::onShow( wxShowEvent& aEvent )
 {
     if( aEvent.IsShown() )

common/widgets/properties_panel.h

@@ -108,6 +108,18 @@ protected:
      */
     bool getItemValue( EDA_ITEM* aItem, PROPERTY_BASE* aProperty, wxVariant& aValue );
 
+    /**
+     * Processes a selection and determines whether the given property should be available or not
+     * and what the common value should be for the items in the selection.
+     * @param aSelection is a set of EDA_ITEMs to process
+     * @param aProperty is the property to look up
+     * @param aValue will be filled with the value common to the selection, or null if different
+     * @param aWritable will be set to whether or not the property can be written for the selection
+     * @return true if the property is available for all the items in the selection
+     */
+    bool extractValueAndWritability( const SELECTION& aSelection, PROPERTY_BASE* aProperty,
+                                     wxVariant& aValue, bool& aWritable );
+
 protected:
     std::vector<PROPERTY_BASE*> m_displayed;
     wxPropertyGrid* m_grid;

pcbnew/widgets/pcb_properties_panel.cpp

@@ -111,7 +111,6 @@ void PCB_PROPERTIES_PANEL::AfterCommit()
 
 void PCB_PROPERTIES_PANEL::updatePropertyValues( const SELECTION& aSelection )
 {
-    // TODO: Refactor to reduce duplication with PROPERTIES_PANEL::rebuildProperties
     BOARD_ITEM* firstItem = static_cast<BOARD_ITEM*>( aSelection.Front() );
 
     for( wxPropertyGridIterator it = m_grid->GetIterator(); !it.AtEnd(); it.Next() )
@@ -123,29 +122,10 @@ void PCB_PROPERTIES_PANEL::updatePropertyValues( const SELECTION& aSelection )
         wxCHECK2( property, continue );
 
         bool writeable = true;
-        bool different = false;
         wxVariant commonVal;
 
-        for( EDA_ITEM* edaItem : aSelection )
+        // Availablility of the property should not be changing here
-        {
+        wxASSERT( extractValueAndWritability( aSelection, property, commonVal, writeable ) );
-            writeable &= property->Writeable( edaItem );
-
-            wxVariant value = commonVal;
-
-            if( getItemValue( edaItem, property, value ) )
-            {
-                // Null value indicates different property values between items
-                if( !different && !commonVal.IsNull() && value != commonVal )
-                {
-                    different = true;
-                    commonVal.MakeNull();
-                }
-                else if( !different )
-                {
-                    commonVal = value;
-                }
-            }
-        }
 
         pgProp->SetValue( commonVal );
         pgProp->Enable( writeable );