1

I have the following bash script that provides a Zentiy TUI to insert data into a database.

#!/bin/bash
tip_run1="$(zenity --entry --text "ENTER number of tip runs:" --entry-text "1")"
 a=1

until [[ $tip_run1 -lt $a ]] do input="$(zenity --forms --title="table tip_run" --text="Add a new tip run" --separator=","
--add-entry="ENTER start_time, e.g. 8:20: "
--add-entry="ENTER finish_time, e.g. 12:30: "
--add-entry="ENTER weight in kg, -t numeric: "
--add-entry="ENTER a note, -t text: ")"

psql -tA -U chh1 -d crewdb -c "SELECT SETVAL('tip_run_tip_runid_seq', (SELECT MAX(tip_runid) FROM tip_run), true);" >/dev/null 2>&1

startt="$(echo "$input" | awk -F, -v OFS=, '{print $1}')" finisht="$(echo "$input" | awk -F, -v OFS=, '{print $2}')"

st="$( date --date="$startt" +%s 2>/dev/null )" ft="$( date --date="$finisht" +%s 2>/dev/null )"

if [ -n "$st" -a "$ft" ] ; then

startt="$(date +%H:%M  -d "$startt"  )"
finisht="$(date +%H:%M -d "$finisht" )"
tzdiff="$(( ft - st ))"

else tzdiff=0 fi

while [[ ( ( ! "$startt" =~ ^[0-1][0-9]:[0-5][0-9]$ ) && ( ! "$startt" =~ ^[0-2][0-3]:[0-5][0-9]$ ) ) || ( ( ! "$finisht" =~ ^[0-1][0-9]:[0-5][0-9]$ ) && ( ! "$finisht" =~ ^[0-2][0-3]:[0-5][0-9]$ ) ) || ( "$tzdiff" -le 0 ) ]]; do var2="$(zenity --forms --title="start_time and/or finish_time are incorrect" --text "Add a start_time and a finish_time" --separator=","
--add-entry="WARNING! Something went wrong. Please enter a valid start_time, e.g. 8:20: "
--add-entry="WARNING! Something went wrong. Please enter a valid finish_time, e.g. 12:30: ")" tzdiff=0

if [ -n "$var2" ] ; then
   b1=$(echo "$var2" | cut -d, -f1 )
   b2=$(echo "$var2" | cut -d, -f2 )

   if [ -n "$b1" -a -n "$b2"  ] ; then
       tz1=$( date --date="$b1" +%s 2>/dev/null )
       tz2=$( date --date="$b2" +%s 2>/dev/null )

   if [ -n "$tz1" -a -n "$tz2" ] ; then
          startt=$(date +%H:%M -d "$b1" )
          finisht=$(date +%H:%M -d "$b2" )
          tzdiff=$(( tz2 - tz1 ))
       fi
   fi
fi

done

var2="$startt,$finisht"

input="$( echo "$input" | awk -v vart="$var2" 'BEGIN { FS="," } { print vart "," $3 "," $4 ; }' )"

input="$((IFS=, read -r b c d e ; echo "${b}ttt,${c}ttt,${d}xxx,${e}www" )<<<"$input")"

IFS=,; set -f; set --$input; out= for i in "$@"; do

    case &quot;$i&quot; in
            xxx) var2=&quot;$(zenity --forms --title=&quot;weight field in table tip_run&quot; --text &quot;Add a weight in kg&quot;  --separator=&quot;,&quot; \
                            --add-entry=&quot;WARNING! You forgot to enter a weight. Please enter a valid weight, e.g. 12.5: &quot;)&quot;

                            until [[ ${var2} =~ ^[0-9]+([.][0-9]+)?$ ]] || [[ ${var2} = NULL ]]; do

                                    var2=&quot;$(zenity --forms --title=&quot;weight field in table tip_run&quot; --text &quot;Add a weight in kg&quot;  --separator=&quot;,&quot; \
                                    --add-entry=&quot;WARNING! You either forgot to enter a weight or didn't enter a number. Please enter a valid weight, e.g. 12.5: &quot;)&quot;

                            done

                            out=&quot;$out,${var2}&quot;                          
                            ;;

    NULLxxx) out=&quot;$out,${i/%xxx/}&quot;;;        
            *xxx) if [[ &quot;${i/%xxx}&quot; =~ ^[0-9]+([.][0-9]+)?$ ]]; then

        out=&quot;$out,${i/%xxx/}&quot;

        else
             until [[ ${var2} =~ ^[0-9]+([.][0-9]+)?$ ]] || [[ ${var2} = NULL ]]; do

                                    var2=&quot;$(zenity --forms --title=&quot;weight field in table tip_run&quot; --text &quot;Add a weight in kg&quot;  --separator=&quot;,&quot; \
                                    --add-entry=&quot;WARNING! You either forgot to enter a weight or didn't enter a number. Please enter a valid weight, e.g. 12.5: &quot;)&quot;

                            done

                            out=&quot;$out,${var2}&quot;
        fi
                            ;;
    *ttt) out=&quot;$out,'${i/%ttt/}:00'&quot;;;
            #TRUEbool) out=&quot;$out,${i/%bool/}&quot;;;
            #FALSEbool) out=&quot;$out,${i/%bool/}&quot;;;
            #*bool) echo &quot;empty input not allowed&quot;; exit 0;;
    NULLwww) out=&quot;$out,${i/%www/}&quot;;;
    www)            var2=&quot;$(zenity --forms --title=&quot;note field in table tip_run&quot; --text &quot;Add a note&quot;  --separator=&quot;,&quot; \
                            --add-entry=&quot;WARNING! You either forgot to enter a note. Please enter a note or NULL: &quot;)&quot;

                            until [[ ! ${var2} = &quot;&quot;  ]]; do

                                    var2=&quot;$(zenity --forms --title=&quot;note field in table tip_run&quot; --text &quot;Add a note&quot;  --separator=&quot;,&quot; \
                                    --add-entry=&quot;WARNING! You either forgot to enter a note or to enter NULL. Please enter a note or NULL: &quot;)&quot;

                            done

            if [[ ${var2} = &quot;NULL&quot;  ]]; then
                out=&quot;$out,${var2}&quot;
            else

            out=&quot;$out,\$\$${var2}\$\$&quot;
            fi;;

    *www) out=&quot;$out,\$\$${i/%www/}\$\$&quot;;;

