fixed issues 11 and 12; added ability to override OS getopt

4 files changed, 95 insertions, 63 deletions

diff --git a/source/1.0/doc/CHANGES-1.0.txt b/source/1.0/doc/CHANGES-1.0.txt
index 60e8040..0016378 100644
--- a/source/1.0/doc/CHANGES-1.0.txt
+++ b/source/1.0/doc/CHANGES-1.0.txt
@@ -4,12 +4,24 @@ Changes in shFlags 1.0.x
 Changes with 1.0.4
 ------------------
 
-Fixed issue #10. Usage of ``expn`` under FreeBSD 7.2 (FreeNAS 0.7.1) and FreeBSD
+Fixed issue #10. Usage of ``expr`` under FreeBSD 7.2 (FreeNAS 0.7.1) and FreeBSD
 8.0 that was causing many unit tests to fail.
 
 Fixed issue where booleans were sometimes mis-configured to require additional
 values like other flags.
 
+Changed _flags_fatal() to exit with FLAGS_ERROR immediately.
+
+Fixed issue #11. When help is requested, the help flag is no longer prefixed
+with [no].
+
+Upgraded shUnit2 to 2.1.6.
+
+Fixed issue #12. Requesting help shouldn't be considered an error.
+
+Added the ability to override the use of the OS default 'getopt' command by
+defining the FLAGS_GETOPT_CMD variable.
+
 
 Changes with 1.0.3
 ------------------
diff --git a/source/1.0/src/shflags b/source/1.0/src/shflags
index 0d0f9de..1baef3e 100644
--- a/source/1.0/src/shflags
+++ b/source/1.0/src/shflags
@@ -50,8 +50,15 @@
 #   $ ./hello.sh -n Kate
 #   Hello, Kate.
 #
-# NOTE: Not all systems include a getopt version that supports long flags. On
-# these systems, only short flags are recognized.
+# CUSTOMIZABLE BEHAVIOR:
+#
+# A script can override the default 'getopt' command by providing the path to
+# an alternate implementation by defining the FLAGS_GETOPT_CMD variable.
+#
+# NOTES:
+#
+# * Not all systems include a getopt version that supports long flags. On these
+#   systems, only short flags are recognized.
 
 #==============================================================================
 # shFlags
@@ -78,32 +85,33 @@
 
 # return if FLAGS already loaded
 [ -n "${FLAGS_VERSION:-}" ] && return 0
+
 FLAGS_VERSION='1.0.4pre'
 
-# return values
+# a user can set the path to a different getopt command by overriding this
+# variable in their script
+FLAGS_GETOPT_CMD=${FLAGS_GETOPT_CMD:-getopt}
+
+# return values that scripts can use
 FLAGS_TRUE=0
 FLAGS_FALSE=1
 FLAGS_ERROR=2
 
-# reserved flag names
-FLAGS_RESERVED_LIST=' ARGC ARGV ERROR FALSE HELP PARENT RESERVED TRUE VERSION '
-
+# 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; }
+_flags_fatal() { echo "flags:FATAL $@" >&2; exit ${FLAGS_ERROR}; }
 
 # specific shell checks
 if [ -n "${ZSH_VERSION:-}" ]; then
   setopt |grep "^shwordsplit$" >/dev/null
   if [ $? -ne ${FLAGS_TRUE} ]; then
     _flags_fatal 'zsh shwordsplit option is required for proper zsh operation'
-    exit ${FLAGS_ERROR}
   fi
   if [ -z "${FLAGS_PARENT:-}" ]; then
     _flags_fatal "zsh does not pass \$0 through properly. please declare' \
 \"FLAGS_PARENT=\$0\" before calling shFlags"
-    exit ${FLAGS_ERROR}
   fi
 fi
 
@@ -111,26 +119,27 @@ fi
 # constants
 #
 
