Don't conflate generated fields (${DNP}) and fields with variable references.

They're not quite the same thing as the later
can have un-applied values which require expanding.

Fixes https://gitlab.com/kicad/code/kicad/-/issues/20417
This commit is contained in:
Jeff Young 2025-04-18 11:01:13 +01:00 committed by Seth Hillbrand
parent c2df0951f9
commit 8113824f7c
8 changed files with 94 additions and 85 deletions

View File

@ -118,11 +118,12 @@ wxString ExpandTextVars( const wxString& aSource,
}
wxString GetTextVars( const wxString& aSource )
wxString GetGeneratedFieldDisplayName( const wxString& aSource )
{
std::function<bool( wxString* )> tokenExtractor =
[&]( wxString* token ) -> bool
{
*token = *token; // token value is the token name
return true;
};
@ -130,9 +131,10 @@ wxString GetTextVars( const wxString& aSource )
}
bool IsTextVar( const wxString& aSource )
bool IsGeneratedField( const wxString& aSource )
{
return aSource.StartsWith( wxS( "${" ) );
static wxRegEx expr( wxS( "^\\$\\{\\w*\\}$" ) );
return expr.Matches( aSource );
}

View File

@ -264,7 +264,7 @@ DIALOG_FIELD_PROPERTIES::DIALOG_FIELD_PROPERTIES( SCH_BASE_FRAME* aParent, const
init();
if( m_isSheetFilename || m_field->IsNamedVariable() )
if( m_isSheetFilename || m_field->IsGeneratedField() )
{
m_StyledTextCtrl->Enable( false );
m_TextCtrl->Enable( false );

View File

@ -319,8 +319,8 @@ DIALOG_SYMBOL_FIELDS_TABLE::DIALOG_SYMBOL_FIELDS_TABLE( SCH_EDIT_FRAME* parent,
if( ( m_job->m_fieldsLabels.size() > i ) && !m_job->m_fieldsLabels[i].IsEmpty() )
field.label = m_job->m_fieldsLabels[i];
else if( IsTextVar( field.name ) )
field.label = GetTextVars( field.name );
else if( IsGeneratedField( field.name ) )
field.label = GetGeneratedFieldDisplayName( field.name );
else
field.label = field.name;
@ -460,7 +460,7 @@ void DIALOG_SYMBOL_FIELDS_TABLE::SetupColumnProperties( int aCol )
attr->SetReadOnly(); // not really; we delegate interactivity to GRID_TRICKS
m_dataModel->SetColAttr( attr, aCol );
}
else if( IsTextVar( m_dataModel->GetColFieldName( aCol ) ) )
else if( IsGeneratedField( m_dataModel->GetColFieldName( aCol ) ) )
{
attr->SetReadOnly();
m_dataModel->SetColAttr( attr, aCol );
@ -728,7 +728,7 @@ void DIALOG_SYMBOL_FIELDS_TABLE::LoadFieldNames()
}
for( const wxString& fieldName : userFieldNames )
AddField( fieldName, GetTextVars( fieldName ), true, false );
AddField( fieldName, GetGeneratedFieldDisplayName( fieldName ), true, false );
// Add any templateFieldNames which aren't already present in the userFieldNames
for( const TEMPLATE_FIELDNAME& templateFieldname :
@ -736,8 +736,8 @@ void DIALOG_SYMBOL_FIELDS_TABLE::LoadFieldNames()
{
if( userFieldNames.count( templateFieldname.m_Name ) == 0 )
{
AddField( templateFieldname.m_Name, GetTextVars( templateFieldname.m_Name ), false,
false );
AddField( templateFieldname.m_Name, GetGeneratedFieldDisplayName( templateFieldname.m_Name ),
false, false );
}
}
}
@ -768,7 +768,7 @@ void DIALOG_SYMBOL_FIELDS_TABLE::OnAddField( wxCommandEvent& event )
}
}
AddField( fieldName, GetTextVars( fieldName ), true, false, true );
AddField( fieldName, GetGeneratedFieldDisplayName( fieldName ), true, false, true );
SetupColumnProperties( m_dataModel->GetColsCount() - 1 );
@ -1941,7 +1941,7 @@ void DIALOG_SYMBOL_FIELDS_TABLE::doApplyBomPreset( const BOM_PRESET& aPreset )
// Properties like label, etc. will be added in the next loop
if( !found )
AddField( fieldName, GetTextVars( fieldName ), false, false );
AddField( fieldName, GetGeneratedFieldDisplayName( fieldName ), false, false );
}
// Sync all fields