esac; done

#echo "${out:1}"

echo "" >> /tmp/crew.txt echo "" >> /tmp/crew.txt echo "-- INSERT INTO tip_run:" >> /tmp/crew.txt echo "INSERT INTO tip_run (date_linkid, start_time, finish_time, weight, note) VALUES (${out:1});" >> /tmp/crew.txt

let a++

done

I know the individual components of the script work but when I run it I get the following error message:

./tip_run.txt: line 64: set: --: invalid option
set: usage: set [-abefhkmnptuvxBCHP] [-o option-name] [--] [arg ...]

Line 64 is at the following location:

 62 input="$((IFS=, read -r  b c d e ; echo "${b}ttt,${c}ttt,${d}xxx,${e}www" )<    <<"$input")"
 63 
 64 IFS=,; set -f; set --$input; out=
 65 for i in "$@"; do
 66 
 67         case "$i" in

I have tried different options with set but none seem to work. Can anyone put me on the right track with this?

2 Answers2

1

The problem is with set --$input, it "should" be set -- $input. Or maybe it shouldn't be here at all.


The purpose of set -- $something and how it works

Few answers to one of your previous questions advised to use set -- $OUTPUT. The full relevant snippet was like:

IFS=,; set -f; set -- $OUTPUT
for i do …

Note: for i do, for i; do and for i in "$@"; do are equivalent.

In that question you wanted to split the content of the variable using , as delimiter. Then you wanted to loop over the resulting chunks. You tried to use awk. It was noted the shell can do this and the snippet is the way. Let's analyze how it works exactly.

The main idea is to use the fact an unquoted variable undergoes word splitting upon expansion. Normally an unquoted variable triggers filename generation as well (e.g. expansion of wildcards like * if they are stored in the variable). Usually you want neither filename generation nor word splitting, therefore the general rule is to always double-quote unless you know what you're doing. Among few cases where not quoting is justified there is set -- $something, but you need to know what you are doing. This is what you are doing:

  • you specify IFS=, so future word splitting treats , as delimiter;
  • by set -f you disable the filename generation mechanism.

After this an unquoted $something expands safely to zero or more words. Still you need to loop over them. It's easy to loop over positional parameters (i.e. parameters you can get with $1, $2, …; or with $@ or (not equivalently) $* to get all of them at once; quoting is important).

set is a shell builtin that can set or unset shell options (e.g. set -f we already know) and positional parameters. Arguments that don't look like options modify the array of positional parameters. E.g. set foo bar makes $1 expand to foo and $2 expand to bar.

Words from the expansion of $something can be assigned to positional parameters with set $something but this is flawed in at least two aspects:

  1. If a word looks like an option to set, it will be treated as an option. The option may or may not be recognized as a valid (supported) option.
  2. If $something expands to exactly zero words then set $something will end up as sole set which does not modify the array of positional parameters.

The first problem is general, not specific to set. Many tools (including set) support double dash (--) to solve it. The second problem is specific to set. The design of the tool makes -- solve the second problem as well. I mean if $something expands to exactly zero words then set -- $something will end up as set -- which does modify the array of positional parameters as expected: all positional parameters will be unset.

So set -- $something is a robust way to assign zero or more words from the expansion of $something to positional parameters you can then easily loop over.

Notes:

  • It's possible to skip set -- $something and start a loop like this:

    IFS=,; set -f
    for i in $something; do
    

    But then you enter the loop with modified IFS and set -f. Using set -- $something and then for i do allows you to revert before for. In your current code you seem not to care about the modified IFS and set -f. In general they can backfire later (you should already know).

  • In Bash you can use a named array (e.g. myarray in this answer) and loop over it.


What is wrong with set --$something or set --"$something"?

