Refactored logging code and fixed some 'set -e' issues.

1 files changed, 39 insertions, 34 deletions

diff --git a/shflags b/shflags
index 46ca3d7..da09932 100644
--- a/shflags
+++ b/shflags
@@ -91,7 +91,7 @@
 #   shellcheck disable=SC2166
 
 # Return if FLAGS already loaded.
-[ -n "${FLAGS_VERSION:-}" ] && return 0
+if [ -n "${FLAGS_VERSION:-}" ]; then return 0; fi
 FLAGS_VERSION='1.2.4pre'
 
 # Return values that scripts can use.
@@ -99,34 +99,38 @@ FLAGS_TRUE=0
 FLAGS_FALSE=1
 FLAGS_ERROR=2
 
-# Logging levels.
-FLAGS_LEVEL_DEBUG=0
-FLAGS_LEVEL_INFO=1
-FLAGS_LEVEL_WARN=2
-FLAGS_LEVEL_ERROR=3
-FLAGS_LEVEL_FATAL=4
-__FLAGS_LEVEL_DEFAULT=${FLAGS_LEVEL_WARN}
-
-# Determine some reasonable command defaults.
-_flags_expr_cmd() {
+# shlib_expr_cmd determines a reasonable default `expr` command.
+# https://github.com/kward/shlib
+#
+# Use like:
+#   EXPR_CMD=$(shlib_expr_cmd)
+#   ${EXPR_CMD} 1 + 1
+#
+# Args:
+#   none
+# Output:
+#   string: expr command
+# Return
+#   int: 0 upon success
+shlib_expr_cmd() {
   if [ "$(uname -s)" = 'BSD' ]; then
     echo 'gexpr --'
     return 0
   fi
 
-  _flags_expr_cmd_='expr --'
+  _shlib_expr_cmd_='expr --'
   # shellcheck disable=SC2003
-  if _flags_output_=$(expr -- 2>&1); then
-    if [ "${_flags_output_}" = '--' ]; then
+  if _shlib_output_=$(${_shlib_expr_cmd_} 2>&1); then
+    if [ "${_shlib_output_}" = '--' ]; then
       # We are likely running inside BusyBox.
-      _flags_expr_cmd_='expr'
+      _shlib_expr_cmd_='expr'
     fi
   fi
 
-  echo "${_flags_expr_cmd_}"
-  unset _flags_expr_cmd_ _flags_output_
+  echo "${_shlib_expr_cmd_}"
+  unset _shlib_expr_cmd_ _shlib_output_
 }
-__FLAGS_EXPR_CMD=`_flags_expr_cmd`
+__FLAGS_EXPR_CMD=`shlib_expr_cmd`
 
 # Commands a user can override if desired.
 FLAGS_EXPR_CMD=${FLAGS_EXPR_CMD:-${__FLAGS_EXPR_CMD}}
@@ -136,26 +140,28 @@ FLAGS_GETOPT_CMD=${FLAGS_GETOPT_CMD:-getopt}
 # Logging functions.
 #
 
-__flags_level=0 # Default logging level.
+# Logging levels.
+FLAGS_LEVEL_DEBUG=0
+FLAGS_LEVEL_INFO=1
+FLAGS_LEVEL_WARN=2
+FLAGS_LEVEL_ERROR=3
+FLAGS_LEVEL_FATAL=4
+__FLAGS_LEVEL_DEFAULT=${FLAGS_LEVEL_WARN}
+__flags_level=${__FLAGS_LEVEL_DEFAULT} # Current logging level.
 
 _flags_debug() {
-  [ ${__flags_level} -le ${FLAGS_LEVEL_DEBUG} ] || return
-  echo "flags:DEBUG $*" >&2
+  if [ ${__flags_level} -le ${FLAGS_LEVEL_DEBUG} ]; then echo "flags:DEBUG $*" >&2; fi
 }
 _flags_info() {
-  [ ${__flags_level} -le ${FLAGS_LEVEL_INFO} ] || return
-  echo "flags:INFO $*" >&2
+  if [ ${__flags_level} -le ${FLAGS_LEVEL_INFO} ]; then echo "flags:INFO $*" >&2; fi
 }
 _flags_warn() {
-  [ ${__flags_level} -le ${FLAGS_LEVEL_WARN} ] || return
-  echo "flags:WARN $*" >&2
+  if [ ${__flags_level} -le ${FLAGS_LEVEL_WARN} ]; then echo "flags:WARN $*" >&2; fi
 }
 _flags_error() {
-  [ ${__flags_level} -le ${FLAGS_LEVEL_ERROR} ] || return
-  echo "flags:ERROR $*" >&2
+  if [ ${__flags_level} -le ${FLAGS_LEVEL_ERROR} ]; then echo "flags:ERROR $*" >&2; fi
 }
 _flags_fatal() {
-  [ ${__flags_level} -le ${FLAGS_LEVEL_FATAL} ] || return
   echo "flags:FATAL $*" >&2
   exit ${FLAGS_ERROR}
 }
@@ -163,7 +169,7 @@ _flags_fatal() {
 # Get the logging level.
 flags_loggingLevel() { echo ${__flags_level}; }
 
-# Set the logging level.
+# Set the logging level by overriding the `__flags_level` variable.
 #
 # Args:
 #   _flags_level_: integer: new logging level
@@ -172,9 +178,9 @@ flags_loggingLevel() { echo ${__flags_level}; }
 flags_setLoggingLevel() {
   [ $# -ne 1 ] && _flags_fatal "flags_setLevel(): logging level missing"
   _flags_level_=$1
-  [ "${_flags_level_}" -ge "${FLAGS_LEVEL_DEBUG}" \
-    -a "${_flags_level_}" -le "${FLAGS_LEVEL_FATAL}" ] \
-      || _flags_fatal "Invalid logging level '${_flags_level_}' specified."
+  if ! [ "${_flags_level_}" -ge "${FLAGS_LEVEL_DEBUG}" -a "${_flags_level_}" -le "${FLAGS_LEVEL_FATAL}" ]; then
+    _flags_fatal "Invalid logging level '${_flags_level_}' specified."
+  fi
   __flags_level=$1
   unset _flags_level_
 }
@@ -327,8 +333,7 @@ _flags_define() {
   _flags_usName_="`_flags_underscoreName "${_flags_name_}"`"
 
   # Check whether the flag name is reserved.
-  _flags_itemInList "${_flags_usName_}" "${__FLAGS_RESERVED_LIST}"
-  if [ $? -eq ${FLAGS_TRUE} ]; then
+  if _flags_itemInList "${_flags_usName_}" "${__FLAGS_RESERVED_LIST}"; then
     flags_error="flag name (${_flags_name_}) is reserved"
     _flags_return_=${FLAGS_ERROR}
   fi