Why is my script calling multiple functions? [duplicate]

Question

I have a few sets of scripts that I've written to make setting up linux very simple. So I have made those scripts into separate functions instead and put it all in one script. I have numbers set so when the user inputs one of the numbers, it will call that function. However, when the user inputs one number, the script calls ALL the functions:

n=1
#note you need spaces between [ ]
while [ $n == 1 ]
do
    echo "Base install=1"
    echo "Mintrepos=2"
    echo "Asusn13 driver=3"
    echo "Install Standard openbox=4"
    echo "Install Xfceopenbox=5"
    echo "backing up standard open box config=6"
    echo "backing up openboxfce=7"
    echo
    read choice
    if [ "$choice"==1 ]
    then
        baseinstall
    fi
    if [ "$choice"==2 ]
    then
        linuxmintrepos
    fi
    if [ "$choice"==3 ]
    then
        asusn13driver
    fi
    if [ "$choice"==4 ]
    then
        standardopenbox
    fi
    if [ "$choice"==5 ]
    then
        xfceopenbox
    fi
    if [ "$choice"==6 ]
    then
        backupstandopenbox
    fi
    if [ "$choice"==7 ]
    then
        backupxfc4openbox
    fi
    echo "You installed: "
    if [ "$choice"==1 ]
    then
        echo "baseinstall"
    fi
    if [ "$choice"==2 ]
    then
        echo "linuxmintrepos"
    fi
    if [ "$choice"==3 ]
    then
        echo asusn13driver
    fi
    if [ "$choice"==4 ]
    then
        echo "standardopenbox"
    fi
    if [ "$choice"==5 ]
    then
        echo "xfceopenbox"
    fi
    if [ "$choice"==6 ]
    then
        echo "backupstandopenbox"
    fi
    if [ "$choice"==7 ]
    then
        echo "backupxfc4openbox"
        echo "If you would like to keep going, type in 1, if not, type in any other number"
    read n
    fi

done

Answer 1

Replace:

if [ "$choice"==1 ]

With:

if [ "$choice" = 1 ]

Do the same for all subsequent tests.

Explanation

When the shell sees "$choice"==1, it sees one string. If choice was, for example, 2, then it sees the string 2==1. This is not an equality test. It is a string. Since it is a non-empty string, the test returns true. Hence all choices are executed.

In order to recognize the equality test, spaces are needed.

Separately, and as a minor point, for [ style tests, the symbol for equality is =. bash will accept == for that but it is not correct and won't work in other shells.

Exploring [...] tests on the command line

It is easy to explore how [ tests work on the command line. First, let's demonstrate that an empty string tests false:

$ if [ "" ]; then echo True; else echo False; fi
False

A non-empty string, whatever it is, is true:

$ if [ abc ]; then echo True; else echo False; fi
True

Now, let's look at 2==1:

$ if [ 2==1 ]; then echo True; else echo False; fi
True

Since 2==1 is a non-empty string, it tests true.

Now, let's add spaces and do equality tests:

$ if [ 2 = 1 ]; then echo True; else echo False; fi
False
$ if [ 2 = 2 ]; then echo True; else echo False; fi
True

As an additional subtlety, = tests for string equality which happens to work fine here. To test for numeric equality, use -eq instead.

$ if [ 2 -eq 2 ]; then echo True; else echo False; fi
True

Answer 2

As others have mentioned, your if test ...;fi statements are syntactically incorrect, but also worth noting is your entire approach is a little rough. There are a couple of areas in which the flow of this script could be drastically improved - specifically in that it will be easier to read and edit later.

First, you can set your selections iteratively in a for loop. In this way any possible choice will be announced to the user and its selector automatically generated, but within the script you can lump them all together in a list of strings without worrying about either:

set --
for c in \
    "Base install:base_fn"                             \
    "Mintrepos:mint_fn"                                \
    "Asusn13 driver:asus_fn"                           \
    "Install Standard openbox:std_obox_fn"             \
    "Install Xfceopenbox:xfce_obox_fn"                 \
    "backing up standard open box config:bkup_fn obox" \
    "backing up openboxfce:bkup_fn xfce"              
do
    set -- "$@" "${c##*:}"
    printf "'%s' = $#\n" "${c%:*}"
done

This encodes the called fn_name into the positional parameter array while simultaneously printing the user friendly description to stdout.

Once you've built your list of selections into "$@" and printed the progress while doing so, you can then read the user's selection in a small loop that will quit after too many failed tries while also verifying its input:

chk=$((($#<1)*5))                           #if not at least one choice quit
until [ "$((chk+=1))" -gt 5 ] && exit 1     #chks up to 5 times or quits
printf '\nSelect: '                         #prompts for each try
read -r c && [ -n "${c##*[!0-9]*}" ]        #fails if input not number or empty
do echo "Invalid selection."; done          #prints a notice and retries

Last you can just shift away any unnecessary parameters and call $1:

shift "$((c-1))"; $1

It is better to verify there are no leading zeroes. shift will just error if the user has selected a number too high.

[ "$((c=$(printf %.d "$c")))" -gt 0 ] || exit 
shift "$((c-1))" && $1

This is for a numeric selection list, which makes the positional array very useful. More generally you might use a case statement:

case "$choice" in
(pattern 1)  do as necessary for a match;;
(pattern 2)  do otherwise for this match;;
(pattern 3)  continue this wise for each;;
(*)          until there isn't a match  ;;
esac 

These are but portable means of emulating what many shells (to include bash) will offer as a select statement as introduced by ksh many years ago.

From man bash:

• select name [ in word ] ; do list ; done
  • The list of words following in is expanded, generating a list of items. The set of expanded words is printed on the standard error, each preceded by a number. If the in word is omitted, the positional parameters (or "$@") are printed (see PARAMETERS below). The $PS3 prompt is then displayed and a line read from the standard input.

Answer 3

You need spaces around the ==. Otherwise, you are passing the string "$choice"==1 itself to if rather than comparing $choice to 1. When [ receives a string, it will evaluate to true as long as the string is non-empty:

$ foo="bar";  [ $foo ] && echo true
true
$ foo="";  [ "$foo" ] && echo true  ## echoes nothing

Outside the test brackets, in the absence of space, the variable would actually get assigned to =1. To illustrate:

$ f==3
$ echo $f
=3
$ [ 10==12 ] && echo yes
yes

As you can see above, f==3 sets the variable $f to =3.

So, in your if block, what is being tested is a non-empty string which always evaluates to true and the if is executed. This would work:

if [ "$choice" == "1" ]
then
    baseinstall
fi

Also, = or == do string comparison in bash. You want arithmetic comparison:

if [ "$choice" -eq "1" ]
then
    baseinstall
fi