Make parse_options work with everything: cmd line, config files, values with spaces, op val, op=val, etc.

docs/percona-toolkit.pod

@@ -182,8 +182,8 @@ The syntax of the configuration files is as follows:
 
 =item *
 
-Whitespace followed by a hash (#) sign signifies that the rest of the line is a
-comment.  This is deleted.
+Whitespace followed by a hash sign (#) signifies that the rest of the line is a
+comment.  This is deleted.  For example:
 
 =item *
 
@@ -200,7 +200,9 @@ Each line is permitted to be in either of the following formats:
   option
   option=value
 
-Whitespace around the equals sign is deleted during processing.
+Do not prefix the option with C<-->.  Do not quote the values, even if
+it has spaces; value are literal.  Whitespace around the equals sign is
+deleted during processing.
 
 =item *
 
@@ -214,6 +216,22 @@ program.
 
 =back
 
+=head2 EXAMPLE
+
+This config file for pt-stalk,
+
+  # Config for pt-stalk
+  variable=Threads_connected
+  cycles=2  # trigger if problem seen twice in a row
+  --
+  --user daniel
+
+is equivalent to this command line:
+
+  pt-stalk --variable Threads_connected --cycles 2 -- --user daniel
+
+Options after C<--> are passed literally to mysql and mysqladmin.
+
 =head2 READ ORDER
 
 The tools read several configuration files in order:

lib/bash/parse_options.sh

@@ -26,6 +26,24 @@
 # GLOBAL $TMPDIR AND $TOOL MUST BE SET BEFORE USING THIS LIB!
 # ***********************************************************
 
+# Parsing command line options with Bash is easy until we have to dealt
+# with values that have spaces, e.g. --option="hello world".  This is
+# further complicated by command line vs. config file.  From the command
+# line, <--option "hello world"> is put into $@ as "--option", "hello world",
+# i.e. 2 args.  From a config file, <option=hello world> is either 2 args
+# split on the space, or 1 arg as a whole line.  It needs to be 2 args
+# split on the = but this isn't possible; see the note before while read
+# in _parse_config_files().  Perl tool config files do not work when the
+# value is quoted, so we can't quote it either.  And in any case, that
+# wouldn't work because then the value would include the literal quotes
+# because it's a line from a file, not a command line where Bash will
+# interpret the quotes and return a single value in the code. So...
+
+# ***************************************************
+# BE CAREFUL MAKING CHANGES TO THIS LIB AND MAKE SURE
+# t/lib/bash/parse_options.sh STILL PASSES!
+# ***************************************************
+
 set -u
 
 # Global variables.  These must be global because declare inside a
@@ -231,25 +249,52 @@ _eval_po() {
 }
 
 _parse_config_files() {
-   local config_files="/etc/percona-toolkit/percona-toolkit.conf /etc/percona-toolkit/$TOOL.conf $HOME/.percona-toolkit.conf $HOME/.$TOOL.conf"
-   for config_file in $config_files; do
+   for config_file in "/etc/percona-toolkit/percona-toolkit.conf" "/etc/percona-toolkit/$TOOL.conf" "$HOME/.percona-toolkit.conf" "$HOME/.$TOOL.conf"
+   do
+      # Next config file if this one doesn't exist.
       test -f "$config_file" || continue
+
+      # We've hit a -- in the config file, so just append everything
+      # else to EXT_ARGV.
       local dashdash=""
-      for conf_opt in $(grep '^[^# ]' "$config_file"); do
+
+      # We must use while read because values can contain spaces.
+      # Else, if we for $(grep ...) then a line like "op=hello world"
+      # will return 2 values: "op=hello" and "world".  If we quote
+      # the command like for "$(grep ...)" then the entire config
+      # file is returned as 1 value like "opt=hello world\nop2=42".
+      while read config_opt; do
+
+         # Skip the line if it begins with a # or is blank.
+         echo "$config_opt" | grep '^[ ]*[^#]' >/dev/null 2>&1 || continue
+
+         # Strip leading and trailing spaces, and spaces around the first =,
+         # and end-of-line # comments.
+         config_opt="$(echo "$config_opt" | sed -e 's/^[ ]*//' -e 's/[ ]*\$//' -e 's/[ ]*=[ ]*/=/' -e 's/[ ]*#.*$//')"
+
          if [ "$dashdash" ]; then
+            # Previous line was -- so this and subsequent options are
+            # really external argvs.
             if [ "$EXT_ARGV" ]; then
-               EXT_ARGV="$EXT_ARGV $conf_opt"
+               EXT_ARGV="$EXT_ARGV $config_opt"
             else
-               EXT_ARGV="$conf_opt"
+               EXT_ARGV="$config_opt"
             fi
          else
-            _parse_command_line "$conf_opt"
+            # Options in a config file are not prefixed with --,
+            # but command line options are, so one or the other has
+            # to add or remove the -- prefix.  We add it for config
+            # files rather than trying to strip it from command line
+            # options because it's a simpler operation here.
+            _parse_command_line "--$config_opt"
+
+            # _parse_command_line() returns 1 when it sees --.
             if [ $? -eq 1 ]; then
                dashdash=1
                EXT_ARGV=""
             fi
          fi
-      done
+      done < "$config_file"
    done
 }
 
@@ -283,7 +328,7 @@ _parse_command_line() {
          val="$opt"
          opt_is_ok=1
       else
-         if [ "$opt" = "--" ]; then
+         if [ "$opt" = "--" -o "$opt" = "----" ]; then
             EXT_ARGV="$@"
             return 1
          fi

t/lib/bash/parse_options.sh

@@ -1,6 +1,6 @@
 #!/usr/bin/env bash
 
-TESTS=44
+TESTS=46
 
 TMPFILE="$TEST_TMPDIR/parse-opts-output"
 TOOL="pt-stalk"
@@ -49,6 +49,10 @@ parse_options "$T_LIB_DIR/samples/bash/po001.sh" --int-opt=42
 
 is "$OPT_INT_OPT" "42" "Specified int option (--option=value)"
 
+parse_options "$T_LIB_DIR/samples/bash/po001.sh" --string-opt="hello world"
+
+is "$OPT_STRING_OPT" "hello world" "Specified int option (--option=\"value\")"
+
 # ############################################################################
 # Negate an option like --no-option.
 # ############################################################################
@@ -128,8 +132,11 @@ is "$OPT_STRING_OPT" "zzz" "Command line overrides config file"
 
 # Config file
 cp "$T_LIB_DIR/samples/bash/config002.conf" "$HOME/.$TOOL.conf"
+
 parse_options "$T_LIB_DIR/samples/bash/po001.sh" ""
+
 is "$OPT_STRING_OPT" "hello world" "Option value with space (conf)"
+is "$OPT_INT_OPT" "100" "Option = value # comment (conf)"
 
 rm "$HOME/.$TOOL.conf"
 TOOL="pt-stalk"

t/lib/samples/bash/config001.conf

@@ -1,5 +1,5 @@
---string-opt=abc
---typeless-option
+string-opt=abc
+typeless-option
 --
 --host=127.1
 --user=daniel

t/lib/samples/bash/config002.conf

@@ -1 +1,5 @@
---string-opt "hello world"
+# Line comment.
+string-opt=hello world
+
+   
+int-opt = 100   # Inline comment.