reworked validFloat() and validInt() functions again

4 files changed, 67 insertions, 113 deletions

diff --git a/source/1.0/src/shflags b/source/1.0/src/shflags
index 371bf3e..7db9573 100644
--- a/source/1.0/src/shflags
+++ b/source/1.0/src/shflags
@@ -116,13 +116,13 @@ fi
 # can we use built-ins?
 ( echo "${FLAGS_TRUE#0}"; ) >/dev/null 2>&1
 if [ $? -eq ${FLAGS_TRUE} ]; then
+  __FLAGS_USE_BUILTIN=${FLAGS_TRUE}
   __FLAGS_FX_MATH='_flags_mathBuiltin'
   __FLAGS_FX_VALID_FLOAT='_flags_validFloatBuiltin'
-  __FLAGS_FX_VALID_INT='_flags_validIntBuiltin'
 else
-  __FLAGS_FX_MATH='_flags_mathExpr'
+  __FLAGS_USE_BUILTIN=${FLAGS_FALSE}
+  __FLAGS_FX_MATH=${FLAGS_EXPR_CMD}
   __FLAGS_FX_VALID_FLOAT='_flags_validFloatExpr'
-  __FLAGS_FX_VALID_INT='_flags_validIntExpr'
 fi
 
 #
@@ -317,7 +317,7 @@ _flags_define()
         ;;
 
       ${__FLAGS_TYPE_INTEGER})
-        if ${__FLAGS_FX_VALID_INT} "${_flags_default_}"; then
+        if _flags_validInt "${_flags_default_}"; then
           :
         else
           flags_error="invalid default flag value '${_flags_default_}'"
@@ -541,7 +541,6 @@ _flags_validBool()
 #
 # Args:
 #   _flags_float_: float: value to validate
-#   _flags_recursed_: boolean: running recursed
 # Returns:
 #   bool: true if the value is a valid integer
 _flags_validFloatBuiltin()
@@ -550,25 +549,18 @@ _flags_validFloatBuiltin()
   [ -n "$1" ] || return ${flags_return}
   _flags_float_=$1
 
-  _flags_curr_=${_flags_float_#-}  # strip leading negative sign
-
-  # strip leading and trailing integers until only the decimal point remains
-  _flags_last_=${_flags_curr_}
-  _flags_curr_=${_flags_curr_#[0-9]}
-  _flags_curr_=${_flags_curr_%[0-9]}
-  while [ \
-    "${_flags_curr_}" != '.' -a \
-    -n "${_flags_curr_}" -a \
-    "${_flags_curr_}" != "${_flags_last_}" \
-  ]; do
-    _flags_last_=${_flags_curr_}
-    _flags_curr_=${_flags_curr_#[0-9]}
-    _flags_curr_=${_flags_curr_%[0-9]}
-  done
-  [ "${_flags_curr_}" = '.' -o -z "${_flags_curr_}" ] && \
+  if _flags_validInt ${_flags_float_}; then
     flags_return=${FLAGS_TRUE}
+  else
+    _flags_float_whole_=${_flags_float_%.*}
+    _flags_float_fraction_=${_flags_float_#*.}
+    if _flags_validInt ${_flags_float_whole_:-0} -a \
+      _flags_validInt ${_flags_float_fraction_}; then
+      flags_return=${FLAGS_TRUE}
+    fi
+  fi
 
-  unset _flags_curr_ _flags_float_ _flags_last_
+  unset _flags_float_ _flags_float_whole_ _flags_float_fraction_
   return ${flags_return}
 }
 
@@ -580,9 +572,11 @@ _flags_validFloatBuiltin()
 #   bool: true if the value is a valid float
 _flags_validFloatExpr()
 {
+  flags_return=${FLAGS_FALSE}
+  [ -n "$1" ] || return ${flags_return}
   _flags_float_=$1
 
-  if ${__FLAGS_FX_VALID_INT} ${_flags_float_}; then
+  if _flags_validInt "${_flags_float_}"; then
     flags_return=${FLAGS_TRUE}
   else
     flags_return=${FLAGS_TRUE}
@@ -603,55 +597,35 @@ _flags_validFloatExpr()
   return ${flags_return}
 }
 
-# Validate an integer using built-ins.
+# Validate an integer.
 #
 # Args:
 #   _flags_int_: integer: value to validate
-#   _flags_recursed_: boolean: running recursed
 # Returns:
 #   bool: true if the value is a valid integer
-_flags_validIntBuiltin()
+_flags_validInt()
 {
   flags_return=${FLAGS_FALSE}
   [ -n "$1" ] || return ${flags_return}
   _flags_int_=$1
 
-  _flags_curr_=${_flags_int_#-}  # strip leading negative sign
-
-  _flags_last_=${_flags_curr_}
-  _flags_curr_=${_flags_curr_#[0-9]}
-  while [ -n "${_flags_curr_}" -a "${_flags_curr_}" != "${_flags_last_}" ]; do
-    _flags_last_=${_flags_curr_}
-    _flags_curr_=${_flags_curr_#[0-9]}
-  done
-  [ -z "${_flags_curr_}" ] && flags_return=${FLAGS_TRUE}
-
-  unset _flags_curr_ _flags_int_ _flags_last_
-  return ${flags_return}
-}
-
-# Validate an integer using expr.
-#
-# Args:
-#   _flags_int_: interger: value to validate
-# Returns:
-#   bool: true if the value is a valid integer
-_flags_validIntExpr()
-{
-  _flags_int_=$1
-
-  flags_return=${FLAGS_TRUE}
   case ${_flags_int_} in
-    -*)  # negative ints
-      _flags_test_=`${FLAGS_EXPR_CMD} -- "${_flags_int_}" : '\(-[0-9][0-9]*\)'`
-      ;;
-    *)  # positive ints
-      _flags_test_=`${FLAGS_EXPR_CMD} -- "${_flags_int_}" : '\([0-9][0-9]*\)'`
+    -*.*) ;;  # ignore negative floats (we'll invalidate them later)
+    -*)  # strip possible leading negative sign
+      if _flags_useBuiltin; then
+        _flags_int_=${_flags_int_#-}
+      else
+        _flags_int_=`${FLAGS_EXPR_CMD} -- "${_flags_int_}" : '-\([0-9][0-9]*\)'`
+      fi
       ;;
   esac
-  [ "${_flags_test_}" != "${_flags_int_}" ] && flags_return=${FLAGS_FALSE}
 
-  unset _flags_int_ _flags_test_
+  case "${_flags_int_}" in
+    *[!0-9]*) flags_return=${FLAGS_FALSE} ;;
+    *) flags_return=${FLAGS_TRUE} ;;
+  esac
+
+  unset _flags_int_
   return ${flags_return}
 }
 
@@ -842,7 +816,7 @@ _flags_parseGetopt()
         ;;
 
       ${__FLAGS_TYPE_INTEGER})
