new to Bash - keep getting Illegal option error

Question

I'm quite new to ubuntu and bash scripting and wanted to know why I might be getting this error when using GETOPTS.

here is the code I use to run it.

sh /home/ubuntu/Desktop/test.sh -f /home/u/Desktop/ -p 'TEST'

I think i'm calling the script correctly, and it should search for the term I enter as a search term using grap. but for some reason it doesn't. Any advice on what I can do as a general rule when working with grep would also be appreciated, thanks.

#!/bin/bash

valid=0
file_arg=""
display_help=""
column=""
pattern=""

while getopts f:d:s:m: opt
do
    case "$opt" in
        d)  display_help=$OPTARG
            ;;
        f)  file_arg=$OPTARG
            ;;
        c)  column=$OPTARG
            ;;
        p)  pattern=$OPTARG
            ;;
        *)  valid=1
            break
            ;;
    esac
done

if [ $valid -eq "0" ]
then
    if [ $pattern != "" ]
    then
        cat $file_arg | grep $pattern
    else
        cat $file
    fi
else
    echo -n "Usage: FILE -f <name> | COLUMN -> -c <name> | HELP -> -d | PATTERN -> -p <expression>"
fi

Answer 1

In getopts you not specify p option you only have f:d:s:m: options.

I think you mean p instead m or vice versa.

It should f:d:s:m:p: or f:d:s:p:

Answer 2

You should also consider the error supression and error handling features of getopts.

If the very first character of the option string is a colon (:) then getopts will not report errors and instead will provide a means of handling the errors yourself. Two additional characters can then be used within your case conditional handling:

• ? If an invalid option is entered then $opt will be set to ? and $OPTARG will hold the invalid character, e.g. if -z was used, which is not in your option string, then $OPTARG will be set to z.

• : If a required additional argument is omitted by the user then $opt will be set to : and $OPTARG will hold the command character, e.g. if -p was used instead of -p arg then $OPTARG will be set to p.

If this is implemented then the catch-all of * becomes redundant and should be removed. Note: If you leave it in and it is above either ? or : then you'll be asking for problems. Also make sure the ? is escaped like this \?).

Hope this helps.

# Note the addition of the inital colon before 'f'.
while getopts :f:d:c:p: opt;
do
    case $opt in
        d)  display_help=$OPTARG
            ;;
        f)  file_arg=$OPTARG
            ;;
        c)  column=$OPTARG
            ;;
        p)  pattern=$OPTARG
            ;;

        # Option error handling.

        \?) valid=0
            echo "An invalid option has been entered: $OPTARG"
            ;;

        :)  valid=0
            echo "The additional argument for option $OPTARG was omitted."
            ;;

        # This is now redundant:
        # *)  valid=0
        #     break
        #     ;;
    esac
done

Answer 3

There are a couple of other issues with your script, as Jayesh mentioned, you need to include all parameters for getopt but you also need to be careful with string comparisons, here's a couple more fixes with suggestions:

(See http://www.tldp.org/LDP/abs/html/comparison-ops.html for string comparison info)

#!/bin/bash

# switch around valid, convention is 1 == true and 0 == false
valid=1
file_arg=""
display_help=""
column=""
pattern=""

# getopt patterns need to match following case statement
while getopts f:d:c:p: opt;
do
    case $opt in
        d)  display_help=$OPTARG
            ;;
        f)  file_arg=$OPTARG
            ;;
        c)  column=$OPTARG
            ;;
        p)  pattern=$OPTARG
            ;;
        *)  valid=0
            break
            ;;
    esac
done

# changed value to reflect true false convention
if [ "$valid" -eq "1" ]
then
    # string comparison in bash should be done using specific operators
    if [ -n "$pattern" ]
    then
        cat $file_arg | grep $pattern
    else
        # typo, this should be file_arg?
        cat $file_arg
    fi
else
    echo -n "Usage: FILE -f <name> | COLUMN -> -c <name> | HELP -> -d | PATTERN -> -p <expression>"
fi