← Back to team overview

kicad-developers team mailing list archive

A fix for our TEXT_CTRL_EVAL class.

 

Hi Orson,

Could you have a look into this small patch.
I am not sure I used the better way to fix an issue.

It fixes an issue when using SetValue() to change the value shown by this widget.

The issue was due to the fact TEXT_CTRL_EVAL stores internally the last entered value in a buffer.
So when calling SetValue() after a value was already entered, and if we try to modify
the displayed value by calling SetValue(), as soon as the widget has the focus, the previous value
replaces the new value.

This is easy to see in Move Exactly dialog:
- enter a value
- reset the value by the Reset button
- move the cursor to the corresponding text control widgets and click on it to give it the focus.

-- 
Jean-Pierre CHARRAS
 common/widgets/text_ctrl_eval.cpp    | 12 +++++++++---
 include/widgets/text_ctrl_eval.h     |  8 ++++++++
 pcbnew/dialogs/dialog_move_exact.cpp |  4 ++--
 3 files changed, 19 insertions(+), 5 deletions(-)

diff --git a/common/widgets/text_ctrl_eval.cpp b/common/widgets/text_ctrl_eval.cpp
index bf1dfc4..7ec0b68 100644
--- a/common/widgets/text_ctrl_eval.cpp
+++ b/common/widgets/text_ctrl_eval.cpp
@@ -37,13 +37,19 @@ TEXT_CTRL_EVAL::TEXT_CTRL_EVAL( wxWindow* aParent, wxWindowID aId, const
             wxCommandEventHandler( TEXT_CTRL_EVAL::onTextEnter ), NULL, this );
 }
 
+void TEXT_CTRL_EVAL::SetValue( const wxString& aValue)
+{
+    wxTextCtrl::SetValue( aValue );
+    evaluate();
+}
+
 
 void TEXT_CTRL_EVAL::onTextFocusGet( wxFocusEvent& aEvent )
 {
     auto oldStr = m_eval.textInput( this );
 
     if( oldStr )
-        SetValue( wxString::FromUTF8( oldStr ) );
+        wxTextCtrl::SetValue( wxString::FromUTF8( oldStr ) );
 
     aEvent.Skip();
 }
@@ -69,8 +75,8 @@ void TEXT_CTRL_EVAL::onTextEnter( wxCommandEvent& aEvent )
 void TEXT_CTRL_EVAL::evaluate()
 {
     if( GetValue().IsEmpty() )
-        SetValue( "0" );
+        wxTextCtrl::SetValue( "0" );
 
     if( m_eval.process( GetValue().mb_str(), this ) )
-        SetValue( wxString::FromUTF8( m_eval.result() ) );
+        wxTextCtrl::SetValue( wxString::FromUTF8( m_eval.result() ) );
 }
diff --git a/include/widgets/text_ctrl_eval.h b/include/widgets/text_ctrl_eval.h
index a523ea9..dfb64f4 100644
--- a/include/widgets/text_ctrl_eval.h
+++ b/include/widgets/text_ctrl_eval.h
@@ -47,6 +47,14 @@ public:
     {
     }
 
+    /**
+     * Set a new value in evaluator buffer, and display it
+     * in the wxTextCtrl.
+     * @param aValue is the new value to store and display
+     * if aValue is empty, the value "0" is stored and displayed
+     */
+    void SetValue( const wxString& aValue ) override;
+
 protected:
     ///> Numeric expression evaluator
     NumericEvaluator m_eval;
diff --git a/pcbnew/dialogs/dialog_move_exact.cpp b/pcbnew/dialogs/dialog_move_exact.cpp
index 3da9ab6..b78e9d9 100644
--- a/pcbnew/dialogs/dialog_move_exact.cpp
+++ b/pcbnew/dialogs/dialog_move_exact.cpp
@@ -250,7 +250,7 @@ void DIALOG_MOVE_EXACT::updateDlgTexts( bool aPolar )
 void DIALOG_MOVE_EXACT::OnClear( wxCommandEvent& event )
 {
     wxObject* obj = event.GetEventObject();
-    wxTextCtrl* entry = NULL;
+    TEXT_CTRL_EVAL* entry = NULL;
 
     if( obj == m_clearX )
     {
@@ -337,7 +337,7 @@ bool DIALOG_MOVE_EXACT::TransferDataFromWindow()
 
 void DIALOG_MOVE_EXACT::OnTextFocusLost( wxFocusEvent& event )
 {
-    wxTextCtrl* obj = static_cast<wxTextCtrl*>( event.GetEventObject() );
+    TEXT_CTRL_EVAL* obj = static_cast<TEXT_CTRL_EVAL*>( event.GetEventObject() );
 
     if( obj->GetValue().IsEmpty() )
         obj->SetValue( "0" );

Follow ups