-        if ${__FLAGS_FX_VALID_INT} "${_flags_arg_}"; then
+        if _flags_validInt "${_flags_arg_}"; then
           eval "FLAGS_${_flags_usName_}='${_flags_arg_}'"
         else
           flags_error="invalid integer value (${_flags_arg_})"
@@ -897,21 +871,20 @@ _flags_mathBuiltin()
   eval echo ${_flags_expr_}
 }
 
-# Perform some path using expr.
+# Use built-in helper function to enable unit testing.
 #
 # Args:
-#   $@: string: math expression to evaluate
+#   None
 # Returns:
-#   string: the result
-_flags_mathExpr()
+#   bool: true if built-ins should be used
+_flags_useBuiltin()
 {
-  eval ${FLAGS_EXPR_CMD} $@
+  return ${__FLAGS_USE_BUILTIN}
 }
 
 #------------------------------------------------------------------------------
 # public functions
-#
-
+# 
 # A basic boolean flag. Boolean flags do not take any arguments, and their
 # value is either 1 (false) or 0 (true). For long flags, the false value is
 # specified on the command line by prepending the word 'no'. With short flags,
diff --git a/source/1.0/src/shflags_test_helpers b/source/1.0/src/shflags_test_helpers
index d1bcd3d..df9784c 100644
--- a/source/1.0/src/shflags_test_helpers
+++ b/source/1.0/src/shflags_test_helpers
@@ -28,9 +28,9 @@ TH_SHUNIT=${SHUNIT_INC:-../lib/shunit2}
 TH_BOOL_VALID='true t 0 false f 1'
 TH_BOOL_INVALID='123 123.0 invalid'
 TH_FLOAT_VALID='-1234.0 -1.0 -.123 0.0 0. .123 1.0 1234.0'
-TH_FLOAT_INVALID='true false 1.2.3 -1.2.3'
+TH_FLOAT_INVALID='true false 1.2.3 -1.2.3 ""'
 TH_INT_VALID='-1234 -1 0 1 1234'
-TH_INT_INVALID='true false -1.0 -.123 0.0 .123 1.0'
+TH_INT_INVALID='true false -1.0 -.123 0.0 .123 1.0 ""'
 
 #
 # test helper functions