View File

@ -552,7 +552,7 @@ int EESCHEMA_JOBS_HANDLER::JobExportBom( JOB* aJob )
}
for( const wxString& fieldName : userFieldNames )
dataModel.AddColumn( fieldName, GetTextVars( fieldName ), true );
dataModel.AddColumn( fieldName, GetGeneratedFieldDisplayName( fieldName ), true );
// Add any templateFieldNames which aren't already present in the userFieldNames
for( const TEMPLATE_FIELDNAME& templateFieldname :
@ -560,7 +560,7 @@ int EESCHEMA_JOBS_HANDLER::JobExportBom( JOB* aJob )
{
if( userFieldNames.count( templateFieldname.m_Name ) == 0 )
{
dataModel.AddColumn( templateFieldname.m_Name, GetTextVars( templateFieldname.m_Name ),
dataModel.AddColumn( templateFieldname.m_Name, GetGeneratedFieldDisplayName( templateFieldname.m_Name ),
false );
}
}
@ -658,8 +658,8 @@ int EESCHEMA_JOBS_HANDLER::JobExportBom( JOB* aJob )
if( ( aBomJob->m_fieldsLabels.size() > i ) && !aBomJob->m_fieldsLabels[i].IsEmpty() )
field.label = aBomJob->m_fieldsLabels[i];
else if( IsTextVar( field.name ) )
field.label = GetTextVars( field.name );
else if( IsGeneratedField( field.name ) )
field.label = GetGeneratedFieldDisplayName( field.name );
else
field.label = field.name;

View File

@ -69,10 +69,10 @@ void FIELDS_EDITOR_GRID_DATA_MODEL::updateDataStoreSymbolField( const SCH_SYMBOL
wxString value = aSymbol.Schematic()->ConvertKIIDsToRefs( field->GetText() );
m_dataStore[aSymbol.m_Uuid][aFieldName] = value;
}
else if( IsTextVar( aFieldName ) )
else if( IsGeneratedField( aFieldName ) )
{
// Handle fields with variables as names that are not present in the symbol
// by giving them the correct value
// Handle generated fields with variables as names (e.g. ${QUANTITY}) that are not present in
// the symbol by giving them the correct value
m_dataStore[aSymbol.m_Uuid][aFieldName] = aFieldName;
}
else
@ -227,19 +227,28 @@ wxString FIELDS_EDITOR_GRID_DATA_MODEL::GetValue( const DATA_MODEL_ROW& group, i
return INDETERMINATE_STATE;
}
wxString refFieldValue;
wxString refFieldValue = m_dataStore[symbolID][m_cols[aCol].m_fieldName];
// Only resolve vars on actual variables, otherwise we want to get
// our values out of the datastore so we can show/export un-applied values
if( resolveVars
&& ( IsTextVar( m_cols[aCol].m_fieldName )
|| IsTextVar( m_dataStore[symbolID][m_cols[aCol].m_fieldName] ) ) )
if( resolveVars )
{
refFieldValue = getFieldShownText( ref, m_cols[aCol].m_fieldName );
}
else
{
refFieldValue = m_dataStore[symbolID][m_cols[aCol].m_fieldName];
if( IsGeneratedField( m_cols[aCol].m_fieldName ) )
{
// Generated fields (e.g. ${QUANTITY}) can't have un-applied values as they're
// read-only. Resolve them against the field.
refFieldValue = getFieldShownText( ref, m_cols[aCol].m_fieldName );
}
else if( refFieldValue.Contains( wxT( "${" ) ) )
{
// Resolve variables in the un-applied value using the parent symbol and instance
// data.
std::function<bool( wxString* )> symbolResolver =
[&]( wxString* token ) -> bool
{
return ref.GetSymbol()->ResolveTextVar( &ref.GetSheetPath(), token );
};
refFieldValue = ExpandTextVars( refFieldValue, & symbolResolver );
}
}
if( listMixedValues )
@ -308,9 +317,9 @@ void FIELDS_EDITOR_GRID_DATA_MODEL::SetValue( int aRow, int aCol, const wxString
{
wxCHECK_RET( aCol >= 0 && aCol < static_cast<int>( m_cols.size() ), wxS( "Invalid column number" ) );
// Can't modify references or text variables column, e.g. ${QUANTITY}
// Can't modify references or generated fields (e.g. ${QUANTITY})
if( ColIsReference( aCol )
|| ( IsTextVar( m_cols[aCol].m_fieldName ) && !ColIsAttribute( aCol ) ) )
|| ( IsGeneratedField( m_cols[aCol].m_fieldName ) && !ColIsAttribute( aCol ) ) )
{
return;
}
@ -478,14 +487,13 @@ bool FIELDS_EDITOR_GRID_DATA_MODEL::groupMatch( const SCH_REFERENCE& lhRef,
if( !m_cols[i].m_group )
continue;
// If the field is a variable, we need to resolve it through the symbol
// to get the actual current value, otherwise we need to pull it out of the
// store so the refresh can regroup based on values that haven't been applied
// to the schematic yet.
// If the field is generated (e.g. ${QUANTITY}), we need to resolve it through the symbol
// to get the actual current value; otherwise we need to pull it out of the store so the
// refresh can regroup based on values that haven't been applied to the schematic yet.
wxString lh, rh;
if( IsTextVar( m_cols[i].m_fieldName )
|| IsTextVar( m_dataStore[lhRefID][m_cols[i].m_fieldName] ) )
if( IsGeneratedField( m_cols[i].m_fieldName )
|| IsGeneratedField( m_dataStore[lhRefID][m_cols[i].m_fieldName] ) )
{
lh = getFieldShownText( lhRef, m_cols[i].m_fieldName );
}
@ -494,8 +502,8 @@ bool FIELDS_EDITOR_GRID_DATA_MODEL::groupMatch( const SCH_REFERENCE& lhRef,
lh = m_dataStore[lhRefID][m_cols[i].m_fieldName];
}
if( IsTextVar( m_cols[i].m_fieldName )
|| IsTextVar( m_dataStore[rhRefID][m_cols[i].m_fieldName] ) )
if( IsGeneratedField( m_cols[i].m_fieldName )
|| IsGeneratedField( m_dataStore[rhRefID][m_cols[i].m_fieldName] ) )
{
rh = getFieldShownText( rhRef, m_cols[i].m_fieldName );
}
@ -529,9 +537,9 @@ wxString FIELDS_EDITOR_GRID_DATA_MODEL::getFieldShownText( const SCH_REFERENCE&
return field->GetShownText( &aRef.GetSheetPath(), false );
}
// Handle fields with variables as names that are not present in the symbol
// by giving them the correct value by resolving against the symbol
if( IsTextVar( aFieldName ) )
// Handle generated fields with variables as names (e.g. ${QUANTITY}) that are not present in
// the symbol by giving them the correct value by resolving against the symbol
if( IsGeneratedField( aFieldName ) )
{
int depth = 0;
const SCH_SHEET_PATH& path = aRef.GetSheetPath();
@ -811,9 +819,9 @@ void FIELDS_EDITOR_GRID_DATA_MODEL::ApplyData( SCH_COMMIT& aCommit )
continue;
}
// Skip special fields with variables as names (e.g. ${QUANTITY}),
// Skip generated fields with variables as names (e.g. ${QUANTITY});
// they can't be edited
if( IsTextVar( srcName ) )
if( IsGeneratedField( srcName ) )
continue;
SCH_FIELD* destField = symbol.GetField( srcName );

View File

@ -51,7 +51,7 @@ SCH_FIELD::SCH_FIELD() :
m_ordinal( 0 ),
m_showName( false ),
m_allowAutoPlace( true ),
m_isNamedVariable( false ),
m_isGeneratedField( false ),
m_autoAdded( false ),
m_showInChooser( true ),
m_renderCacheValid( false ),
@ -106,15 +106,15 @@ SCH_FIELD::SCH_FIELD( const SCH_FIELD& aField ) :
SCH_ITEM( aField ),
EDA_TEXT( aField )
{
m_private = aField.m_private;
m_private = aField.m_private;
setId( aField.m_id ); // will also set the layer
m_ordinal = aField.m_ordinal;
m_name = aField.m_name;
m_showName = aField.m_showName;
m_allowAutoPlace = aField.m_allowAutoPlace;
m_isNamedVariable = aField.m_isNamedVariable;
m_autoAdded = aField.m_autoAdded;
m_showInChooser = aField.m_showInChooser;
m_ordinal = aField.m_ordinal;
m_name = aField.m_name;
m_showName = aField.m_showName;
m_allowAutoPlace = aField.m_allowAutoPlace;
m_isGeneratedField = aField.m_isGeneratedField;
m_autoAdded = aField.m_autoAdded;
m_showInChooser = aField.m_showInChooser;
m_renderCache.clear();
@ -137,13 +137,13 @@ SCH_FIELD& SCH_FIELD::operator=( const SCH_FIELD& aField )
{
EDA_TEXT::operator=( aField );
m_private = aField.m_private;
m_private = aField.m_private;
setId( aField.m_id ); // will also set the layer
m_ordinal = aField.m_ordinal;
m_name = aField.m_name;
m_showName = aField.m_showName;
m_allowAutoPlace = aField.m_allowAutoPlace;
m_isNamedVariable = aField.m_isNamedVariable;
m_ordinal = aField.m_ordinal;
m_name = aField.m_name;
m_showName = aField.m_showName;
m_allowAutoPlace = aField.m_allowAutoPlace;
m_isGeneratedField = aField.m_isGeneratedField;
m_renderCache.clear();
@ -185,7 +185,7 @@ void SCH_FIELD::setId( FIELD_T aId )
wxString SCH_FIELD::GetShownName() const
{
return m_isNamedVariable ? GetTextVars( GetName() ) : GetName();
return m_isGeneratedField ? GetGeneratedFieldDisplayName( GetName() ) : GetName();
}
@ -415,7 +415,7 @@ void SCH_FIELD::swapData( SCH_ITEM* aItem )
std::swap( m_layer, item->m_layer );
std::swap( m_showName, item->m_showName );
std::swap( m_allowAutoPlace, item->m_allowAutoPlace );
std::swap( m_isNamedVariable, item->m_isNamedVariable );
std::swap( m_isGeneratedField, item->m_isGeneratedField );
std::swap( m_private, item->m_private );
SwapText( *item );
SwapAttributes( *item );
@ -1067,18 +1067,17 @@ void SCH_FIELD::DoHypertextAction( EDA_DRAW_FRAME* aFrame ) const
void SCH_FIELD::SetName( const wxString& aName )
{
m_name = aName;
m_isNamedVariable = m_name.StartsWith( wxT( "${" ) );
m_isGeneratedField = ::IsGeneratedField( aName );
if( m_isNamedVariable )
if( m_isGeneratedField )
EDA_TEXT::SetText( aName );
}
void SCH_FIELD::SetText( const wxString& aText )
{
// Don't allow modification of text value when using named variables
// as field name.
if( m_isNamedVariable )
// Don't allow modification of text value of generated fields.
if( m_isGeneratedField )
return;
// Mandatory fields should not have leading or trailing whitespace.
@ -1414,7 +1413,7 @@ bool SCH_FIELD::operator==( const SCH_FIELD& aOther ) const
if( GetPosition() != aOther.GetPosition() )
return false;
if( IsNamedVariable() != aOther.IsNamedVariable() )
if( IsGeneratedField() != aOther.IsGeneratedField() )
return false;
if( IsNameShown() != aOther.IsNameShown() )
@ -1455,7 +1454,7 @@ double SCH_FIELD::Similarity( const SCH_ITEM& aOther ) const
if( GetPosition() != field.GetPosition() )
similarity *= 0.5;
if( IsNamedVariable() != field.IsNamedVariable() )
if( IsGeneratedField() != field.IsGeneratedField() )
similarity *= 0.5;
if( IsNameShown() != field.IsNameShown() )
@ -1616,17 +1615,17 @@ static struct SCH_FIELD_DESC
propMgr.Mask( TYPE_HASH( SCH_FIELD ), TYPE_HASH( EDA_TEXT ), _HKI( "Orientation" ) );
auto isNotNamedVariable =
auto isNotGeneratedField =
[]( INSPECTABLE* aItem ) -> bool
{
if( SCH_FIELD* field = dynamic_cast<SCH_FIELD*>( aItem ) )
return !field->IsNamedVariable();
return !field->IsGeneratedField();
return true;
};
propMgr.OverrideWriteability( TYPE_HASH( SCH_FIELD ), TYPE_HASH( EDA_TEXT ), _HKI( "Text" ),
isNotNamedVariable );
isNotGeneratedField );
auto isNonMandatoryField =

View File

@ -209,12 +209,12 @@ public:
void SetNameShown( bool aShown = true ) { m_showName = aShown; }
/**
* Named variables are fields whose names are variables like ${VAR}.
* Generated fields are fields whose names are variables like ${VAR}.
*
* The shown name of these fields is VAR and the value is resolved from
* ${VAR}
*/
bool IsNamedVariable() const { return m_isNamedVariable; }
bool IsGeneratedField() const { return m_isGeneratedField; }
bool CanAutoplace() const { return m_allowAutoPlace; }
void SetCanAutoplace( bool aCanPlace ) { m_allowAutoPlace = aCanPlace; }
@ -333,17 +333,17 @@ protected:
void setId( FIELD_T aId );
private:
FIELD_T m_id; ///< Field id, @see enum FIELD_T
int m_ordinal; ///< Sort order for non-mandatory fields
FIELD_T m_id; ///< Field id, @see enum FIELD_T
int m_ordinal; ///< Sort order for non-mandatory fields
wxString m_name;
bool m_showName; ///< Render the field name in addition to its value
bool m_allowAutoPlace; ///< This field can be autoplaced
bool m_isNamedVariable; ///< If the field name is a variable name, e.g. ${DNP}
///< then the value field is forced to be the same as the name
bool m_showName; ///< Render the field name in addition to its value
bool m_allowAutoPlace; ///< This field can be autoplaced
bool m_isGeneratedField; ///< If the field name is a variable name (e.g. ${DNP}) then
///< the value field is forced to be the same as the name
bool m_autoAdded; ///< Was this field automatically added to a LIB_SYMBOL?
bool m_showInChooser; ///< This field is available as a data column for the chooser
bool m_autoAdded; ///< Was this field automatically added to a LIB_SYMBOL?
bool m_showInChooser; ///< This field is available as a data column for the chooser
mutable bool m_renderCacheValid;
mutable VECTOR2I m_renderCachePos;

View File

@ -100,12 +100,12 @@ KICOMMON_API wxString ExpandTextVars( const wxString& aSource, const PROJECT* aP
/**
* Returns any variables unexpanded, e.g. ${VAR} -> VAR
*/
KICOMMON_API wxString GetTextVars( const wxString& aSource );
KICOMMON_API wxString GetGeneratedFieldDisplayName( const wxString& aSource );
/**
* Returns true if the string is a text var, e.g starts with ${
* Returns true if the string is generated, e.g contains a single text var reference
*/
KICOMMON_API bool IsTextVar( const wxString& aSource );
KICOMMON_API bool IsGeneratedField( const wxString& aSource );
/**
* Replace any environment and/or text variables in URIs