Re: [lmi] Difficulty editing particular fields in census manager

[Vadim Zeitlin]

On Wed, 29 Aug 2018 21:09:43 +0000 Greg Chicares <address@hidden> wrote:

GC> To reproduce:
GC>   File | New | Census
GC>   Census | Add cell
GC>   Census | Edit cell...
GC>     "Riders" tab, "Spouse rider" group: check "Elected"
GC>     OK
GC> Double-click the "Spouse Rider Amount" column that now appears in the
GC> census manager, and press the "2" key (or any other digit). (Use HEAD
GC> for this, because it was only earlier today that the default "sample"
GC> products acquired this rider.)
GC> 
GC> Symptoms: The default "10000" amount is highlighted. Attempting to
GC> type any digit elicits only a beep, and does not alter the contents
GC> of the edit control embedded in the dataview.

 Thanks for the as always detailed instructions, I can reproduce the
problem perfectly well following them.

 I also recognized the problem even before reproducing it because it is an
old issue in wxWidgets numeric validator classes, reported as
http://trac.wxwidgets.org/ticket/12968 already 8 years ago... The
discussion there is unfortunately inconclusive, but the issue is very real
and I had hoped to fix it for quite some time. Maybe the time to do it has
finally come -- and I'll try to actually do it next.

GC> Discussion: Apparently this occurs for scalar trammelled numeric
GC> fields with positive limits. (There aren't many of those, because
GC> so many numeric fields are input sequences, whose elements are not
GC> constrained by lmi's MVC framework.) The symptoms are suppressed
GC> by this undesirable exploratory patch:
GC> 
GC> 
---------8<--------8<--------8<--------8<--------8<--------8<--------8<-------
GC> diff --git a/census_view.cpp b/census_view.cpp
GC> index f410bd07..3b701149 100644
GC> --- a/census_view.cpp
GC> +++ b/census_view.cpp
GC> @@ -870,8 +870,8 @@ bool CensusViewDataViewModel::SetValueByRow
GC>  
GC>      cell = new_val;
GC>  
GC> -    Input& model = view_.cell_parms()[row];
GC> -    model.Reconcile();
GC> +//  Input& model = view_.cell_parms()[row];
GC> +//  model.Reconcile();
GC>  
GC>      view_.document().Modify(true);
GC>  
GC> 
--------->8-------->8-------->8-------->8-------->8-------->8-------->8-------

 I admit I'm puzzled by the fact that this patch fixes the problem. But
perhaps it's not worth digging further into this because what also fixes
the problem for me, even though it can't be committed neither, is this
patch:

---------------------------------- >8 --------------------------------------
diff --git a/src/common/valnum.cpp b/src/common/valnum.cpp
index c2522d1075..abad6d3849 100644
--- a/src/common/valnum.cpp
+++ b/src/common/valnum.cpp
@@ -166,6 +166,7 @@ void wxNumValidatorBase::OnChar(wxKeyEvent& event)
     int pos;
     GetCurrentValueAndInsertionPoint(val, pos);
 
+#ifdef FIXME_VZ
     if ( !IsCharOk(val, pos, ch) )
     {
         if ( !wxValidator::IsSilent() )
@@ -174,6 +175,7 @@ void wxNumValidatorBase::OnChar(wxKeyEvent& event)
         // Do not skip the event in this case, stop handling it here.
         event.Skip(false);
     }
+#endif // FIXME_VZ
 }
 
 void wxNumValidatorBase::OnKillFocus(wxFocusEvent& event)
---------------------------------- >8 --------------------------------------

which disables key checks in the validator. With it, even if I don't do
anything else in lmi, the editor works tolerably, i.e. it allows to change
the value as long as the final value is in the correct range. This is still
not optimal because it just silently discards it instead of giving an error
if it isn't, but dealing with this is another subject...

GC> Vadim, can you translate my impressionistic description to the
GC> dataviewctrl API and suggest a way to fix this?

 Unfortunately I don't think it's possible to fix it without stopping to
use wxFloatingPointValidator completely, which would be undesirable. I can
fix it inside this class itself, but I'm not yet sure how exactly to do it,
so I can't say how long will it take.

 If a fix is needed urgently, perhaps the least bad solution would be to
set the lower end of the valid range to 0 when it's strictly positive. This
wouldn't actually be a big problem for lmi, as invalid values still
wouldn't be accepted, albeit still without any error message, because of a
later call to Reconcile() and while doing it at wxFloatingPointValidator
level remains the best solution, perhaps you could use this patch

---------------------------------- >8 --------------------------------------
diff --git a/census_view.cpp b/census_view.cpp
index f410bd073..b05ec2843 100644
--- a/census_view.cpp
+++ b/census_view.cpp
@@ -291,7 +291,7 @@ class DoubleRangeRenderer
     )
 {
     wxFloatingPointValidator<double> val;
-    val.SetRange(data.min, data.max);
+    val.SetRange(std::min(data.min, 0), data.max);
 
     wxTextCtrl* ctrl = new(wx) wxTextCtrl
         (parent
---------------------------------- >8 --------------------------------------

for now?

 Regards,
VZ