In case of set --$something (or set --"$something") -- is concatenated with the first word from the expansion of $something (or with the only word from the expansion of "$something"). There are some edge cases that would make -- stay -- but most likely it becomes --whatever. This string starts with -, so it looks like an option or options.

There are few conventions regarding options. Most tools support single-dash-single-character options (short options, e.g. -a or -1). Many tools support double-dash-multi-character options (long options, e.g. --almost-all). Few tools treat single-dash-multi-character options as single options (e.g. in 7z there is -bd or -bt) but many will treat -abc like -a -b -c, if neither -a nor -b requires an option-argument. In general a tool can parse and interpret its arguments in any way it wants, so this can be complicated.

set --whatever does not treat whatever as a long option, it doesn't support long options at all. It looks set --whatever is almost like set -- -w -h -a -t -e -v -e -r, except this -- does not work like the double dash elaborated earlier. Here -- tries to work as a short option, a single-dash-single-character option where the single-character part happens to be a dash. But there is no such short option:

set: --: invalid option

-- needs to be a separate argument. By omitting the space after -- you made set wrongly interpret its first argument as options. Quite amusing, regarding the fact that the main point of properly used -- is to indicate the end of options.


What is wrong with set -- "$something"?

In a (now deleted) comment the idea of set -- "$something" appeared, probably because of the "always quote" rule. This expands $something to a single word. The main goal in the first place was to split to possibly multiple words, so quoting is certainly not the right thing to do.

Let's suppose for a while you really wanted $something as a single word, you did set -- "$something" and it worked well. In such case the consecutive for i in "$@"; do makes no sense because you're iterating over a single element. There's no need for a loop.


Do you really need set, for and case?

In your already mentioned question you tested each field with the same code. Iterating over fields was a good idea.

In the current question:

  • At some point you're passing the value of $input through awk and I strongly suspect that by design there are exactly four comma-separated fields afterwards.
  • Then you're using IFS=, read -r to split $input to $b, $c, $d and $e; these are the fields.
  • Then you're saving ${b}ttt,${c}ttt,${d}xxx,${e}www altogether again.
  • Then you're splitting it again, this time with set -- $input, so instead of $b, $c, $d and $e you have $1, $2, $3 and $4.
  • Then you're looping over the fields and you're using case to run different code for different fields. I mean NULLxxx can only match the third field (${d}xxx). You're using xxx to recognize the third field, but you had it separated as $d earlier. Why not testing $d for being NULL in the first place?

Splitting to positional parameters is useful if you don't know in advance how many fields you have. If you know you have exactly four, then

IFS=, read -r  b c d e

in the main shell will give you four well-defined separate variables to work with. You can still loop with for i in "$b" "$c" "$d" "$e"; do, but since in your original code only ${b}ttt and ${c}ttt share a case they can match (*ttt), it makes sense to loop over "$b" and "$c", then parse "$d" and "$e" separately. The code common for "$b" and "$c" is so simple you can as well repeat it without introducing a loop:

out="$out,'$b:00'"
out="$out,'$c:00'"

and then:

# what to do if $d is empty
# what to do if $d is NULL
# what to do if $e is NULL
# what to do if $e is empty
# what to do if $e is valid
# etc.

The approach where you mark different fields with ttt or xxx makes some sense if you want to be able to quickly upgrade the code to support additional fields that need to be validated like the already supported fields. Is this the case? Even then splitting and concatenating and splitting again seems overcomplicated.

0

Can anyone put me on the right track with this?

As always, with bash scripts that don't quite work, I run them through ShellCheck – shell script analysis tool.

Your script generates the following errors/warnings:

$ shellcheck myscript

Line 23: if [ -n "$st" -a "$ft" ] ; then ^-- SC2166: Prefer [ p ] && [ q ] as [ p -a q ] is not well defined.

Line 45: if [ -n "$b1" -a -n "$b2" ] ; then ^-- SC2166: Prefer [ p ] && [ q ] as [ p -a q ] is not well defined.

Line 49: if [ -n "$tz1" -a -n "$tz2" ] ; then ^-- SC2166: Prefer [ p ] && [ q ] as [ p -a q ] is not well defined.

Line 62: input="$((IFS=, read -r b c d e ; echo "${b}ttt,${c}ttt,${d}xxx,${e}www" )<<<"$input")" ^-- SC1102: Shells disambiguate $(( differently or not at all. For $(command substition), add space after $( . For $((arithmetics)), fix parsing errors.

Line 64: IFS=,; set -f; set --$input; out= ^-- SC2086: Double quote to prevent globbing and word splitting.

Did you mean: (apply this, apply all SC2086) IFS=,; set -f; set --"$input"; out=

Line 125: echo "" >> /tmp/crew.txt ^-- SC2129: Consider using { cmd1; cmd2; } >> file instead of individual redirects.

Line 130: let a++ ^-- SC2219: Instead of 'let expr', prefer (( expr )) .

$

I suggest you take a close look at the messages output for lines 62 and 64.

Note that if you run the script yourself through ShellCheck you will see that the output includes clickable links to more information about the various messages produced.

DavidPostill
  • 162,382