providing getopt error output to user; don't run getopt if no args passed

3 files changed, 63 insertions, 50 deletions

diff --git a/source/1.0/src/shflags b/source/1.0/src/shflags
index 6546be3..a60d32d 100644
--- a/source/1.0/src/shflags
+++ b/source/1.0/src/shflags
@@ -573,15 +573,15 @@ _flags_getoptStandard()
 
   if [ ${flags_return} -eq ${FLAGS_TRUE} ]; then
     __flags_opts=`getopt ${_flags_shortOpts_} $@ 2>&1`
-    if [ $? -ne ${FLAGS_TRUE} ]; then
-      # TODO(kward): actually output the failed value
-      _flags_warn 'getopt on this platform supports only short flags.'
+    _flags_rtrn_=$?
+    if [ ${_flags_rtrn_} -ne ${FLAGS_TRUE} ]; then
+      _flags_warn "${__flags_opts}"
       flags_error='unable to parse provided options with getopt.'
       flags_return=${FLAGS_ERROR}
     fi
   fi
 
-  unset _flags_match_ _flags_opt_ _flags_shortOpts_
+  unset _flags_match_ _flags_opt_ _flags_rtrn_ _flags_shortOpts_
   return ${flags_return}
 }
 
@@ -606,13 +606,14 @@ _flags_getoptEnhanced()
       -o ${_flags_shortOpts_} \
       -l "${_flags_longOpts_},${_flags_boolOpts_}" \
       -- "$@" 2>&1`
-  if [ $? -ne ${FLAGS_TRUE} ]; then
-    # TODO(kward): actually output the failed value
+  _flags_rtrn_=$?
+  if [ ${_flags_rtrn_} -ne ${FLAGS_TRUE} ]; then
+    _flags_warn "${__flags_opts}"
     flags_error='unable to parse provided options with getopt.'
     flags_return=${FLAGS_ERROR}
   fi
 
-  unset _flags_boolOpts_ _flags_longOpts_ _flags_shortOpts_
+  unset _flags_boolOpts_ _flags_longOpts_ _flags_rtrn_ _flags_shortOpts_
   return ${flags_return}
 }
 
@@ -653,6 +654,7 @@ _flags_parseGetopt()
     _flags_opt_=$1
     _flags_arg_=${2:-}
     _flags_type_=${__FLAGS_TYPE_NONE}
+    _flags_name_=''
 
     # determine long flag name
     case "${_flags_opt_}" in
@@ -807,19 +809,22 @@ FLAGS()
   [ -z "${__flags_help_type:-}" ] && \
       DEFINE_boolean 'help' false 'show this help' 'h'
 
-  # record original number of args for use elsewhere
-  flags_argc_=$#
-
   # parse options
-  if [ ${__FLAGS_GETOPT_VERS} -ne ${__FLAGS_GETOPT_VERS_ENH} ]; then
-    _flags_getoptStandard "$@"
+  if [ $# -gt 0 ]; then
+    if [ ${__FLAGS_GETOPT_VERS} -ne ${__FLAGS_GETOPT_VERS_ENH} ]; then
+      _flags_getoptStandard "$@"
+    else
+      _flags_getoptEnhanced "$@"
+    fi
+    flags_return=$?
   else
-    _flags_getoptEnhanced "$@"
+    # nothing passed; won't bother running getopt
+    __flags_opts='--'
+    flags_return=${FLAGS_TRUE}
   fi
-  flags_return=$?
 
   if [ ${flags_return} -eq ${FLAGS_TRUE} ]; then
-    _flags_parseGetopt ${flags_argc_} "${__flags_opts}"
+    _flags_parseGetopt $# "${__flags_opts}"
     flags_return=$?
   fi
 
diff --git a/source/1.0/src/shflags_test_defines.sh b/source/1.0/src/shflags_test_defines.sh
index 8c54aa8..552a999 100755
--- a/source/1.0/src/shflags_test_defines.sh
+++ b/source/1.0/src/shflags_test_defines.sh
@@ -186,7 +186,7 @@ testFlagNameIsReserved()
       >"${stdoutF}" 2>"${stderrF}" )
   rtrn=$?
   assertEquals ${FLAGS_ERROR} ${rtrn}
-  th_showOutput ${rtrn} "${stdoutF}" "${stderrF}"
+  assertErrorMsg 'flag name (TRUE) is reserved'
 }
 
 #------------------------------------------------------------------------------
diff --git a/source/1.0/src/shflags_test_parsing.sh b/source/1.0/src/shflags_test_parsing.sh
index cc58979..f34fb06 100755
--- a/source/1.0/src/shflags_test_parsing.sh
+++ b/source/1.0/src/shflags_test_parsing.sh
@@ -19,7 +19,7 @@
 # suite tests
 #
 
-testGetopt_Standard()
+testGetoptStandard()
 {
   _flags_getoptStandard '-b' >"${stdoutF}" 2>"${stderrF}"
   rslt=$?
@@ -30,7 +30,7 @@ testGetopt_Standard()
   assertFalse "parsed invalid flag 'x'" $?
 }
 
-testGetopt_Enhanced()
+testGetoptEnhanced()
 {
   flags_getoptIsEnh || startSkipping
 
@@ -177,27 +177,29 @@ _testInvalidIntegers()
 
 testValidStrings()
 {
-  _testValidStrings '-s'
+  _testValidStrings -s single_word
   flags_getoptIsEnh || startSkipping
-  _testValidStrings '--str'
+  _testValidStrings --str single_word
+  _testValidStrings --str 'string with spaces'
 }
 
 _testValidStrings()
 {
   flag=$1
-  for value in single_word 'string with spaces'; do
-    FLAGS ${flag} "${value}" >"${stdoutF}" 2>"${stderrF}"
-    rtrn=$?
-    assertTrue "FLAGS (${value}) returned a non-zero result (${rtrn})" ${rtrn}
-    assertEquals "string (${value}) test failed." "${value}" "${FLAGS_str}"
-    if [ ${rtrn} -eq ${FLAGS_TRUE} ]; then
-      assertFalse 'expected no output to STDERR' "[ -s '${stderrF}' ]"
-    else
-      # validate that an error is thrown for unsupported getopt uses
-      assertFatalMsg '.* spaces in options'
-    fi
-    th_showOutput ${rtrn} "${stdoutF}" "${stderrF}"
-  done
+  value=$2
+
+  FLAGS ${flag} "${value}" >"${stdoutF}" 2>"${stderrF}"
+  rtrn=$?
+  assertTrue "'FLAGS ${flag} ${value}' returned a non-zero result (${rtrn})" \
+      ${rtrn}
+  assertEquals "string (${value}) test failed." "${value}" "${FLAGS_str}"
+  if [ ${rtrn} -eq ${FLAGS_TRUE} ]; then
+    assertFalse 'expected no output to STDERR' "[ -s '${stderrF}' ]"
+  else
+    # validate that an error is thrown for unsupported getopt uses
+    assertFatalMsg '.* spaces in options'
+  fi
+  th_showOutput ${rtrn} "${stdoutF}" "${stderrF}"
 }
 
 testMultipleFlags()
@@ -232,7 +234,7 @@ _testMultipleFlags()
 
 _testNonFlagArgs()
 {
-  argv=$1
+  argc=$1
   shift
 
   FLAGS "$@" >"${stdoutF}" 2>"${stderrF}"
@@ -241,7 +243,7 @@ _testNonFlagArgs()
   th_showOutput ${rtrn} "${stdoutF}" "${stderrF}"
 
   eval set -- "${FLAGS_ARGV}"
-  assertEquals 'wrong count of argv arguments returned.' ${argv} $#
+  assertEquals 'wrong count of argv arguments returned.' ${argc} $#
   assertEquals 'wrong count of argc arguments returned.' 0 ${FLAGS_ARGC}
 }
 
@@ -257,6 +259,7 @@ testMultipleNonFlagArgs()
 
 testMultipleNonFlagStringArgsWithSpaces()
 {
+  flags_getoptIsEnh || startSkipping
   _testNonFlagArgs 3 argOne 'arg two' arg3
 }
 
@@ -274,32 +277,37 @@ testFlagsWithEquals()
   assertEquals 'wrong count of argc arguments returned.' 1 ${FLAGS_ARGC}
 }
 
-_testComplicatedCommandLine()
+testComplicatedCommandLineStandard()
 {
-  FLAGS "$@" >"${stdoutF}" 2>"${stderrF}"
+  flags_getoptIsStd || startSkipping
+
+  # note: standard getopt stops parsing after first non-flag argument :-(
+  FLAGS -i 1 non_flag_1 -s 'two' non_flag_2 -f 3 non_flag_3 \
+      >"${stdoutF}" 2>"${stderrF}"
   rtrn=$?
   assertTrue 'FLAGS returned a non-zero result' ${rtrn}
-  assertEquals 'failed std int test' 1 ${FLAGS_int}
-  assertEquals 'failed std str test' 'two' "${FLAGS_str}"
-  assertEquals 'failed std float test' 3 ${FLAGS_float}
+  assertEquals 'failed int test' 1 ${FLAGS_int}
   th_showOutput ${rtrn} "${stdoutF}" "${stderrF}"
 
   eval set -- "${FLAGS_ARGV}"
-  assertEquals 'incorrect number of std argv values' 3 $#
-}
-
-testComplicatedCommandLine_Standard()
-{
-  _testComplicatedCommandLine \
-      -i 1 non_flag_1 -s 'two' non_flag_2 -f 3 non_flag_3
+  assertEquals 'incorrect number of argv values' 7 $#
 }
 
-testComplicatedCommandLine_Enhanced()
+testComplicatedCommandLineEnhanced()
 {
   flags_getoptIsEnh || startSkipping
 
-  _testComplicatedCommandLine \
-      -i 1 non_flag_1 --str='two' non_flag_2 --float 3 'non flag 3'
+  FLAGS -i 1 non_flag_1 --str='two' non_flag_2 --float 3 'non flag 3' \
+      >"${stdoutF}" 2>"${stderrF}"
+  rtrn=$?
+  assertTrue 'FLAGS returned a non-zero result' ${rtrn}
+  assertEquals 'failed int test' 1 ${FLAGS_int}
+  assertEquals 'failed str test' 'two' "${FLAGS_str}"
+  assertEquals 'failed float test' 3 ${FLAGS_float}
+  th_showOutput ${rtrn} "${stdoutF}" "${stderrF}"
+
+  eval set -- "${FLAGS_ARGV}"
+  assertEquals 'incorrect number of argv values' 3 $#
 }
 
 #------------------------------------------------------------------------------