kicad-developers team mailing list archive

[Maciej Sumiński <maciej.suminski@xxxxxxx>]

Hi Jean-Pierre,

Thank you for investigating the issue, I think you are correct about the
changes. The attached patches follow Jeff's suggestion about resetting
the previous expression value in NumericEvaluator, but only the modified
expression is cleared. I realize that right now there is no difference
whether all variables or a particular one is cleared, but I do not want
to lose the possibility of defining custom variables or referencing
other fields.

Regards,
Orson

On 02/25/2018 11:17 AM, jp charras wrote:
> 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.
> 
> 
> 
> _______________________________________________
> Mailing list: https://launchpad.net/~kicad-developers
> Post to     : kicad-developers@xxxxxxxxxxxxxxxxxxx
> Unsubscribe : https://launchpad.net/~kicad-developers
> More help   : https://help.launchpad.net/ListHelp
> 

From dc75131909db8d8fa83b10400e278f31441d738c Mon Sep 17 00:00:00 2001
From: Maciej Suminski <maciej.suminski@xxxxxxx>
Date: Tue, 27 Feb 2018 11:40:27 +0100
Subject: [PATCH 1/2] Change NumericEvaluator::clear() to optionally accept an
 object to clear

---
 common/libeval/numeric_evaluator.cpp | 4 +++-
 include/libeval/numeric_evaluator.h  | 4 ++--
 2 files changed, 5 insertions(+), 3 deletions(-)

diff --git a/common/libeval/numeric_evaluator.cpp b/common/libeval/numeric_evaluator.cpp
index 6945f715b..04cee6d15 100644
--- a/common/libeval/numeric_evaluator.cpp
+++ b/common/libeval/numeric_evaluator.cpp
@@ -96,12 +96,14 @@ NumericEvaluator :: init()
 }
 
 void
-NumericEvaluator :: clear()
+NumericEvaluator :: clear(const void* pObj)
 {
    free(clToken.token);
    clToken.token = nullptr;
    clToken.input = nullptr;
    bClError = true;
+
+   if (bClTextInputStorage && pObj) clObjMap.erase(pObj);
 }
 
 void
diff --git a/include/libeval/numeric_evaluator.h b/include/libeval/numeric_evaluator.h
index 274c22e14..77d577f4f 100644
--- a/include/libeval/numeric_evaluator.h
+++ b/include/libeval/numeric_evaluator.h
@@ -100,10 +100,10 @@ public:
    /* Initialization and destruction. init() is invoked be the constructor and should not be needed
     * by the user.
     * clear() should be invoked by the user if a new input string is to be processed. It will reset
-    * the parser. User defined variables are retained.
+    * the parser and clear the original expression value for a requested object (if pObj != null).
     */
    void init();
-   void clear();
+   void clear(const void* pObj = nullptr);
 
    /* Set the decimal separator for the input string. Defaults to '.' */
    void setDecimalSeparator(char sep);
-- 
2.13.3

From 3c739e1657ddec967ea9e8536eda30a80f603566 Mon Sep 17 00:00:00 2001
From: jean-pierre charras <jp.charras@xxxxxxxxxx>
Date: Tue, 27 Feb 2018 11:41:11 +0100
Subject: [PATCH 2/2] Fix SetValue() method in TEXT_CTRL_EVAL

Normal SetValue() call would temporarily change the displayed
value, but as soon as the text widget receives focus again, the original
expression (not evaluated) is restored.

To avoid this, the original expression is cleared in the associated
NumericEvaluator object.
---
 common/widgets/text_ctrl_eval.cpp | 13 ++++++++++---
 include/widgets/text_ctrl_eval.h  |  8 ++++++++
 2 files changed, 18 insertions(+), 3 deletions(-)

diff --git a/common/widgets/text_ctrl_eval.cpp b/common/widgets/text_ctrl_eval.cpp
index bf1dfc445..c5d630c24 100644
--- a/common/widgets/text_ctrl_eval.cpp
+++ b/common/widgets/text_ctrl_eval.cpp
@@ -38,12 +38,19 @@ TEXT_CTRL_EVAL::TEXT_CTRL_EVAL( wxWindow* aParent, wxWindowID aId, const
 }
 
 
+void TEXT_CTRL_EVAL::SetValue( const wxString& aValue )
+{
+    wxTextCtrl::SetValue( aValue );
+    m_eval.clear( this );
+}
+
+
 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 +76,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 a523ea96f..dfb64f447 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;
-- 
2.13.3

Attachment: signature.asc
Description: OpenPGP digital signature