haskal
/
kicad
1
0
Fork 0
Code Issues Pull Requests Packages Projects Releases Wiki Activity

Improve validation of symbol fields editor. 

Don't beep when inserting character from focused grid cell (but
before editor is opened).

Handle reference validation separately from name validation and
separately from user field value validation.  The old way of setting
the fieldId on the validator wasn't working because the validator
gets copied.

Run validation when leaving cell.  Don't just check for empty
(particularly for fields that CAN be empty).

Fixes: lp:1782917
* https://bugs.launchpad.net/kicad/+bug/1782917

(cherry picked from commit 79e04de)
This commit is contained in:
Jeff Young 2018-07-21 21:23:13 +01:00
parent 3a9e98c8c7
commit 07a665f4fd
9 changed files with 162 additions and 92 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

9
common/validators.cpp
View File

@ -41,12 +41,12 @@ GRID_CELL_TEXT_EDITOR::GRID_CELL_TEXT_EDITOR() : wxGridCellTextEditor()
}
void GRID_CELL_TEXT_EDITOR::SetValidator(const wxValidator& validator)
void GRID_CELL_TEXT_EDITOR::SetValidator( const wxValidator& validator )
{
// keep our own copy because wxGridCellTextEditor's is annoyingly private
m_validator.reset( static_cast<wxValidator*>( validator.Clone() ) );
wxGridCellTextEditor::SetValidator( validator );
wxGridCellTextEditor::SetValidator( *m_validator );
}
@ -58,8 +58,11 @@ void GRID_CELL_TEXT_EDITOR::StartingKey( wxKeyEvent& event )
m_validator.get()->ProcessEvent( event );
}
if( !event.WasProcessed() )
if( event.GetSkipped() )
{
wxGridCellTextEditor::StartingKey( event );
event.Skip( false );
}
}

48
common/widgets/grid_text_button_helpers.cpp
View File

