Batch or not if/then

Shell Programming and Scripting
1

I am having trouble modifying a .sh that allows the user to run different command based of an input. So based on the user input of Y or N a different command is run. Thank you :).

 #!/bin/bash
while true
do
        printf "Is this a batch  : " ; read id
        [ -z "$id" ] && break
If "$id" = Y, Then
        [ "$id" = "end" ] && break
        perl -ne 'chomp; system ("perl table_annovar.pl $_ humandb/ -buildver hg19 -protocol refGene,popfreq_all,common,clinvar,clinvarsubmit,clinvarreference -operation g,f,f,f,f,f -otherinfo")' < file.txt 
Else
      If "$id" = N, Then 
      printf "Enter ID  : " ; read id
      [ -z "$id" ] && break
      [ "$id" = "end" ] && break
      perl table_annovar.pl ${id}_matched.avinput humandb/ -buildver hg19 -protocol refGene,popfreq_all,common,clinvar,clinvarsubmit,clinvarreference -operation g,f,f,f,f,f -otherinfo
done
2

Your if's and then's are wrong.

if [ "$id" = "Y" ]
then
        echo "do something
elif [ "$id" = "N" ]
then
        echo "do something else"
else
        echo "unknown option"
fi

Note that the extra linebreaks aren't optional. You need either a linebreak or ; between if ... and then

But personally I'd do this with case instead:

case "$id" in
[yY])
        echo "do something"
        ;;
[nN])
        echo "do something else"
        ;;
end)
        break
        ;;
)
        break
        ;;
*)
        echo "Unknown option"
        ;;
esac
3

You ought to pay attention to the casing and the proper syntax of the bash shell.
If is not the same that if
Then is not the same that then
Else is not the same that else
Case matters

If "$id" = Y, Then
        [ "$id" = "end" ] && break

That's not proper syntax for an if condition in bash

The correct syntax could be:
if [ "$id" = Y ]; then

Further more, if the variable $id contains the char Y, how can ever contain the string "end"? Think about that one.

I going to pretend I did not see any of the perl lines.

4

I am doing something wrong, but not familiar enough yet to debug it properly. Thank you :).

 #!/bin/bash
while true
do
        printf "Is this a batch  : " ; read id
        [ -z "$id" ] && break
        [ "$id" = "end" ] && break
case "$id" in
[yY])
        echo "perl -ne 'chomp; system ("perl table_annovar.pl $_ humandb/ -buildver hg19 -protocol refGene,popfreq_all,common,clinvar,clinvarsubmit,clinvarreference -operation g,f,f,f,f,f -otherinfo")' < file.txt"
        ;;
[nN])
        printif "Enter ID : ' ; read id
		[ -z "$id" ] && break
        [ "$id" = "end" ] && break
        ;;
end)
       
	    ;;
)	  

	    ;;
*)
        echo "perl table_annovar.pl ${id}_matched.avinput humandb/ -buildver hg19 -protocol refGene,popfreq_all,common,clinvar,clinvarsubmit,clinvarreference -operation g,f,f,f,f,f -otherinfo"
	    ;;
esac
done 

  bash ~/test.sh
/home/cmccabe/test.sh: line 7: syntax error near unexpected token `$'in\r''
'home/cmccabe/test.sh: line 7: `case "$id" in
5

This particular error means "Stop editing your scripts in Microsoft Notepad". Editing it in a Windows editor has filled them with carriage returns. To repair it:

tr -d '\r' < windowstext > unixtext

Why have you put absolutely everything in the case statement? Most of those statements will never be executed, since it jumps from ;; to esac. Not to mention, you can check for "end" in the same case statement. Try this:

printf "Is this a batch :"
while read id
do
        case "$id" in
        [yY]) echo perl -ne 'chomp; system ("perl table_annovar.pl $_ humandb/ -buildver hg19 -protocol refGene,popfreq_all,common,clinvar,clinvarsubmit,clinvarreference -operation g,f,f,f,f,f -otherinfo")' < file.txt 
                ;;
        [nN]) # code for X
                printf "Enter ID  : " ; read id
                [ -z "$id" ] && break
                [ "$id" = "end" ] && break
                echo perl table_annovar.pl ${id}_matched.avinput humandb/ -buildver hg19 -protocol refGene,popfreq_all,common,clinvar,clinvarsubmit,clinvarreference -operation g,f,f,f,f,f -otherinfo
                ;;
        end)  break ;;
        *)     ;; # Unknown option, do nothing
        esac

        printf "Is this a batch :"
done
6

It's also quite interesting how here, you have a shell script, which calls perl... which calls system (which is another shell!) -- which calls perl again! You could maybe remove two or three levels of code there with no change.

7 
 #!/bin/bash
while true
do
        printf "Is this a batch :"
while read id
do
        case "$id" in
        [yY]) echo perl -ne 'chomp; system ("perl table_annovar.pl $_ humandb/ -buildver hg19 -protocol refGene,popfreq_all,common,clinvar,clinvarsubmit,clinvarreference -operation g,f,f,f,f,f -otherinfo")' < file.txt 
                ;;
        [nN]) # code for X
                printf "Enter ID  : " ; read id
                [ -z "$id" ] && break
                [ "$id" = "end" ] && break
                echo perl table_annovar.pl ${id}_matched.avinput humandb/ -buildver hg19 -protocol refGene,popfreq_all,common,clinvar,clinvarsubmit,clinvarreference -operation g,f,f,f,f,f -otherinfo
                ;;
        end)  break ;;
        *)     ;; # Unknown option, do nothing
		esac
		
		printf "Is this a batch :"
