- fixed ksh and zsh version detection

- added alternate validFloat and validInt functions that use built-ins instead
  of expr
- fixed some minor issues with FreeBSD 9.1
- fixed unit tests for Solaris 8 and 9

7 files changed, 195 insertions, 65 deletions

diff --git a/source/1.0/doc/CHANGES-1.0.txt b/source/1.0/doc/CHANGES-1.0.txt
index 4644720..a259c2c 100644
--- a/source/1.0/doc/CHANGES-1.0.txt
+++ b/source/1.0/doc/CHANGES-1.0.txt
@@ -41,6 +41,9 @@ test to fail.
 
 Issue #22. Fixed broken testGetFlagInfo() test.
 
+Created alternate validFloat() and validInt() functions that use shell built-ins
+where possible to increase performance and reduce the usage of the expr command.
+
 
 Changes with 1.0.3
 ------------------
diff --git a/source/1.0/lib/versions b/source/1.0/lib/versions
index 20f2088..57fb8fa 100755
--- a/source/1.0/lib/versions
+++ b/source/1.0/lib/versions
@@ -42,6 +42,7 @@ versions_osName()
       fi
       ;;
   esac
+
   echo ${os_name_}
   unset os_name_ os_system_
 }
@@ -90,6 +91,7 @@ versions_osVersion()
       fi
       ;;
   esac
+
   echo ${os_version_}
   unset os_name_ os_release_ os_version_ major_ minor_ sub_
 }