@ -60,6 +60,54 @@ void GRID_CELL_TEXT_BUTTON::SetSize( const wxRect& aRect )
}
void GRID_CELL_TEXT_BUTTON::StartingKey( wxKeyEvent& event )
{
// Note: this is a copy of wxGridCellTextEditor's StartingKey()
// Since this is now happening in the EVT_CHAR event EmulateKeyPress is no
// longer an appropriate way to get the character into the text control.
// Do it ourselves instead. We know that if we get this far that we have
// a valid character, so not a whole lot of testing needs to be done.
wxTextEntry* textEntry = dynamic_cast<wxTextEntry*>( Combo() );
int ch;
bool isPrintable;
#if wxUSE_UNICODE
ch = event.GetUnicodeKey();
if ( ch != WXK_NONE )
isPrintable = true;
else
#endif // wxUSE_UNICODE
{
ch = event.GetKeyCode();
isPrintable = ch >= WXK_SPACE && ch < WXK_START;
}
switch (ch)
{
case WXK_DELETE:
// Delete the initial character when starting to edit with DELETE.
textEntry->Remove(0, 1);
break;
case WXK_BACK:
// Delete the last character when starting to edit with BACKSPACE.
{
const long pos = textEntry->GetLastPosition();
textEntry->Remove(pos - 1, pos);
}
break;
default:
if ( isPrintable )
textEntry->WriteText(static_cast<wxChar>(ch));
break;
}
}
void GRID_CELL_TEXT_BUTTON::BeginEdit( int aRow, int aCol, wxGrid* aGrid )
{
auto evtHandler = static_cast<wxGridCellEditorEvtHandler*>( m_control->GetEventHandler() );

13
eeschema/dialogs/dialog_edit_component_in_schematic.cpp
View File

@ -357,7 +357,7 @@ bool DIALOG_EDIT_COMPONENT_IN_SCHEMATIC::Validate()
if( !SCH_COMPONENT::IsReferenceStringValid( m_fields->at( REFERENCE ).GetText() ) )
{
DisplayErrorMessage( nullptr, _( "References must start with a letter." ) );
DisplayErrorMessage( this, _( "References must start with a letter." ) );
m_delayedFocusColumn = FDC_VALUE;
m_delayedFocusRow = REFERENCE;
@ -544,14 +544,17 @@ void DIALOG_EDIT_COMPONENT_IN_SCHEMATIC::OnOKButtonClick( wxCommandEvent& event
void DIALOG_EDIT_COMPONENT_IN_SCHEMATIC::OnGridCellChanging( wxGridEvent& event )
{
if( event.GetString().IsEmpty() )
{
DisplayErrorMessage( this, _( "Field value cannot be empty." ) );
event.Veto();
wxGridCellEditor* editor = m_grid->GetCellEditor( event.GetRow(), event.GetCol() );
wxControl* control = editor->GetControl();
if( control && control->GetValidator() && !control->GetValidator()->Validate( control ) )
{
event.Veto();
m_delayedFocusRow = event.GetRow();
m_delayedFocusColumn = event.GetCol();
}
editor->DecRef();
}

11
eeschema/dialogs/dialog_edit_libentry_fields_in_lib.cpp
View File

@ -322,14 +322,17 @@ void DIALOG_EDIT_LIBENTRY_FIELDS_IN_LIB::OnEditSpiceModel( wxCommandEvent& event
void DIALOG_EDIT_LIBENTRY_FIELDS_IN_LIB::OnGridCellChanging( wxGridEvent& event )
{
if( event.GetCol() == FDC_NAME && event.GetString().IsEmpty() )
{
DisplayErrorMessage( this, _( "Fields must have a name." ) );
event.Veto();
wxGridCellEditor* editor = m_grid->GetCellEditor( event.GetRow(), event.GetCol() );
wxControl* control = editor->GetControl();
if( control && control->GetValidator() && !control->GetValidator()->Validate( control ) )
{
event.Veto();
m_delayedFocusRow = event.GetRow();
m_delayedFocusColumn = event.GetCol();
}
editor->DecRef();
}

118
eeschema/fields_grid_table.cpp
View File

@ -48,17 +48,23 @@ FIELDS_GRID_TABLE<T>::FIELDS_GRID_TABLE( DIALOG_SHIM* aDialog, bool aInLibEdit,
m_userUnits( aDialog->GetUserUnits() ),
m_part( aPart ),
m_inLibEdit( aInLibEdit ),
m_valueValidator( aInLibEdit, REFERENCE )
m_fieldNameValidator( aInLibEdit, FIELD_NAME ),
m_referenceValidator( aInLibEdit, REFERENCE )
{
// Build the column attributes.
// Build the various grid cell attributes.
m_readOnlyAttr = new wxGridCellAttr;
m_readOnlyAttr->SetReadOnly( true );
m_valueColAttr = new wxGridCellAttr;
GRID_CELL_TEXT_EDITOR* textEditor = new GRID_CELL_TEXT_EDITOR();
textEditor->SetValidator( m_valueValidator );
m_valueColAttr->SetEditor( textEditor );
m_fieldNameAttr = new wxGridCellAttr;
GRID_CELL_TEXT_EDITOR* nameEditor = new GRID_CELL_TEXT_EDITOR();
nameEditor->SetValidator( m_fieldNameValidator );
m_fieldNameAttr->SetEditor( nameEditor );
m_referenceAttr = new wxGridCellAttr;
GRID_CELL_TEXT_EDITOR* referenceEditor = new GRID_CELL_TEXT_EDITOR();
referenceEditor->SetValidator( m_referenceValidator );
m_referenceAttr->SetEditor( referenceEditor );
m_footprintAttr = new wxGridCellAttr;
m_footprintAttr->SetEditor( new GRID_CELL_FOOTPRINT_EDITOR( aDialog ) );
@ -66,33 +72,33 @@ FIELDS_GRID_TABLE<T>::FIELDS_GRID_TABLE( DIALOG_SHIM* aDialog, bool aInLibEdit,
m_urlAttr = new wxGridCellAttr;
m_urlAttr->SetEditor( new GRID_CELL_URL_EDITOR( aDialog ) );
m_boolColAttr = new wxGridCellAttr;
m_boolColAttr->SetRenderer( new wxGridCellBoolRenderer() );
m_boolColAttr->SetEditor( new wxGridCellBoolEditor() );
m_boolColAttr->SetAlignment( wxALIGN_CENTER, wxALIGN_BOTTOM );
m_boolAttr = new wxGridCellAttr;
m_boolAttr->SetRenderer( new wxGridCellBoolRenderer() );
m_boolAttr->SetEditor( new wxGridCellBoolEditor() );
m_boolAttr->SetAlignment( wxALIGN_CENTER, wxALIGN_BOTTOM );
wxArrayString vAlignNames;
vAlignNames.Add( _( "Top" ) );
vAlignNames.Add( _( "Center" ) );
vAlignNames.Add( _( "Bottom" ) );
m_vAlignColAttr = new wxGridCellAttr;
m_vAlignColAttr->SetEditor( new wxGridCellChoiceEditor( vAlignNames ) );
m_vAlignColAttr->SetAlignment( wxALIGN_CENTER, wxALIGN_BOTTOM );
m_vAlignAttr = new wxGridCellAttr;
m_vAlignAttr->SetEditor( new wxGridCellChoiceEditor( vAlignNames ) );
m_vAlignAttr->SetAlignment( wxALIGN_CENTER, wxALIGN_BOTTOM );
wxArrayString hAlignNames;
hAlignNames.Add( _( "Left" ) );
hAlignNames.Add(_( "Center" ) );
hAlignNames.Add(_( "Right" ) );
m_hAlignColAttr = new wxGridCellAttr;
m_hAlignColAttr->SetEditor( new wxGridCellChoiceEditor( hAlignNames ) );
m_hAlignColAttr->SetAlignment( wxALIGN_CENTER, wxALIGN_BOTTOM );
m_hAlignAttr = new wxGridCellAttr;
m_hAlignAttr->SetEditor( new wxGridCellChoiceEditor( hAlignNames ) );
m_hAlignAttr->SetAlignment( wxALIGN_CENTER, wxALIGN_BOTTOM );
wxArrayString orientationNames;
orientationNames.Add( _( "Horizontal" ) );
orientationNames.Add(_( "Vertical" ) );
m_orientationColAttr = new wxGridCellAttr;
m_orientationColAttr->SetEditor( new wxGridCellChoiceEditor( orientationNames ) );
m_orientationColAttr->SetAlignment( wxALIGN_CENTER, wxALIGN_BOTTOM );
m_orientationAttr = new wxGridCellAttr;
m_orientationAttr->SetEditor( new wxGridCellChoiceEditor( orientationNames ) );
m_orientationAttr->SetAlignment( wxALIGN_CENTER, wxALIGN_BOTTOM );
}
@ -100,13 +106,14 @@ template <class T>
FIELDS_GRID_TABLE<T>::~FIELDS_GRID_TABLE()
{
m_readOnlyAttr->DecRef();
m_boolColAttr->DecRef();
m_valueColAttr->DecRef();
m_fieldNameAttr->DecRef();
m_boolAttr->DecRef();
m_referenceAttr->DecRef();
m_footprintAttr->DecRef();
m_urlAttr->DecRef();
m_vAlignColAttr->DecRef();
m_hAlignColAttr->DecRef();
m_orientationColAttr->DecRef();
m_vAlignAttr->DecRef();
m_hAlignAttr->DecRef();
m_orientationAttr->DecRef();
}
@ -177,24 +184,35 @@ wxGridCellAttr* FIELDS_GRID_TABLE<T>::GetAttr( int aRow, int aCol, wxGridCellAtt
return m_readOnlyAttr;
}
else
return nullptr;
{
m_fieldNameAttr->IncRef();
return m_fieldNameAttr;
}
case FDC_VALUE:
// This field is the lib name and the default value when loading this component in
// schematic. The value is now not editable here (in this dialog) because changing
// it is equivalent to create a new component or alias. This is handled in libedit,
// not in this dialog.
if( m_inLibEdit && aRow == VALUE )
if( aRow == REFERENCE )
{
m_readOnlyAttr->IncRef();
return m_readOnlyAttr;
m_referenceAttr->IncRef();
return m_referenceAttr;
}
// For power symbols, the value is not editable, because value and pin
// name must be same and can be edited only in library editor
else if( m_part && m_part->IsPower() )
else if( aRow == VALUE )
{
m_readOnlyAttr->IncRef();
return m_readOnlyAttr;
if( m_inLibEdit )
{
// This field is the lib name and the default value when loading this component
// in schematic. The value is now not editable here (in this dialog) because
// changing it is equivalent to create a new component or alias. This is handled
// in libedit, not in this dialog.
m_readOnlyAttr->IncRef();
return m_readOnlyAttr;
}
else if( m_part && m_part->IsPower() )
{
// For power symbols, the value is not editable, because value and pin name must
// be the same and can be edited only in library editor.
m_readOnlyAttr->IncRef();
return m_readOnlyAttr;
}
}
else if( aRow == FOOTPRINT )
{
@ -206,15 +224,7 @@ wxGridCellAttr* FIELDS_GRID_TABLE<T>::GetAttr( int aRow, int aCol, wxGridCellAtt
m_urlAttr->IncRef();
return m_urlAttr;
}
else
{
// Some fields have different validation semantics. Make sure the
// validator knows what it's validating.
m_valueValidator.SetFieldId( aRow );
m_valueColAttr->IncRef();
return m_valueColAttr;
}
return nullptr;
case FDC_TEXT_SIZE:
case FDC_POSX:
@ -222,22 +232,22 @@ wxGridCellAttr* FIELDS_GRID_TABLE<T>::GetAttr( int aRow, int aCol, wxGridCellAtt
return nullptr;
case FDC_H_ALIGN:
m_hAlignColAttr->IncRef();
return m_hAlignColAttr;
m_hAlignAttr->IncRef();
return m_hAlignAttr;
case FDC_V_ALIGN:
m_vAlignColAttr->IncRef();
return m_vAlignColAttr;
m_vAlignAttr->IncRef();
return m_vAlignAttr;
case FDC_ORIENTATION:
m_orientationColAttr->IncRef();
return m_orientationColAttr;
m_orientationAttr->IncRef();
return m_orientationAttr;
case FDC_SHOWN:
case FDC_ITALIC:
case FDC_BOLD:
m_boolColAttr->IncRef();
return m_boolColAttr;
m_boolAttr->IncRef();
return m_boolAttr;
default:
wxFAIL;

15
eeschema/fields_grid_table.h
View File

@ -30,7 +30,6 @@
#include <sch_component.h>
#include <grid_tricks.h>
class FIELDS_GRID_TRICKS : public GRID_TRICKS
{
public:
@ -96,16 +95,18 @@ private:
LIB_PART* m_part;
bool m_inLibEdit;
SCH_FIELD_VALIDATOR m_valueValidator;
SCH_FIELD_VALIDATOR m_fieldNameValidator;
SCH_FIELD_VALIDATOR m_referenceValidator;
wxGridCellAttr* m_readOnlyAttr;
wxGridCellAttr* m_valueColAttr;
wxGridCellAttr* m_fieldNameAttr;
wxGridCellAttr* m_referenceAttr;
wxGridCellAttr* m_footprintAttr;
wxGridCellAttr* m_urlAttr;
wxGridCellAttr* m_boolColAttr;
wxGridCellAttr* m_vAlignColAttr;
wxGridCellAttr* m_hAlignColAttr;
wxGridCellAttr* m_orientationColAttr;
wxGridCellAttr* m_boolAttr;
wxGridCellAttr* m_vAlignAttr;
wxGridCellAttr* m_hAlignAttr;
wxGridCellAttr* m_orientationAttr;
};

30
eeschema/sch_validators.cpp
View File

@ -30,12 +30,12 @@
#include <sch_validators.h>
#include <template_fieldnames.h>
SCH_FIELD_VALIDATOR::SCH_FIELD_VALIDATOR( bool aIsCmplibEditor,
int aFieldId, wxString* aValue ) :
SCH_FIELD_VALIDATOR::SCH_FIELD_VALIDATOR( bool aIsLibEditor, int aFieldId, wxString* aValue ) :
wxTextValidator( wxFILTER_EXCLUDE_CHAR_LIST, aValue )
{
m_fieldId = aFieldId;
m_isLibEditor = aIsCmplibEditor;
m_isLibEditor = aIsLibEditor;
// Fields cannot contain carriage returns, line feeds, or tabs.
wxString excludes( "\r\n\t" );
@ -49,7 +49,7 @@ SCH_FIELD_VALIDATOR::SCH_FIELD_VALIDATOR( bool aIsCmplibEditor,
long style = GetStyle();
// The reference and value fields cannot be empty.
if( aFieldId == REFERENCE || aFieldId == VALUE )
if( aFieldId == REFERENCE || aFieldId == VALUE || aFieldId == FIELD_NAME )
style |= wxFILTER_EMPTY;
SetStyle( style );
@ -82,19 +82,20 @@ bool SCH_FIELD_VALIDATOR::Validate( wxWindow *aParent )
switch( m_fieldId )
{
case REFERENCE: fieldName = _( "reference designator" ); break;
case VALUE: fieldName = _( "value" ); break;
case FOOTPRINT: fieldName = _( "footprint" ); break;
case DATASHEET: fieldName = _( "data sheet" ); break;
default: fieldName = _( "user defined" ); break;
case FIELD_NAME: fieldName = _( "field name" ); break;
case REFERENCE: fieldName = _( "reference field" ); break;
case VALUE: fieldName = _( "value field" ); break;
case FOOTPRINT: fieldName = _( "footprint field" ); break;
case DATASHEET: fieldName = _( "datasheet field" ); break;
default: fieldName = _( "user defined field" ); break;
};
wxString errorMsg;
wxString msg;
// We can only do some kinds of validation once the input is complete, so
// check for them here:
if( HasFlag( wxFILTER_EMPTY ) && val.empty() )
errorMsg.Printf( _( "The %s field cannot be empty." ), fieldName );
msg.Printf( _( "The %s cannot be empty." ), fieldName );
else if( HasFlag( wxFILTER_EXCLUDE_CHAR_LIST ) && ContainsExcludedCharacters( val ) )
{
wxArrayString whiteSpace;
@ -124,15 +125,14 @@ bool SCH_FIELD_VALIDATOR::Validate( wxWindow *aParent )
else
wxCHECK_MSG( false, true, wxT( "Invalid illegal character in field validator." ) );
errorMsg.Printf( _( "The %s field cannot contain %s characters." ), fieldName, badChars );
msg.Printf( _( "The %s cannot contain %s characters." ), fieldName, badChars );
}
if ( !errorMsg.empty() )
if ( !msg.empty() )
{
m_validatorWindow->SetFocus();
wxMessageBox( errorMsg, _( "Field Validation Error" ),
wxOK | wxICON_EXCLAMATION, aParent );
wxMessageBox( msg, _( "Field Validation Error" ), wxOK | wxICON_EXCLAMATION, aParent );
return false;
}

9
eeschema/sch_validators.h
View File

@ -30,9 +30,12 @@
#ifndef _SCH_VALIDATORS_H_
#define _SCH_VALIDATORS_H_
#include <wx/valtext.h>
#define FIELD_NAME -1
/**
* class SCH_FILED_VALIDATOR
*
@ -50,7 +53,7 @@ class SCH_FIELD_VALIDATOR : public wxTextValidator
bool m_isLibEditor;
public:
SCH_FIELD_VALIDATOR( bool aIsCmplibEditor, int aFieldId, wxString* aValue = NULL );
SCH_FIELD_VALIDATOR( bool aIsLibEditor, int aFieldId, wxString* aValue = NULL );
SCH_FIELD_VALIDATOR( const SCH_FIELD_VALIDATOR& aValidator );
@ -66,8 +69,6 @@ public:
* @return true if the text in the control is valid otherwise false.
*/
virtual bool Validate( wxWindow *aParent ) override;
void SetFieldId( int aFieldId ) { m_fieldId = aFieldId; }
};

1
include/widgets/grid_text_button_helpers.h
View File

@ -42,6 +42,7 @@ public:
void SetSize( const wxRect& aRect ) override;
void StartingKey( wxKeyEvent& event ) override;
void BeginEdit( int aRow, int aCol, wxGrid* aGrid ) override;
bool EndEdit( int , int , const wxGrid* , const wxString& , wxString *aNewVal ) override;
void ApplyEdit( int aRow, int aCol, wxGrid* aGrid ) override;