From 73ed8cb53e52e4f30f36558202c2652fa01e4e54 Mon Sep 17 00:00:00 2001
From: "Tom \"Ravi\" Hale" <tom@hale.ee>
Date: Sat, 25 Aug 2018 20:30:42 +0700
Subject: [PATCH] scmb_expand_args: return an array to fix quoting issues

---
 lib/git/status_shortcuts.sh           | 34 ++++++++++++---------------
 test/lib/git/status_shortcuts_test.sh | 20 ++++++++++------
 2 files changed, 28 insertions(+), 26 deletions(-)

diff --git a/lib/git/status_shortcuts.sh b/lib/git/status_shortcuts.sh
index 69410a2..f7c57cf 100644
--- a/lib/git/status_shortcuts.sh
+++ b/lib/git/status_shortcuts.sh
@@ -79,8 +79,8 @@ git_add_shortcuts() {
 git_silent_add_shortcuts() {
   if [ -n "$1" ]; then
     # Expand args and process resulting set of files.
-    IFS=$'\t'
-    for file in $(scmb_expand_args "$@"); do
+    eval "args=$(scmb_expand_args "$@")"  # create $args array
+    for file in "${args[@]}"; do
       # Use 'git rm' if file doesn't exist and 'ga_auto_remove' is enabled.
       if [[ $ga_auto_remove == "yes" ]] && ! [ -e "$file" ]; then
         echo -n "# "
@@ -90,7 +90,6 @@ git_silent_add_shortcuts() {
         echo -e "# Added '$file'"
       fi
     done
-    unset IFS
     echo "#"
   fi
 }
@@ -121,32 +120,29 @@ scmb_expand_args() {
     shift
   fi
 
-  first=1
-  OLDIFS="$IFS"; IFS=" " # We need to split on spaces to loop over expanded range
+  local -a args=()  # initially empty array
   for arg in "$@"; do
     if [[ "$arg" =~ ^[0-9]{0,4}$ ]] ; then      # Substitute $e{*} variables for any integers
-      if [ "$first" -eq 1 ]; then first=0; else printf '\t'; fi
       if [ -e "$arg" ]; then
         # Don't expand files or directories with numeric names
-        printf '%s' "$arg"
+        args+=("$arg")
       else
-        _print_path "$relative" "$git_env_char$arg"
+        args+=("$(_print_path "$relative" "$git_env_char$arg")")
       fi
     elif [[ "$arg" =~ ^[0-9]+-[0-9]+$ ]]; then           # Expand ranges into $e{*} variables
-
       for i in $(eval echo {${arg/-/..}}); do
-        if [ "$first" -eq 1 ]; then first=0; else printf '\t'; fi
-        _print_path "$relative" "$git_env_char$i"
+        args+=("$(_print_path "$relative" "$git_env_char$i")")
       done
     else   # Otherwise, treat $arg as a normal string.
-      if [ "$first" -eq 1 ]; then first=0; else printf '\t'; fi
-      printf '%s' "$arg"
+      args+=("$arg")
     fi
   done
-  IFS="$OLDIFS"
+  args=$(declare -p args)  # Get $args array as a string which can be `eval`-ed to recreate itself
+  args=${args#*=}  # Remove `typeset -a args=` from beginning of string to allow caller to name variable
+  echo "$args"
 }
 
-# Expand a variable into a (possibly relative) pathname
+# Expand a variable (named by $2) into a (possibly relative) pathname
 _print_path() {
   local pathname=$(eval printf '%s' "\"\${$2}\"")
   if [ "$1" = 1 ]; then  # print relative
@@ -158,7 +154,8 @@ _print_path() {
 # Execute a command with expanded args, e.g. Delete files 6 to 12: $ ge rm 6-12
 # Fails if command is a number or range (probably not worth fixing)
 exec_scmb_expand_args() {
-  eval "$(scmb_expand_args "$@" | sed -e "s/\([][|;()<>^ \"'&]\)/"'\\\1/g')"
+  eval "args=$(scmb_expand_args "$@")"  # create $args array
+  _safe_eval "${args[@]}"
 }
 
 # Clear numbered env variables
@@ -180,13 +177,12 @@ git_clear_vars() {
 _git_resolve_merge_conflict() {
   if [ -n "$2" ]; then
     # Expand args and process resulting set of files.
-    IFS=$'\t'
-    for file in $(scmb_expand_args "${@:2}"); do
+    eval "args=$(scmb_expand_args "$@")"  # create $args array
+    for file in "${args[@]:2}"; do
       git checkout "--$1""s" "$file"   # "--$1""s" is expanded to --ours or --theirs
       git add "$file"
       echo -e "# Added $1 version of '$file'"
     done
-    unset IFS
     echo -e "# -- If you have finished resolving conflicts, commit the resolutions with 'git commit'"
   fi
 }
diff --git a/test/lib/git/status_shortcuts_test.sh b/test/lib/git/status_shortcuts_test.sh
index d83d9ca..57b861b 100755
--- a/test/lib/git/status_shortcuts_test.sh
+++ b/test/lib/git/status_shortcuts_test.sh
@@ -49,16 +49,22 @@ setupTestRepo() {
 #-----------------------------------------------------------------------------
 
 test_scmb_expand_args() {
-  local e1="one"; local e2="two"; local e3="three"; local e4="four"; local e5="five"; local e6="six"; local e7="seven"
+  local e1="one" e2="two" e3="three" e4="four" e5="five" e6="six" e7='$dollar' e8='two words'
   local error="Args not expanded correctly"
-  assertEquals "$error" "$(printf 'one\tthree\tseven')" "$(scmb_expand_args 1 3 7)"
-  assertEquals "$error" "$(printf 'one\ttwo\tthree\tsix')" "$(scmb_expand_args 1-3 6)"
-  assertEquals "$error" "$(printf 'seven\ttwo\tthree\tfour\tfive\tone')" "$(scmb_expand_args seven 2-5 1)"
+  assertEquals "$error" "'one' 'three' 'six'" \
+    "$(eval a=$(scmb_expand_args 1 3 6); token_quote "${a[@]}")"
+  assertEquals "$error" "'one' 'two' 'three' 'five'" \
+    "$(eval a=$(scmb_expand_args 1-3 5); token_quote "${a[@]}")"
+  assertEquals "$error" "'\$dollar' 'two' 'three' 'four' 'one'" \
+    "$(eval a=$(scmb_expand_args 7 2-4 1); token_quote "${a[@]}")"
 
   # Test that any args with spaces remain quoted
-  assertEquals "$error" "$(printf -- '-m\tTest Commit Message\tone')" "$(scmb_expand_args -m "Test Commit Message" 1)"
-  assertEquals "$error" "$(printf -- '-ma\tTest Commit Message\tUnquoted')"\
-                        "$(scmb_expand_args -ma "Test Commit Message" "Unquoted")"
+  assertEquals "$error" "'-m' 'Test Commit Message' 'one'" \
+    "$(eval a=$(scmb_expand_args -m "Test Commit Message" 1); token_quote "${a[@]}")"
+  assertEquals "$error" "'-ma' 'Test Commit Message' 'Unquoted'"\
+    "$(eval a=$(scmb_expand_args -ma "Test Commit Message" "Unquoted"); token_quote "${a[@]}")"
+  assertEquals "$error" "'\$dollar' 'one' 'two words'" \
+    "$(eval a=$(scmb_expand_args 7 1-1 8); token_quote "${a[@]}")"
 }
 
 test_command_wrapping_escapes_special_characters() {