+# reserved flag names
+__FLAGS_RESERVED_LIST=' ARGC ARGV ERROR FALSE GETOPT_CMD HELP PARENT TRUE '
+__FLAGS_RESERVED_LIST="${__FLAGS_RESERVED_LIST} VERSION "
+
 # getopt version
 __FLAGS_GETOPT_VERS_STD=0
 __FLAGS_GETOPT_VERS_ENH=1
 __FLAGS_GETOPT_VERS_BSD=2
 
-getopt >/dev/null 2>&1
+${FLAGS_GETOPT_CMD} >/dev/null 2>&1
 case $? in
   0) __FLAGS_GETOPT_VERS=${__FLAGS_GETOPT_VERS_STD} ;;  # bsd getopt
   2)
     # TODO(kward): look into '-T' option to test the internal getopt() version
-    if [ "`getopt --version`" = '-- ' ]; then
+    if [ "`${FLAGS_GETOPT_CMD} --version`" = '-- ' ]; then
       __FLAGS_GETOPT_VERS=${__FLAGS_GETOPT_VERS_STD}
     else
       __FLAGS_GETOPT_VERS=${__FLAGS_GETOPT_VERS_ENH}
     fi
     ;;
-  *)
-    _flags_fatal 'unable to determine getopt version'
-    exit ${FLAGS_ERROR}
-    ;;
+  *) _flags_fatal 'unable to determine getopt version' ;;
 esac
 
 # getopt optstring lengths
@@ -229,7 +238,7 @@ _flags_define()
   # TODO(kward): check for validity of the flag name (e.g. dashes)
 
   # check whether the flag name is reserved
-  _flags_itemInList ${_flags_name_} "${FLAGS_RESERVED_LIST}"
+  _flags_itemInList ${_flags_name_} "${__FLAGS_RESERVED_LIST}"
   if [ $? -eq ${FLAGS_TRUE} ]; then
     flags_error="flag name (${_flags_name_}) is reserved"
     _flags_return_=${FLAGS_ERROR}
@@ -352,8 +361,7 @@ _flags_genOptStr()
 
   for _flags_flag_ in ${__flags_longNames}; do
     _flags_type_=`_flags_getFlagInfo ${_flags_flag_} ${__FLAGS_INFO_TYPE}`
-    [ $? -eq ${FLAGS_TRUE} ] || \
-        ( _flags_fatal 'call to _flags_type_ failed'; return ${FLAGS_ERROR} )
+    [ $? -eq ${FLAGS_TRUE} ] || _flags_fatal 'call to _flags_type_ failed'
     case ${_flags_optStrType_} in
       ${__FLAGS_OPTSTR_SHORT})
         _flags_shortName_=`_flags_getFlagInfo \
@@ -606,7 +614,7 @@ _flags_getoptEnhanced()
       |sed 's/^ *//;s/ *$//;s/ /,/g'`
   _flags_longOpts_=`_flags_genOptStr ${__FLAGS_OPTSTR_LONG}`
 