done 

  bash ~/test.sh
/home/cmccabe/test.sh: line 22: syntax error: unexpected end of file

I have to get out of the habit of using a windows editor (I do that a lot), why is there a second

 printf "Is this a batch :"

. The program is written in perl and I am using a shell to make it easier for others to use. Thanks :).

8

The "unexpected end of file" error is because you added a while true; do to the beginning of the script which has no matching done

9

In addition to what Chubbier_XL already said...

If you want to specify that the script is to be run by bash when you don't specify a shell to run your script on the command line, you can't have a space before the #!/bin/bash on the 1st line in your script!

If you get into the habit of indenting code in do, then, else, and selections in cases; mistakes like your missing done would be obvious to you.

1 Like
10

Yes, you, too, deserve beautified code: maintainable and more error free! :smiley:

I have seen people adding comments in SQL 'BEGIN -- {' and 'END --}' so they could use vi's % operator to check balance. Shell do and done, then and fi, case and esac, etc. could use that trick for big entries.

2 Likes
11

Understood. If system() is the very last thing a Perl program does, you might try exec() instead, for some time and resource savings -- perl will replace itself, effectively quitting early, instead of creating a brand new process, and the program which launched it will end up waiting for the new shell.

You could also put exec inside the exec() block informing the shell to do the same thing for further savings -- the process gets replaced twice, the program which launches it ends up just waiting for perl, not shell waiting for perl waiting for shell waiting for perl. The amount of time that saves can be surprising.

That would end up looking something like

#!/bin/sh

...

perl -e 'exec("exec perl ...");'

...

...if that looks torturous, consider: It's actually far more direct than was written :stuck_out_tongue:

1 Like
12

All your sugesstions are valid and will help, but I can not seem where to put the done. If I put it after the

 while read id

to close the first

 do

I get:

 /home/cmccabe/test.sh: line 6: syntax error near unexpected token `done'
/home/cmccabe/test.sh: line 6: `        done'

If I put it before the

 while read id

to close the first

 do

I get: Is this batch repeated. Thanks :).

 #!/bin/bash
while true
	do
        printf "Is this a batch :"
while read id
	done
	do
        case "$id" in
        [yY])  perl -e 'exec("exec perl -ne 'chomp; system ("perl table_annovar.pl $_ humandb/ -buildver hg19 -protocol refGene,popfreq_all,common,clinvar,clinvarsubmit,clinvarreference -operation g,f,f,f,f,f -otherinfo")' < file.txt");' 
                ;;
        [nN]) # code for X
                printf "Enter ID  : " ; read id
                [ -z "$id" ] && break
                [ "$id" = "end" ] && break
				perl -e 'exec("exec perl table_annovar.pl ${id}_matched.avinput humandb/ -buildver hg19 -protocol refGene,popfreq_all,common,clinvar,clinvarsubmit,clinvarreference -operation g,f,f,f,f,f -otherinfo");'
                ;;
        end)  break ;;
        *)     ;; # Unknown option, do nothing
		esac
		
		printf "Is this a batch :"
	done
13

I suggested to replace system() with exec(), not wrap it in yet another block.

And my suggestion wasn't entirely serious. The point was to illustrate how many completely unnecessary processes you are running and how much extra complexity to use something that was supposed to be simple. A better solution would be to strip out most of that code and just write it in shell in the first place.

14 
 #!/bin/bash
        while true
	do
		printf "Is this a batch :"
		
        case "$id" in
        [yY])  echo "perl -ne 'chomp; system ("perl table_annovar.pl $_ humandb/ -buildver hg19 -protocol refGene,popfreq_all,common,clinvar,clinvarsubmit,clinvarreference -operation g,f,f,f,f,f -otherinfo")' < file.txt 
                ;;
        [nN]) # code for X
                printf "Enter ID  : " ; read id
                [ -z "$id" ] && break
                [ "$id" = "end" ] && break
				echo ("perl table_annovar.pl ${id}_matched.avinput humandb/ -buildver hg19 -protocol refGene,popfreq_all,common,clinvar,clinvarsubmit,clinvarreference -operation g,f,f,f,f,f -otherinfo
                ;;
        end)  break ;;
        *)     ;; # Unknown option, do nothing
		esac
		printf "Is this a batch :"
	done

Using the code above "Is this a batch" is repeated no matter in y or n is entered. Thanks :).

15 
 #!/bin/bash
        while true
	do
		printf "Is this a batch :"
		
        case "$id" in
        [yY])  echo "perl -ne 'chomp; system ("perl table_annovar.pl $_ humandb/ -buildver hg19 -protocol refGene,popfreq_all,common,clinvar,clinvarsubmit,clinvarreference -operation g,f,f,f,f,f -otherinfo")' < file.txt 
                ;;
        [nN]) # code for X
                printf "Enter ID  : " ; read id # This has to be outside the case
                [ -z "$id" ] && break # This is not needed, $id will never contain a zero length
                [ "$id" = "end" ] && break # This is not needed. $id will never contain "end", here 
				echo ("perl table_annovar.pl ${id}_matched.avinput humandb/ -buildver hg19 -protocol refGene,popfreq_all,common,clinvar,clinvarsubmit,clinvarreference -operation g,f,f,f,f,f -otherinfo
                ;;
        end)  break ;;
        *)     ;; # Unknown option, do nothing
		esac
		printf "Is this a batch :"
	done

The echo has to go. The red `"' do not have a matching closing on them. They need to go. The rest is explained in the comments.