capser
capser

Reputation: 2635

Using a while loop for iterating over an argument list goes on a infinite loop

I am putting in some date parameters into a script

#!/bin/bash
set -x
today=$(date +%Y%m%d)
while  [ ! $# -eq 0 ] #while number of arguments is NOT zero, do this
    do
    if     [[ "$1" = "--test" ]] || [[ "$1" = "-t" ]] ;     then
            today=$(date +%Y%m%d).test
    elif  [[ "$1" = "--yesterday" ]]  || [[ "$1" =  "-y" ]] ; then
            DOW=$(date +%u)
            if [[ "$DOW" =  "1" ]] ; then
                today=$(date --date="3 days ago" +%Y%m%d)
            else
                today=$(date -d "yesterday 13:00" +%Y%m%d)
            fi
    fi
done

echo "$today"
exit 

on the set-x I get in infinite loop. It never gets to print today. I ran it though the shellcheck and it says that it is ok.

++ '[' '!' 1 -eq 0 ']'
++ [[ --t = \-\-\t\e\s\t ]]
++ [[ --t = \-\t ]]
++ [[ --t = \-\-\y\e\s\t\e\r\d\a\y ]]
++ [[ --t = \-\y ]]
++ '[' '!' 1 -eq 0 ']'
++ [[ --t = \-\-\t\e\s\t ]]
++ [[ --t = \-\t ]]
++ [[ --t = \-\-\y\e\s\t\e\r\d\a\y ]]
++ [[ --t = \-\y ]]
++ '[' '!' 1 -eq 0 ']'
++ [[ --t = \-\-\t\e\s\t ]]
++ [[ --t = \-\t ]]
++ [[ --t = \-\-\y\e\s\t\e\r\d\a\y ]]
++ [[ --t = \-\y ]]
++ '[' '!' 1 -eq 0 ']'
++ [[ --t = \-\-\t\e\s\t ]]
++ [[ --t = \-\t ]]
++ [[ --t = \-\-\y\e\s\t\e\r\d\a\y ]]
++ [[ --t = \-\y ]]

Upvotes: 2

Views: 2892

Answers (3)

Kusalananda
Kusalananda

Reputation: 15623

The error comes from a missing shift in the loop, preventing the loop from looking at all command line parameters.

Suggestion:

#!/bin/sh

fmt='%Y%m%d'
when='now'

while [ "$#" -ne 0 ]; do
    case "$1" in
        -t|--test)                   # or just:  -t|--t*)
            fmt="$fmt.test" ;;
        -y|--yesterday)              # or just:  -y|--y*)
            if [ "$(date +%u)" -eq 1 ]; then
                when='3 days ago'    # or 'last friday'
            else
                when='yesterday 13:00'
            fi ;;
        [!-]*) break ;;        # non-option detected, break out of loop
        --)    shift; break ;; # explicit end of options detected, break
        *)  echo 'Error in command line options' >&2
            exit 1
    esac
    shift
done

today=$(date --date="$when" +"$fmt")
echo "$today"

This is the same script, but I have chosen to use case ... esac instead of a if statement to look at the parameters, which apart from being easier to read, also allows you to support abbreviated command line options (see comments in code).

I also move the final setting of today to the end of the script and only set when and fmt in the loop. Notice how this allows you to use both --test and --yesterday together.

If your script does not need long options, this could be written

#!/bin/sh

fmt='%Y%m%d'
when='now'

while getopts 'ty' opt; do
    case "$opt" in
        t)
            fmt="$fmt.test" ;;
        y)
            DOW=$(date +%u)
            if [ "$(date +%u)" -eq 1 ]; then
                when='3 days ago'
            else
                when='yesterday 13:00'
            fi ;;
        *)  echo 'Error in command line options' >&2
            exit 1
    esac
done

shift "$(( OPTIND - 1 ))"

today=$(date --date="$when" +"$fmt")
echo "$today"

This makes it possible to use -ty to specify both -t and -y.

The final shift is only needed if you need to use the non-option arguments from the command line later in the script.

Since the scripts above does not use anything specific to bash, I have chosen to run them with /bin/sh.

Upvotes: 3

glenn jackman
glenn jackman

Reputation: 246942

Or use the tricky getopt command:

#!/bin/bash
tmp=$(getopt -o ty --long test,yesterday -- "$@")
[[ $? -eq 0 ]] || exit
eval set -- "$tmp"

date_fmt="+%Y%m%d"

while true; do
    case $1 in
        -t|--test) 
            today=$(date "$date_fmt").test
            shift
            ;;
        -y|--yesterday)
            dow=$(date +%w)     # (0..6), 0 is Sunday
            if (( $dow <= 1 )); then
                rel_date="last friday"
            else
                rel_date=yesterday
            fi
            today=$(date -d "$rel_date" "$date_fmt")
            shift
            ;;
        --) shift; break ;;
        *) echo "Oops, something is wrong" >&2; exit 2 ;;
    esac
done
echo "${today:=$(date "$date_fmt")}"

Upvotes: 1

Inian
Inian

Reputation: 85690

Yes, ain't that obvious, the positional argument count $# will always be the number of arguments passed, unless you do an equivalent decrement operation on that, the loop is never exit.

You need to use shift after the else condition so that positional argument is moved by one and the loop then decides if there are any more arguments left to be processed.

You ended up in this loop because it was a bad idea in the first place to use a while loop to process the arguments, should have used a for-loop

for arg; do

or a traditional C-style loop

for ((i=1; i<=$#; i++)); do

To suit your purpose on while loop, you could do something like below, but it is not recommended. Notice the shift which is what you were missing in the original post leading to the infinite loop.

while [ -n "$1" ]; do
  shift
  # Your code logic goes here
done

Upvotes: 4

Related Questions