consolidated validFloat() back to single function; math() fixes

2 files changed, 53 insertions, 68 deletions

diff --git a/source/1.0/src/shflags b/source/1.0/src/shflags
index 883c41c..06a2ed0 100644
--- a/source/1.0/src/shflags
+++ b/source/1.0/src/shflags
@@ -118,10 +118,8 @@ fi
 ( echo "${FLAGS_TRUE#0}"; ) >/dev/null 2>&1
 if [ $? -eq ${FLAGS_TRUE} ]; then
   __FLAGS_USE_BUILTIN=${FLAGS_TRUE}
-  __FLAGS_FX_VALID_FLOAT='_flags_validFloatBuiltin'
 else
   __FLAGS_USE_BUILTIN=${FLAGS_FALSE}
-  __FLAGS_FX_VALID_FLOAT='_flags_validFloatExpr'
 fi
 
 #
@@ -307,7 +305,7 @@ _flags_define()
         ;;
 
       ${__FLAGS_TYPE_FLOAT})
-        if ${__FLAGS_FX_VALID_FLOAT} "${_flags_default_}"; then
+        if _flags_validFloat "${_flags_default_}"; then
           :
         else
           flags_error="invalid default flag value '${_flags_default_}'"
@@ -536,13 +534,13 @@ _flags_validBool()
   return ${flags_return}
 }
 
-# Validate a float using built-ins.
+# Validate a float.
 #
 # Args:
 #   _flags_float_: float: value to validate
 # Returns:
 #   bool: true if the value is a valid integer
-_flags_validFloatBuiltin()
+_flags_validFloat()
 {
   flags_return=${FLAGS_FALSE}
   [ -n "$1" ] || return ${flags_return}
@@ -550,33 +548,14 @@ _flags_validFloatBuiltin()
 
   if _flags_validInt ${_flags_float_}; then
     flags_return=${FLAGS_TRUE}
-  else
+  elif _flags_useBuiltin; then
     _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_float_ _flags_float_whole_ _flags_float_fraction_
-  return ${flags_return}
-}
-
-# Validate a float.
-#
-# Args:
-#   _flags__float: float: value to validate
-# Returns:
-#   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_validInt "${_flags_float_}"; then
-    flags_return=${FLAGS_TRUE}
+    unset _flags_float_whole_ _flags_float_fraction_
   else
     flags_return=${FLAGS_TRUE}
     case ${_flags_float_} in
@@ -590,9 +569,10 @@ _flags_validFloatExpr()
         ;;
     esac
     [ "${_flags_test_}" != "${_flags_float_}" ] && flags_return=${FLAGS_FALSE}
+    unset _flags_test_
   fi
 
-  unset _flags_float_ _flags_test_
+  unset _flags_float_ _flags_float_whole_ _flags_float_fraction_
   return ${flags_return}
 }
 
@@ -729,7 +709,7 @@ _flags_parseGetopt()
   # properly give user access to non-flag arguments mixed in between flag
   # arguments. Its usage was replaced by FLAGS_ARGV, and it is being kept only
   # for backwards compatibility reasons.
-  FLAGS_ARGC=`${_flags_math} $# - 1 - ${_flags_argc_}`
+  FLAGS_ARGC=`_flags_math $# - 1 - ${_flags_argc_}`
 
   # handle options. note options with values must do an additional shift
   while true; do
@@ -817,7 +797,7 @@ _flags_parseGetopt()
         ;;
 
       ${__FLAGS_TYPE_FLOAT})
-        if ${__FLAGS_FX_VALID_FLOAT} "${_flags_arg_}"; then
+        if _flags_validFloat "${_flags_arg_}"; then
           eval "FLAGS_${_flags_usName_}='${_flags_arg_}'"
         else
           flags_error="invalid float value (${_flags_arg_})"
