0

Based on this answer I wrote some simple optional argument parser

#!/bin/bash
TEST_PATH=pluto
if [ $# -eq 0 ]; then
    echo "Usage:"
    echo "./run.sh [--valgrind|-v] [--test|-t] <mesh.dat> <csv_points_file>"
else
    . config.sh
    while [[ "$#" -gt 0 ]]; do
        case $1 in
            -v|--valgrind) VALGRIND="valgrind --track-origins=yes"; shift ;;
            -t|--test) TEST="mickey goofy" shift ;;
            *) ARGS+=($1) shift ;;
        esac
        shift
        echo $TEST
    done
    echo ${VALGRIND} ${TEST_PATH} "${ARGS[*]:-$TEST}"
fi

Except when executing ./somescript.sh -v the output is correct, while with the -t argument nothing except the correct value of TEST_PATH The expected behavior was meant to be

$./somescript.sh -t
pluto mickey goofy

$./somescript.sh daffy duck
pluto daffy duck

What am I doing wrong?

Eugenio
  • 244
  • 2
  • 13
  • Use `set -x` to enable tracing so you can observe what's happening in practice. – Charles Duffy Aug 24 '20 at 21:47
  • Also, you've got serious bugs here until/unless you change `ARGS` to be an array instead of a single string. – Charles Duffy Aug 24 '20 at 21:50
  • Make it `ARGS+=( "$1" )`, and then `"${ARGS[@]}"` -- the quotes are important. Don't ever use `${array[*]}` unless you know exactly what the differences from `"${array[@]}"` are and why it's what you need. – Charles Duffy Aug 25 '20 at 18:13
  • As a simple test case, try passing `'*.txt'` (as a glob expression, in a directory that contains files with `.txt` extensions) as a single quoted argument through this kind of shim. `'./Documents/a filename with spaces'` is another instructive example. – Charles Duffy Aug 25 '20 at 18:14
  • 1
    (aside: all-caps names are used for variables meaningful to the shell itself, and to POSIX-defined OS components; whereas names with at least one lower-case character are guaranteed not to conflict. See the specification at https://pubs.opengroup.org/onlinepubs/9699919799/basedefs/V1_chap08.html, keeping in mind that shell variables and environment variables share a single namespace -- setting a shell variable will overwrite any like-named environment variable. – Charles Duffy Aug 25 '20 at 18:28

1 Answers1

2

Take out the extra shifts.

That is:

        case $1 in
            -v|--valgrind) VALGRIND="valgrind --track-origins=yes";;
            -t|--test) TEST="mickey goofy";;
            *) ARGS+=( "$1" );;
        esac

You should only do an extra shift inside your individual cases when the option you're processing takes an option-argument, and you need to remove that option-argument from the argument vector before processing can continue.

Charles Duffy
  • 280,126
  • 43
  • 390
  • 441
  • Are you suggesting I should remove the while loop as well? Because if I remove the shifts and leave the while, that's going to cause an infinite loop. May you elaborate on the solution? – Eugenio Aug 25 '20 at 13:41
  • 1
    No, I am not suggesting you remove the while loop. Leave only the one `shift` you have near the end of the loop unconditionally. By showing only a new version of the `case`, I meant to suggest that you change _only_ the `case` and leave everything else alone. – Charles Duffy Aug 25 '20 at 16:26
  • Ok thank you, now working flawlessly. Fixing the array use you suggested – Eugenio Aug 25 '20 at 17:39