diff --git a/source/1.0/src/shflags_test_parsing.sh b/source/1.0/src/shflags_test_parsing.sh
index f585e96..9f4ba05 100755
--- a/source/1.0/src/shflags_test_parsing.sh
+++ b/source/1.0/src/shflags_test_parsing.sh
@@ -12,6 +12,12 @@
 # TODO(kward): assert on FLAGS errors
 # TODO(kward): testNonStandardIFS()
 
+# exit immediately if a pipeline or subshell exits with a non-zero status.
+#set -e
+
+# treat unset variables as an error
+set -u
+
 # load test helpers
 . ./shflags_test_helpers
 
diff --git a/source/1.0/src/shflags_test_private.sh b/source/1.0/src/shflags_test_private.sh
index 7065940..38e9c32 100755
--- a/source/1.0/src/shflags_test_private.sh
+++ b/source/1.0/src/shflags_test_private.sh
@@ -23,11 +23,6 @@ testColumns()
   assertNotNull "unexpected screen width (${cols})" "${value}"
 }
 
-testExpr()
-{
-  :
-}
-
 testGenOptStr()
 {
   _testGenOptStr '' ''
@@ -132,13 +127,11 @@ _testValidFloat()
 
 testValidFloatBuiltin()
 {
-  # Are we running a shell that can handle a built-in version? The Solaris
-  # Bourne shell for one does not support what we need.
-  if [ "${__FLAGS_FX_VALID_FLOAT}" != '_flags_validFloatBuiltin' ]; then
-    echo 'SKIPPED: this shell does not support the necessary builtins'
-    return
+  if _flags_useBuiltin; then
+    _testValidFloat _flags_validFloatBuiltin
+  else
+    echo 'SKIPPED: this shell does not support the necessary built-ins'
   fi
-  _testValidFloat _flags_validFloatBuiltin
 }
 
 testValidFloatExpr()
@@ -148,62 +141,44 @@ testValidFloatExpr()
 
 _testValidInt()
 {
-  fx=$1
-
   # valid values
   for value in ${TH_INT_VALID}; do
-    ${fx} "${value}"
+    _flags_validInt "${value}"
     assertTrue "valid value (${value}) did not validate" $?
   done
 
   # invalid values
   for value in ${TH_INT_INVALID}; do
-    ${fx} "${value}"
+    _flags_validInt "${value}"
     assertFalse "invalid value (${value}) should not validate" $?
   done
 }
 
 testValidIntBuiltin()
 {
-  # Are we running a shell that can handle a built-in version? The Solaris
-  # Bourne shell for one does not support what we need.
-  if [ "${__FLAGS_FX_VALID_INT}" != '_flags_validIntBuiltin' ]; then
-    echo 'SKIPPED: this shell does not support the necessary builtins'
-    return
+  if _flags_useBuiltin; then
+    _testValidInt
+  else
+    echo 'SKIPPED: this shell does not support the necessary built-ins'
   fi
-  _testValidInt _flags_validIntBuiltin
 }
 
 testValidIntExpr()
 {
-  _testValidInt _flags_validIntExpr
-}
-
-_testMath()
-{
-  fx=$1
-
-  assertEquals 2 `${fx} 1 + 1`
-  assertEquals 2 `${fx} '1 + 1'`
-
-  assertNotEquals 3 `${fx} 1 + 1`
-  assertNotEquals 3 `${fx} '1 + 1'`
+  (
+    _flags_useBuiltin() { return ${FLAGS_FALSE}; }
+    _testValidInt
+  )
 }
 
 testMathBuiltin()
 {
-  # Are we running a shell that can handle a built-in version? The Solaris
-  # Bourne shell for one does not support what we need.
-  if [ "${__FLAGS_FX_VALID_INT}" != '_flags_validIntBuiltin' ]; then
-    echo 'SKIPPED: this shell does not support the necessary builtins'
-    return
+  if _flags_useBuiltin; then
+    assertEquals 2 `_flags_mathBuiltin 1 + 1`
+    assertEquals 2 `_flags_mathBuiltin '1 + 1'`
+  else
+    echo 'SKIPPED: this shell does not support the necessary built-ins'
   fi
-  _testMath _flags_mathBuiltin
-}
-
-testMathExpr()
-{
-  _testMath _flags_mathExpr
 }
 
 #------------------------------------------------------------------------------
@@ -215,7 +190,7 @@ oneTimeSetUp()
   th_oneTimeSetUp
 }
 
-setUp()
+tearDown()
 {
   flags_reset
 }