cheng/wxWidgets
1
0
Fork 0
Code Issues Pull Requests Packages Projects Releases Wiki Activity

Fix confusion around '-' handling in numeric validators code

Browse Source 
Actual implementation of IsCharOk() didn't correspond to the comments in
it or near the function declaration which stated that it's never called
with ch='-' as argument -- it was called with it and called IsMinusOk()
right before a comment saying that it doesn't need to do it, which was
very confusing.

Fix this by making the behaviour really correspond to the comments and
handling '-' at the base class level. This required introducing a new
pure virtual CanBeNegative() function, but it's going to be useful for
other things later too.

Still keep IsMinusOk() helper, but make it private now because it
doesn't need to be called from the derived class IsCharOk() any longer.
This commit is contained in:
Vadim Zeitlin 2021-02-21 15:09:03 +01:00
parent 709b259bbb
commit 7371131f1e
2 changed files with 33 additions and 25 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

21
include/wx/valnum.h
View File

@ -79,11 +79,6 @@ protected:
// bits of our style to the corresponding wxNumberFormatter::Style values.
int GetFormatFlags() const;
// Return true if pressing a '-' key is acceptable for the current control
// contents and insertion point. This is meant to be called from the
// derived class IsCharOk() implementation.
bool IsMinusOk(const wxString& val, int pos) const;
// Return the string which would result from inserting the given character
// at the specified position.
wxString GetValueAfterInsertingChar(wxString val, int pos, wxChar ch) const
@ -92,6 +87,11 @@ protected:
return val;
}
// Return true if this control allows negative numbers in it.
//
// If it doesn't, we don't allow entering "-" at all.
virtual bool CanBeNegative() const = 0;
private:
// Check whether the specified character can be inserted in the control at
// the given position in the string representing the current controls
@ -114,6 +114,11 @@ private:
// Determine the current insertion point and text in the associated control.
void GetCurrentValueAndInsertionPoint(wxString& val, int& pos) const;
// Return true if pressing a '-' key is acceptable for the current control
// contents and insertion point. This is used by OnChar() to handle '-' and
// relies on CanBeNegative() implementation in the derived class.
bool IsMinusOk(const wxString& val, int pos) const;
// Combination of wxVAL_NUM_XXX values.
int m_style;
@ -346,6 +351,9 @@ public:
virtual wxObject *Clone() const wxOVERRIDE { return new wxIntegerValidator(*this); }
protected:
virtual bool CanBeNegative() const wxOVERRIDE { return DoGetMin() < 0; }
private:
wxDECLARE_NO_ASSIGN_CLASS(wxIntegerValidator);
};
@ -458,6 +466,9 @@ public:
return new wxFloatingPointValidator(*this);
}
protected:
virtual bool CanBeNegative() const wxOVERRIDE { return DoGetMin() < 0; }
private:
typedef typename Base::LongestValueType LongestValueType;

37
src/common/valnum.cpp
View File

@ -117,6 +117,10 @@ wxNumValidatorBase::GetCurrentValueAndInsertionPoint(wxString& val,
bool wxNumValidatorBase::IsMinusOk(const wxString& val, int pos) const
{
// We need to know if we accept negative numbers at all.
if ( !CanBeNegative() )
return false;
// Minus is only ever accepted in the beginning of the string.
if ( pos != 0 )
return false;
@ -125,6 +129,15 @@ bool wxNumValidatorBase::IsMinusOk(const wxString& val, int pos) const
if ( !val.empty() && val[0] == '-' )
return false;
// Notice that entering '-' can make our value invalid, for example if
// we're limited to -5..15 range and the current value is 12, then the
// new value would be (invalid) -12. We consider it better to let the
// user do this because perhaps he is going to press Delete key next to
// make it -2 and forcing him to delete 1 first would be unnatural.
//
// TODO: It would be nice to indicate that the current control contents
// is invalid (if it's indeed going to be the case) once
// wxValidator supports doing this non-intrusively.
return true;
}
@ -165,7 +178,10 @@ void wxNumValidatorBase::OnChar(wxKeyEvent& event)
int pos;
GetCurrentValueAndInsertionPoint(val, pos);
if ( !IsCharOk(val, pos, ch) )
// Minus is a special case because we can deal with it directly here, for
// all the rest call the derived class virtual function.
const bool ok = ch == '-' ? IsMinusOk(val, pos) : IsCharOk(val, pos, ch);
if ( !ok )
{
if ( !wxValidator::IsSilent() )
wxBell();
@ -226,21 +242,6 @@ wxIntegerValidatorBase::FromString(const wxString& s, LongestValueType *value)
bool
wxIntegerValidatorBase::IsCharOk(const wxString& val, int pos, wxChar ch) const
{
// We may accept minus sign if we can represent negative numbers at all.
if ( ch == '-' )
{
// Notice that entering '-' can make our value invalid, for example if
// we're limited to -5..15 range and the current value is 12, then the
// new value would be (invalid) -12. We consider it better to let the
// user do this because perhaps he is going to press Delete key next to
// make it -2 and forcing him to delete 1 first would be unnatural.
//
// TODO: It would be nice to indicate that the current control contents
// is invalid (if it's indeed going to be the case) once
// wxValidator supports doing this non-intrusively.
return m_min < 0 && IsMinusOk(val, pos);
}
// We only accept digits here (remember that '-' is taken care of by the
// base class already).
if ( ch < '0' || ch > '9' )
@ -292,10 +293,6 @@ wxFloatingPointValidatorBase::IsCharOk(const wxString& val,
int pos,
wxChar ch) const
{
// We may accept minus sign if we can represent negative numbers at all.
if ( ch == '-' )
return m_min < 0 && IsMinusOk(val, pos);
const wxChar separator = wxNumberFormatter::GetDecimalSeparator();
if ( ch == separator )
{