@@ -148,22 +150,22 @@ versions_shell_ksh()
 {
   versions_shell_=$1
 
-  # see if --version gives a result
-  versions_version_=`${versions_shell_} --version 2>&1 \
+  # try a few different ways to figure out the version
+  versions_version_=`${versions_shell_} --version : 2>&1 \
     |sed 's/.*\([0-9][0-9][0-9][0-9]-[0-9][0-9]-[0-9][0-9]\).*/\1/'`
 
-  # --version didn't work... look into the binary
-  if [ $? -ne ${TRUE} ]; then 
+  if [ -z "${versions_version_}" ]; then
     _versions_have_strings
     versions_version_=`strings ${versions_shell_} 2>&1 \
       |grep Version \
       |sed 's/^.*Version \(.*\)$/\1/;s/ s+ \$$//;s/ /-/g'`
-    [ -z "${versions_version_}" ] \
-        && versions_version_=`versions_shell_pdksh ${versions_shell_}`
   fi
 
-  echo ${versions_version_}
+  if [ -z "${versions_version_}" ]; then
+    versions_version_=`versions_shell_pdksh ${versions_shell_}`
+  fi
 
+  echo ${versions_version_}
   unset versions_shell_ versions_version_
 }
 
@@ -179,14 +181,14 @@ versions_shell_zsh()
 {
   versions_shell_=$1
 
-  versions_version_=`${versions_shell_} --version 2>&1 |awk '{print $2}'`
+  # try a few different ways to figure out the version
+  versions_version_=`echo 'echo ${ZSH_VERSION}' |${versions_shell_}`
 
-  if [ $? -ne ${TRUE} ]; then
-    versions_version_=`echo 'echo ${ZSH_VERSION}' |${versions_shell_}`
+  if [ -z "${versions_version_}" ]; then
+    versions_version_=`${versions_shell_} --version 2>&1 |awk '{print $2}'`
   fi
 
   echo ${versions_version_}
-
   unset versions_shell_ versions_version_
 }
 
diff --git a/source/1.0/src/shflags b/source/1.0/src/shflags
index 4755b11..1955839 100644
--- a/source/1.0/src/shflags
+++ b/source/1.0/src/shflags
@@ -87,29 +87,22 @@
 [ -n "${FLAGS_VERSION:-}" ] && return 0
 FLAGS_VERSION='1.0.4pre'
 
-# Set some reasonable defaults.
+# return values that scripts can use
+FLAGS_TRUE=0
+FLAGS_FALSE=1
+FLAGS_ERROR=2
+
+# determine some reasonable command defaults
 __FLAGS_UNAME_S=`uname -s`
 case "${__FLAGS_UNAME_S}" in
   BSD) __FLAGS_EXPR_CMD='gexpr' ;;
   *) __FLAGS_EXPR_CMD='expr' ;;
 esac
 
-# A user can set the path to different commands by overriding these variables in
-# their script.
+# commands a user can override if needed
 FLAGS_EXPR_CMD=${FLAGS_EXPR_CMD:-${__FLAGS_EXPR_CMD}}
 FLAGS_GETOPT_CMD=${FLAGS_GETOPT_CMD:-getopt}
 
-# return values that scripts can use
-FLAGS_TRUE=0
-FLAGS_FALSE=1
-FLAGS_ERROR=2
-
-# logging functions
-_flags_debug() { echo "flags:DEBUG $@" >&2; }
-_flags_warn() { echo "flags:WARN $@" >&2; }
-_flags_error() { echo "flags:ERROR $@" >&2; }
-_flags_fatal() { echo "flags:FATAL $@" >&2; exit ${FLAGS_ERROR}; }
-
 # specific shell checks
 if [ -n "${ZSH_VERSION:-}" ]; then
   setopt |grep "^shwordsplit$" >/dev/null
@@ -122,6 +115,16 @@ if [ -n "${ZSH_VERSION:-}" ]; then
   fi
 fi
 
+# can we use built-ins?
+( echo "${FLAGS_TRUE#0}"; ) >/dev/null 2>&1
+if [ $? -eq ${FLAGS_TRUE} ]; then
+  __FLAGS_FX_VALID_FLOAT='_flags_validFloatBuiltin'
+  __FLAGS_FX_VALID_INT='_flags_validIntBuiltin'
+else
+  __FLAGS_FX_VALID_FLOAT='_flags_validFloatExpr'
+  __FLAGS_FX_VALID_INT='_flags_validIntExpr'
+fi
+
 #
 # constants
 #
@@ -209,6 +212,12 @@ __flags_opts=''  # temporary storage for parsed getopt flags
 # private functions
 #
 
+# logging functions
+_flags_debug() { echo "flags:DEBUG $@" >&2; }
+_flags_warn() { echo "flags:WARN $@" >&2; }
+_flags_error() { echo "flags:ERROR $@" >&2; }
+_flags_fatal() { echo "flags:FATAL $@" >&2; exit ${FLAGS_ERROR}; }
+
 # Define a flag.
 #
 # Calling this function will define the following info variables for the
@@ -287,7 +296,7 @@ _flags_define()
   if [ ${_flags_return_} -eq ${FLAGS_TRUE} ]; then
     case ${_flags_type_} in
       ${__FLAGS_TYPE_BOOLEAN})
-        if _flags_validateBoolean "${_flags_default_}"; then
+        if _flags_validBool "${_flags_default_}"; then
           case ${_flags_default_} in
             true|t|0) _flags_default_=${FLAGS_TRUE} ;;
             false|f|1) _flags_default_=${FLAGS_FALSE} ;;
@@ -299,7 +308,7 @@ _flags_define()
         ;;
 
       ${__FLAGS_TYPE_FLOAT})
-        if _flags_validateFloat "${_flags_default_}"; then
+        if ${__FLAGS_FX_VALID_FLOAT} "${_flags_default_}"; then
           :
         else
           flags_error="invalid default flag value '${_flags_default_}'"
@@ -308,7 +317,7 @@ _flags_define()
         ;;
 
       ${__FLAGS_TYPE_INTEGER})
-        if _flags_validateInteger "${_flags_default_}"; then
+        if ${__FLAGS_FX_VALID_INT} "${_flags_default_}"; then
           :
         else
           flags_error="invalid default flag value '${_flags_default_}'"
@@ -513,7 +522,7 @@ _flags_columns()
 #   _flags__bool: boolean: value to validate
 # Returns:
 #   bool: true if the value is a valid boolean
-_flags_validateBoolean()
+_flags_validBool()
 {
   _flags_bool_=$1
 
@@ -528,28 +537,63 @@ _flags_validateBoolean()
   return ${flags_return}
 }
 
+# Validate a float using built-ins.
+#
+# Args:
+#   _flags_float_: float: value to validate
+#   _flags_recursed_: boolean: running recursed
+# Returns:
+#   bool: true if the value is a valid integer
+_flags_validFloatBuiltin()
+{
+  flags_return=${FLAGS_FALSE}
+  [ -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_}" ] && \
+    flags_return=${FLAGS_TRUE}
+
+  unset _flags_curr_ _flags_float_ _flags_last_
+  return ${flags_return}
+}
+
 # Validate a float.
 #
 # Args:
 #   _flags__float: float: value to validate
 # Returns:
 #   bool: true if the value is a valid float
-_flags_validateFloat()
+_flags_validFloatExpr()
 {
   _flags_float_=$1
 
-  if _flags_validateInteger ${_flags_float_}; then
+  if ${__FLAGS_FX_VALID_INT} ${_flags_float_}; then
     flags_return=${FLAGS_TRUE}
   else
     flags_return=${FLAGS_TRUE}
     case ${_flags_float_} in
       -*)  # negative floats
         _flags_test_=`${FLAGS_EXPR_CMD} -- "${_flags_float_}" :\
-            '\(-[0-9][0-9]*\.[0-9][0-9]*\)'`
+            '\(-[0-9]*\.[0-9]*\)'`
         ;;
       *)  # positive floats
         _flags_test_=`${FLAGS_EXPR_CMD} -- "${_flags_float_}" :\
-            '\([0-9][0-9]*\.[0-9][0-9]*\)'`
+            '\([0-9]*\.[0-9]*\)'`
         ;;
     esac
     [ "${_flags_test_}" != "${_flags_float_}" ] && flags_return=${FLAGS_FALSE}
@@ -559,13 +603,40 @@ _flags_validateFloat()
   return ${flags_return}
 }
 
-# Validate an integer.
+# Validate an integer using built-ins.
+#
+# 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_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__integer: interger: value to validate
+#   _flags_int_: interger: value to validate
 # Returns:
 #   bool: true if the value is a valid integer
-_flags_validateInteger()
+_flags_validIntExpr()
 {
   _flags_int_=$1
 
@@ -761,7 +832,7 @@ _flags_parseGetopt()
         ;;
 
       ${__FLAGS_TYPE_FLOAT})
-        if _flags_validateFloat "${_flags_arg_}"; then
+        if ${__FLAGS_FX_VALID_FLOAT} "${_flags_arg_}"; then
           eval "FLAGS_${_flags_usName_}='${_flags_arg_}'"
         else
           flags_error="invalid float value (${_flags_arg_})"
@@ -771,7 +842,7 @@ _flags_parseGetopt()
         ;;
 
       ${__FLAGS_TYPE_INTEGER})
-        if _flags_validateInteger "${_flags_arg_}"; then
+        if ${__FLAGS_FX_VALID_INT} "${_flags_arg_}"; then
           eval "FLAGS_${_flags_usName_}='${_flags_arg_}'"
         else
           flags_error="invalid integer value (${_flags_arg_})"
diff --git a/source/1.0/src/shflags_test_helpers b/source/1.0/src/shflags_test_helpers
index bbdd5a8..e78beac 100644
--- a/source/1.0/src/shflags_test_helpers
+++ b/source/1.0/src/shflags_test_helpers
@@ -26,11 +26,11 @@ TH_SHFLAGS=${SHFLAGS_INC:-./shflags}
 TH_SHUNIT=${SHUNIT_INC:-../lib/shunit2}
 
 TH_BOOL_VALID='true t 0 false f 1'
-TH_BOOL_INVALID='123 123.0 blah'
-TH_FLOAT_VALID='-1234.0 -1234 -1.0 -1 0.0 0 1.0 1 1234.0 1234'
-TH_FLOAT_INVALID='true false invalid 1.2.3 -1.2.3 0. -.5'
+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_INT_VALID='-1234 -1 0 1 1234'
-TH_INT_INVALID='-1.0 0.0 1.0 invalid'
+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 e93a62f..6cc748d 100755
--- a/source/1.0/src/shflags_test_parsing.sh
+++ b/source/1.0/src/shflags_test_parsing.sh
@@ -45,7 +45,7 @@ testGetoptEnhanced()
   assertFalse "parsed invalid flag 'xyz'" $?
 }
 
-testValidBooleanShort()
+testValidBoolsShort()
 {
   # flip flag to true
   FLAGS -b >"${stdoutF}" 2>"${stderrF}"
@@ -68,11 +68,12 @@ testValidBooleanShort()
   th_showOutput $? "${stdoutF}" "${stderrF}"
 }
 
-testValidBooleanLong()
+# TODO(kate): separate into multiple functions to reflect correct usage
+testValidBoolsLong()
 {
   flags_getoptIsEnh || startSkipping
 
-  # note: the default value of bool is 'false'
+  # Note: the default value of bool is 'false'.
 
   # leave flag false
   FLAGS --nobool >"${stdoutF}" 2>"${stderrF}"
@@ -131,10 +132,11 @@ _testInvalidFloats()
 {
   flag=$1
   for value in ${TH_FLOAT_INVALID}; do
-    FLAGS ${flag} ${value} >"${stdoutF}" 2>"${stderrF}"
+    ( FLAGS ${flag} ${value} >"${stdoutF}" 2>"${stderrF}"; )
     rtrn=$?
     assertFalse "FLAGS (${value}) returned a zero result" ${rtrn}
-    assertTrue 'expected no output to STDERR' "[ -s '${stderrF}' ]"
+    assertFalse 'expected no output to STDOUT' "[ -s '${stdoutF}' ]"
+    assertTrue 'expected output to STDERR' "[ -s '${stderrF}' ]"
   done
 }
 
@@ -169,10 +171,11 @@ _testInvalidIntegers()
 {
   flag=$1
   for value in ${TH_INT_INVALID}; do
-    FLAGS ${flag} ${value} >"${stdoutF}" 2>"${stderrF}"
+    ( FLAGS ${flag} ${value} >"${stdoutF}" 2>"${stderrF}"; )
     rtrn=$?
     assertFalse "invalid integer (${value}) test returned success." ${rtrn}
-    assertTrue 'expected no output to STDERR' "[ -s '${stderrF}' ]"
+    assertFalse 'expected no output to STDOUT' "[ -s '${stdoutF}' ]"
+    assertTrue 'expected output to STDERR' "[ -s '${stderrF}' ]"
   done
 }
 
@@ -328,13 +331,17 @@ oneTimeSetUp()
 
 setUp()
 {
-  flags_reset
   DEFINE_boolean bool false 'boolean test' 'b'
   DEFINE_float float 0.0 'float test' 'f'
   DEFINE_integer int 0 'integer test' 'i'
   DEFINE_string str '' 'string test' 's'
 }
 
+tearDown()
+{
+  flags_reset
+}
+
 # load and run shUnit2
 [ -n "${ZSH_VERSION:-}" ] && SHUNIT_PARENT=$0
 . ${TH_SHUNIT}
diff --git a/source/1.0/src/shflags_test_private.sh b/source/1.0/src/shflags_test_private.sh
index 4f7ccaf..f353bb3 100755
--- a/source/1.0/src/shflags_test_private.sh
+++ b/source/1.0/src/shflags_test_private.sh
@@ -23,6 +23,11 @@ testColumns()
   assertNotNull "unexpected screen width (${cols})" "${value}"
 }
 
+testExpr()
+{
+  :
+}
+
 testGenOptStr()
 {
   _testGenOptStr '' ''
@@ -93,51 +98,87 @@ testItemInList()
   assertFalse 'empty lists should not match' $?
 }
 
-testValidateBoolean()
+testValidBool()
 {
   # valid values
   for value in ${TH_BOOL_VALID}; do
-    _flags_validateBoolean "${value}"
+    _flags_validBool "${value}"
     assertTrue "valid value (${value}) did not validate" $?
   done
 
   # invalid values
   for value in ${TH_BOOL_INVALID}; do
-    _flags_validateBoolean "${value}"
+    _flags_validBool "${value}"
     assertFalse "invalid value (${value}) validated" $?
   done
 }
 
-testValidateFloat()
+_testValidFloat()
 {
+  fx=$1
+
   # valid values
   for value in ${TH_INT_VALID} ${TH_FLOAT_VALID}; do
-    _flags_validateFloat "${value}"
+    ${fx} "${value}"
     assertTrue "valid value (${value}) did not validate" $?
   done
 
   # invalid values
   for value in ${TH_FLOAT_INVALID}; do
-    _flags_validateFloat "${value}"
+    ${fx} "${value}"
     assertFalse "invalid value (${value}) validated" $?
   done
 }
 
-testValidateInteger()
+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
+  fi
+  _testValidFloat _flags_validFloatBuiltin
+}
+
+testValidFloatExpr()
+{
+  _testValidFloat _flags_validFloatExpr
+}
+
+_testValidInt()
 {
+  fx=$1
+
   # valid values
   for value in ${TH_INT_VALID}; do
-    _flags_validateInteger "${value}"
+    ${fx} "${value}"
     assertTrue "valid value (${value}) did not validate" $?
   done
 
   # invalid values
   for value in ${TH_INT_INVALID}; do
-    _flags_validateInteger "${value}"
-    assertFalse "invalid value (${value}) validated" $?
+    ${fx} "${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
+  fi
+  _testValidInt _flags_validIntBuiltin
+}
+
+testValidIntExpr()
+{
+  _testValidInt _flags_validIntExpr
+}
+
 #------------------------------------------------------------------------------
 # suite functions
 #
diff --git a/source/1.0/src/shflags_test_public.sh b/source/1.0/src/shflags_test_public.sh
index 0da9d02..adffb74 100755
--- a/source/1.0/src/shflags_test_public.sh
+++ b/source/1.0/src/shflags_test_public.sh
@@ -99,16 +99,17 @@ flags:
   -h  show this help (default: false)
 EOF
   (
-    _flags_columns() { mock_flags_columns "$@"; }
+    _flags_columns() { mock_flags_columns "\$@"; }
     FLAGS_HELP=${help};
     FLAGS -h >"${stdoutF}" 2>"${stderrF}"
   )
   r3turn=$?
   assertTrue 'a call for help should not return an error' ${r3turn}
+
   diff "${expectedF}" "${stderrF}" >/dev/null
-  differed=$?
-  assertTrue 'unexpected help output' ${differed}
-  th_showOutput ${differed} "${stdoutF}" "${stderrF}"
+  r3turn=$?
+  assertTrue 'unexpected help output' ${r3turn}
+  th_showOutput ${r3turn} "${stdoutF}" "${stderrF}"
 }
 
 testEnhancedHelpOutput()
@@ -137,9 +138,14 @@ flags:
     (default: 'this_is_a_long_default_value_to_force_alternate_indentation')
   -h,--help:  show this help (default: false)
 EOF
-  ( FLAGS_HELP=${help}; FLAGS -h >"${stdoutF}" 2>"${stderrF}" )
+  (
+    _flags_columns() { mock_flags_columns "\$@"; }
+    FLAGS_HELP=${help};
+    FLAGS -h >"${stdoutF}" 2>"${stderrF}"
+  )
   r3turn=$?
   assertTrue 'a call for help should not return an error' ${r3turn}
+
   diff "${expectedF}" "${stderrF}" >/dev/null
   differed=$?
   assertTrue 'unexpected help output' ${differed}