-  __flags_opts=`getopt \
+  __flags_opts=`${FLAGS_GETOPT_CMD} \
       -o ${_flags_shortOpts_} \
       -l "${_flags_longOpts_},${_flags_boolOpts_}" \
       -- "$@" 2>&1`
@@ -755,7 +763,7 @@ _flags_parseGetopt()
       if [ ${FLAGS_help} -eq ${FLAGS_TRUE} ]; then
         flags_help
         flags_error='help requested'
-        flags_return=${FLAGS_FALSE}
+        flags_return=${FLAGS_TRUE}
         break
       fi
     fi
@@ -836,7 +844,7 @@ FLAGS()
   return ${flags_return}
 }
 
-# This is a helper function for determining the `getopt` version for platforms
+# This is a helper function for determining the 'getopt' version for platforms
 # where the detection isn't working. It simply outputs debug information that
 # can be included in a bug report.
 #
@@ -862,10 +870,10 @@ flags_getoptInfo()
   fi
 
   # getopt info
-  getopt >/dev/null
+  ${FLAGS_GETOPT_CMD} >/dev/null
   _flags_getoptReturn=$?
   _flags_debug "getopt return: ${_flags_getoptReturn}"
-  _flags_debug "getopt --version: `getopt --version 2>&1`"
+  _flags_debug "getopt --version: `${FLAGS_GETOPT_CMD} --version 2>&1`"
 
   unset _flags_getoptReturn
 }
@@ -925,14 +933,16 @@ flags_help()
       flags_type_=`_flags_getFlagInfo \
           "${flags_name_}" ${__FLAGS_INFO_TYPE}`
 
-      [ "${flags_short_}" != "${__FLAGS_NULL}" ] \
-          && flags_flagStr_="-${flags_short_}"
+      [ "${flags_short_}" != "${__FLAGS_NULL}" ] && \
+          flags_flagStr_="-${flags_short_}"
 
       if [ ${__FLAGS_GETOPT_VERS} -eq ${__FLAGS_GETOPT_VERS_ENH} ]; then
-        [ "${flags_short_}" != "${__FLAGS_NULL}" ] \
-            && flags_flagStr_="${flags_flagStr_},"
-        [ ${flags_type_} -eq ${__FLAGS_TYPE_BOOLEAN} ] \
-            && flags_boolStr_='[no]'
+        [ "${flags_short_}" != "${__FLAGS_NULL}" ] && \
+            flags_flagStr_="${flags_flagStr_},"
+        # add [no] to long boolean flag names, except the 'help' flag
+        [ ${flags_type_} -eq ${__FLAGS_TYPE_BOOLEAN} \
+          -a "${flags_name_}" != 'help' ] && \
+            flags_boolStr_='[no]'
         flags_flagStr_="${flags_flagStr_}--${flags_boolStr_}${flags_name_}:"
       fi
 
diff --git a/source/1.0/src/shflags_test_helpers b/source/1.0/src/shflags_test_helpers
index 01199b5..bbdd5a8 100644
--- a/source/1.0/src/shflags_test_helpers
+++ b/source/1.0/src/shflags_test_helpers
@@ -51,7 +51,7 @@ th_oneTimeSetUp()
   . ${TH_SHFLAGS}
 
   # these files will be cleaned up automatically by shUnit2
-  tmpDir=${shunit_tmpDir}
+  tmpDir=${SHUNIT_TMPDIR}
   stdoutF="${tmpDir}/stdout"
   stderrF="${tmpDir}/stderr"
   expectedF="${tmpDir}/expected"
diff --git a/source/1.0/src/shflags_test_public.sh b/source/1.0/src/shflags_test_public.sh
index 6f6d8d8..5077862 100755
--- a/source/1.0/src/shflags_test_public.sh
+++ b/source/1.0/src/shflags_test_public.sh
@@ -22,32 +22,32 @@ testHelp()
   # test short -h option
   #
 
-  rslt=`FLAGS -h 2>&1`
-  rtrn=$?
-  assertFalse \
-      'short request for help should have returned non-zero exit code.' \
-      ${rtrn}
-  echo "${rslt}" |grep -- 'show this help' >/dev/null
-  rtrn=$?
+  result=`FLAGS -h 2>&1`
+  r3turn=$?
+  assertTrue \
+      'short help request should have returned a true exit code.' \
+      ${r3turn}
+  echo "${result}" |grep -- 'show this help' >/dev/null
+  grepped=$?
   assertTrue \
       'short request for help should have produced some help output.' \
-      ${rtrn}
-  [ ${rtrn} -ne ${FLAGS_TRUE} ] && echo "${rslt}" >&2
+      ${grepped}
+  [ ${grepped} -ne ${FLAGS_TRUE} ] && echo "${result}" >&2
 
   # test proper output when FLAGS_HELP set
-  rslt=`FLAGS_HELP='this is a test'; FLAGS -h 2>&1`
-  echo "${rslt}" |grep -- 'this is a test' >/dev/null
-  rtrn=$?
-  assertTrue 'setting FLAGS_HELP did not produce expected result' ${rtrn}
-  [ ${rtrn} -ne ${FLAGS_TRUE} ] && echo "${rslt}" >&2
+  result=`FLAGS_HELP='this is a test'; FLAGS -h 2>&1`
+  echo "${result}" |grep -- 'this is a test' >/dev/null
+  grepped=$?
+  assertTrue 'setting FLAGS_HELP did not produce expected result' ${grepped}
+  [ ${grepped} -ne ${FLAGS_TRUE} ] && echo "${result}" >&2
 
   # test that "'" chars work in help string
   DEFINE_boolean b false "help string containing a ' char" b
-  rslt=`FLAGS -h 2>&1`
-  echo "${rslt}" |grep -- "help string containing a ' char" >/dev/null
-  rtrn=$?
-  assertTrue "help strings containing apostrophes don't work" ${rtrn}
-  [ ${rtrn} -ne ${FLAGS_TRUE} ] && echo "${rslt}" >&2
+  result=`FLAGS -h 2>&1`
+  echo "${result}" |grep -- "help string containing a ' char" >/dev/null
+  grepped=$?
+  assertTrue "help strings containing apostrophes don't work" ${grepped}
+  [ ${grepped} -ne ${FLAGS_TRUE} ] && echo "${result}" >&2
 
   #
   # test long --help option
@@ -55,10 +55,16 @@ testHelp()
 
   flags_getoptIsEnh || startSkipping
 
-  rslt=`FLAGS --help 2>&1`
-  assertFalse 'long help request should have returned non-zero exit code' $?
-  echo "${rslt}" |grep -- 'show this help' >/dev/null
-  assertTrue 'long help request should have produced some help output.' $?
+  result=`FLAGS --help 2>&1`
+  r3turn=$?
+  assertTrue \
+      'long help request should have returned a true exit code' \
+      ${r3turn}
+  echo "${result}" |grep -- 'show this help' >/dev/null
+  grepped=$?
+  assertTrue \
+      'long help request should have produced some help output.' \
+      ${grepped}
 }
 
 testStandardHelpOutput()
@@ -88,10 +94,12 @@ flags:
   -h  show this help (default: false)
 EOF
   ( 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
-  rtrn=$?
-  assertTrue 'unexpected help output' ${rtrn}
-  th_showOutput ${rtrn} "${stdoutF}" "${stderrF}"
+  differed=$?
+  assertTrue 'unexpected help output' ${differed}
+  th_showOutput ${differed} "${stdoutF}" "${stderrF}"
 }
 
 testEnhancedHelpOutput()
@@ -118,13 +126,15 @@ flags:
                    (default: 'blah')
   -F,--long_default:  testing of long default value
     (default: 'this_is_a_long_default_value_to_force_alternate_indentation')
-  -h,--[no]help:  show this help (default: false)
+  -h,--help:  show this help (default: false)
 EOF
   ( 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
-  rtrn=$?
-  assertTrue 'unexpected help output' ${rtrn}
-  th_showOutput ${rtrn} "${stdoutF}" "${stderrF}"
+  differed=$?
+  assertTrue 'unexpected help output' ${differed}
+  th_showOutput ${differed} "${stdoutF}" "${stderrF}"
 }
 
 testNoHelp()
@@ -132,8 +142,8 @@ testNoHelp()
   flags_getoptIsEnh || startSkipping
 
   ( FLAGS --nohelp >"${stdoutF}" 2>"${stderrF}" )
-  rtrn=$?
-  assertTrue "FLAGS returned a non-zero result (${rtrn})" ${rtrn}
+  r3turn=$?
+  assertTrue "FLAGS returned a non-zero result (${r3turn})" ${r3turn}
   assertFalse 'expected no output to STDOUT' "[ -s '${stdoutF}' ]"
   assertFalse 'expected no output to STDERR' "[ -s '${stderrF}' ]"
 }