@@ -878,17 +858,20 @@ _flags_parseGetopt()
 #   bool: success of math evaluation
 _flags_math()
 {
-  if _flags_useBuiltin; then
-    eval expr $@
-  else
+  if [ $# -eq 0 ]; then
+    flags_return=${FLAGS_FALSE}
+  elif _flags_useBuiltin; then
     # Variable assignment is needed as workaround for Solaris Bourne shell,
     # which cannot parse a bare $((expression)).
     _flags_expr_='$(($@))'
     eval echo ${_flags_expr_}
+    flags_return=$?
+    unset _flags_expr_
+  else
+    eval expr $@
+    flags_return=$?
   fi
-  flags_return=$?
 
-  unset _flags_expr_
   return ${flags_return}
 }
 
@@ -906,14 +889,12 @@ _flags_strlen()
 
   if [ -z "${_flags_str_}" ]; then
     flags_output=0
-    flags_return=${FLAGS_TRUE}
   elif _flags_useBuiltin; then
     flags_output=${#_flags_str_}
-    flags_return=$?
   else
     flags_output=`${FLAGS_EXPR_CMD} -- "${_flags_str_}" : '.*'`
-    flags_return=$?
   fi
+  flags_return=$?
 
   unset _flags_str_
   echo ${flags_output}
diff --git a/source/1.0/src/shflags_test_private.sh b/source/1.0/src/shflags_test_private.sh
index 0d36ad3..899f5df 100755
--- a/source/1.0/src/shflags_test_private.sh
+++ b/source/1.0/src/shflags_test_private.sh
@@ -110,33 +110,31 @@ testValidBool()
 
 _testValidFloat()
 {
-  fx=$1
-
   # valid values
   for value in ${TH_INT_VALID} ${TH_FLOAT_VALID}; do
-    ${fx} "${value}"
+    _flags_validFloat "${value}"
     assertTrue "valid value (${value}) did not validate" $?
   done
 
   # invalid values
   for value in ${TH_FLOAT_INVALID}; do
-    ${fx} "${value}"
+    _flags_validFloat "${value}"
     assertFalse "invalid value (${value}) validated" $?
   done
 }
 
 testValidFloatBuiltin()
 {
-  if _flags_useBuiltin; then
-    _testValidFloat _flags_validFloatBuiltin
-  else
-    echo 'SKIPPED: this shell does not support the necessary built-ins'
-  fi
+  _flags_useBuiltin || startSkipping
+  _testValidFloat
 }
 
 testValidFloatExpr()
 {
-  _testValidFloat _flags_validFloatExpr
+  (
+    _flags_useBuiltin() { return ${FLAGS_FALSE}; }
+    _testValidFloat
+  )
 }
 
 _testValidInt()
@@ -156,11 +154,8 @@ _testValidInt()
 
 testValidIntBuiltin()
 {
-  if _flags_useBuiltin; then
-    _testValidInt
-  else
-    echo 'SKIPPED: this shell does not support the necessary built-ins'
-  fi
+  _flags_useBuiltin || startSkipping
+  _testValidInt
 }
 
 testValidIntExpr()
@@ -173,17 +168,26 @@ testValidIntExpr()
 
 _testMath()
 {
-  assertEquals 2 `_flags_math 1 + 1`
-  assertEquals 2 `_flags_math '1 + 1'`
+  result=`_flags_math 1 + 1`
+  assertTrue '1+1 failed' $?
+  assertEquals '1+1' 2 ${result}
+
+  result=`_flags_math '1 + 2'`
+  assertTrue '1+2 failed' $?
+  assertEquals '1+2' 3 ${result}
+
+  result=`_flags_math 1 + 2 + 3`
+  assertTrue '1+2+3 failed' $?
+  assertEquals '1+2+3' 6 ${result}
+
+  result=`_flags_math`
+  assertFalse 'missing math succeeded' $?
 }
 
 testMathBuiltin()
 {
-  if _flags_useBuiltin; then
-    _testMath
-  else
-    echo 'SKIPPED: this shell does not support the necessary built-ins'
-  fi
+  _flags_useBuiltin || startSkipping
+  _testMath
 }
 
 testMathExpr()
@@ -197,29 +201,26 @@ testMathExpr()
 _testStrlen()
 {
   len=`_flags_strlen`
-  assertTrue 'missing argument unsuccessful' $?
+  assertTrue 'missing argument failed' $?
   assertEquals 'missing argument' 0 ${len}
 
   len=`_flags_strlen ''`
-  assertTrue 'empty argument unsuccessful' $?
+  assertTrue 'empty argument failed' $?
   assertEquals 'empty argument' 0 ${len}
 
   len=`_flags_strlen abc123`
-  assertTrue 'single-word unsuccessful' $?
+  assertTrue 'single-word failed' $?
   assertEquals 'single-word' 6 ${len}
 
   len=`_flags_strlen 'This is a test'`
-  assertTrue 'multi-word unsuccessful' $?
+  assertTrue 'multi-word failed' $?
   assertEquals 'multi-word' 14 ${len}
 }
 
 testStrlenBuiltin()
 {
-  if _flags_useBuiltin; then
-    _testStrlen
-  else
-    echo 'SKIPPED: this shell does not support the necessary built-ins'
-  fi
+  _flags_useBuiltin || startSkipping
+  _testStrlen
 }
 
 testStrlenExpr()
@@ -237,6 +238,9 @@ testStrlenExpr()
 oneTimeSetUp()
 {
   th_oneTimeSetUp
+
+  _flags_useBuiltin || \
+    th_warn 'Shell built-ins not supported. Some tests will be skipped.'
 }
 
 tearDown()