3
\$\begingroup\$

I am a beginner and have created this bash script that helps me with creating bash scripts.

The script shows no issues on shellcheck.net

The code can be found here. https://github.com/plutesci/Iwanttobash/blob/master/Icode.bash

I would like some feedback from more experienced users. Any suggestions or thoughts about this.

#!/bin/bash 
# This Program is to Show user the commands and or to create a bash script
# to execute this program type at terminal ./ICode.bash
# I am attempting to have the program open the named file with the header 
# shebang at the top #!/bin/bash as it is not there you will need to add  it.
# created by plutesci for self or public use.

clear
# Banner shows a banner to install should it not show on Ubuntu is.
# sudo apt-get install sysvbanner
# banner "Icode"
# figlet is a banner aswell to install that just type in your terminal
# sudo apt-get install figlet
figlet "Icode"
echo Showing a way to use the command line steps to start a Hello World   program
echo Where you see test.bash, Please rename test.bash
echo Follow Steps Below... 
echo ####################
echo "echo '#!/bin/bash' > test.bash " #change test
echo "'echo Hello World' >> test.bash " # change test.bash
echo "chmod 755 test.bash"  # change test.bash
echo "nano test.bash"   # change test.bash
echo "Exit Ctrl c" 
echo ###################
#echo '#!/bin/bash' >> "$text.bash"

# I want to add an option to ask user if they want to start a new bash \
# program yes \ no? 
# if yes\Yes query user for new name, What to call this bash ? ...... 
# I am happy with the the selection part but would like it going into a      menu 
# afterwards with press 9 return to previous menu

######## Things to improve on.

# when playing i found a program called openvt, It would be great to have the 
# bash program open virtual termin and run the output 
echo -n "Please Enter a Filename For Your New Bash Script > "
read -r text 
echo "Your New Bash Script is named: $text"

# chmod +x $text.bash

# I need to figure out how to add .bash to the $text varable 

selection=
until [ "$selection" = "0" ]; do
echo "
ICODE MENU
1 - Start a new Bash Script 
2 - Make it Executable
3 - Run Bash Program
4 - Continue working on script
5 - Start python idle
6 - Open a new shell
7 - Create Automatic Shell scripts
8 - Delete the script
9 - Help
0 - Exit Program
"
    echo -n "Enter selection: "
    read -r selection
    echo ""
    case $selection in
    1 ) echo "#!/bin/bash" > "$text.bash" ; nano "$text.bash"  ;;
    2 ) chmod 755 "$text.bash" ;;
    3 ) gnome-terminal ; echo "./$text.bash" ;; 
    4 ) nano "$text.bash" ;;
    5 ) idle ;;
    6 ) gnome-terminal ;;
    7 ) nano ;; # would be something like grep a special crafted document
    8 ) echo "#!/bin/bash" > "$text.bash" ;;
    9 ) cat icodehelp ;;
    0 ) exit ;;
    * ) echo "Please enter 1, 2, 3, 4, 5, 6, 7, 8, 9, or 0"
    esac
done

# Things I would like to add to the program would be a auto statment maker 
# and auto create bash programs like open the randomly grep for certain 
# keywords, and random cut from a libary of scripts 10 - 100 scripts
# run it in a virtual terminal, looking for errors and print out example
# 3 out of 100  run with no errors, would you like to view these?
# 17 out of 100 return a slight error, would you like to inspec these.
# 83% percent should be removed. something like that just visualising it   out

# My first bash application would like any help, still trying to learn how 
# github works 
\$\endgroup\$
  • \$\begingroup\$ Changing the code under review is not a good thing on this site. Since noone answered yet it seems to be allowed, but generally you should open a new question with a new version of the code. codereview.stackexchange.com/help/someone-answers \$\endgroup\$ – chicks Jul 20 '18 at 13:11
2
\$\begingroup\$

Good things

  • lots of comments are good.
  • clear keeps things clean when you start
  • This is an interesting way to learn shell scripting. My friends would call it "very meta" which is high praise indeed.
  • I usually tell people they should have run their code through https://www.shellcheck.net/ before worrying about a review. Amazingly enough you didn't trip any of the checks over there so: try it on your next shell project. Or read their docs to see what to avoid in the future.

Suggestions

  • In your lines echo #################### all of the hashes (####################) are completely superfluous. As you have it or simply writing echo will do the same thing. Just reading the code it implies that the hashes are getting output, which they're not since the shell views it as one big comment. You could put the hashes in quotes and then the shell would not "remove" it from the output.
  • Indenting most of the stuff inside the until is good. It'd be nice to do the same thing for the case statement. Your compact layout actually works in this case so I wouldn't mind seeing:

    case $selection in
        1 ) echo "#!/bin/bash" > "$text.bash" ; nano "$text.bash"  ;;
        2 ) chmod 755 "$text.bash" ;;
        3 ) gnome-terminal ; echo "./$text.bash" ;; 
        4 ) nano "$text.bash" ;;
        5 ) idle ;;
        6 ) gnome-terminal ;;
        7 ) nano ;; # would be something like grep a special crafted document
        8 ) echo "#!/bin/bash" > "$text.bash" ;;
        9 ) cat icodehelp ;;
        0 ) exit ;;
        * ) echo "Please enter 1, 2, 3, 4, 5, 6, 7, 8, 9, or 0"
    esac
    

    but it would look more like most people write shell scripts if you spaced it out like:

    case $selection in
        1 ) 
            echo "#!/bin/bash" > "$text.bash"
            nano "$text.bash"
            ;;
        2 )
            chmod 755 "$text.bash"
            ;;
        3 )
            gnome-terminal
            echo "./$text.bash"
            ;;
        4 )
            nano "$text.bash"
            ;;
        5 )
            idle
            ;;
        6 )
            gnome-terminal
            ;;
        7 )
            nano
            ;; # would be something like grep a special crafted document
        8 )
            echo "#!/bin/bash" > "$text.bash"
            ;;
        9 )
            cat icodehelp
            ;;
        0 )
            exit
            ;;
        * )
            echo "Please enter 1, 2, 3, 4, 5, 6, 7, 8, 9, or 0"
    esac
    

    That's obviously take up three times as many lines, but if you decide to add more to any of the sections it will remain readable.

  • Much of your code is outputting blocks of text. Rather than doing the echo after echo thing it is easier to scale this to multiple lines of text at once with a "Here Document". To replace your block of echos after figlet you could do:

    cat <<-EXAMPLE
    Showing a way to use the command line steps to start a Hello World   program
    Where you see test.bash, Please rename test.bash
    Follow Steps Below... 
    
    echo '#!/bin/bash' > test.bash 
    'echo Hello World' >> test.bash
    chmod 755 test.bash
    nano test.bash
    Exit Ctrl-c
    
    EXAMPLE
    

    This will do the same thing as what you already had, but it avoids needing to worrying about the quoting on each line. It does remove the ability to comment within it, but usually that's a smaller issue than keeping straight whether you are in the text to be output or in scripting code. By including the - in <<- we are saying to ignore initial tabs in the Heredoc content. The tabs get removed on the fly and not exposed to the end user. This lets you use it inside the until to indent the whole cat and keep everything looking neat.

  • You could avoid the need for banner or figlet by the users of your script if you took the output of them and saved it into your script, probably in a Heredoc. Since you're not passing anything variable into them, their output won't change and so "caching" a static version won't reduce any existing functionality.

    cat <<'ICODE'
     ___              _      
    |_ _|___ ___   __| | ___ 
     | |/ __/ _ \ / _` |/ _ \
     | | (_| (_) | (_| |  __/
    |___\___\___/ \__,_|\___|
    ICODE
    

    In this case the Heredoc starts with single quotes (<<'ICODE') so that the contents are passed through without any values being replaced.

  • Your option 9 for help could be done without an external file. Typically this would be done by defining a function and then calling it inside your case statement. Inside the function could simply be a cat with a Heredoc. But doing that means you don't have to worry about the help file getting lost.

Ideas

  • If you like figlet try cowsay! :=)
  • Try reading the bash man page a few times. It is long and arduous, but full of lots of useful info if you can make it.
  • https://kvz.io/blog/2013/11/21/bash-best-practices/ has some easier to digest suggestions. Looking at your menu section I was really hoping to find a set -x so we could see the commands running too. This may not fit your UX, but it is worth trying to see how tracing works.
\$\endgroup\$
  • 1
    \$\begingroup\$ This is awesome, I really appreciate your time and effort. Thank you very much. I will be taken all your advice into account, and applying it to all my future projects also reading all the extra documentation you have provided. Tried to vote this up but I am bit shy of the rep needed. \$\endgroup\$ – plutesci Jul 20 '18 at 15:00

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

Not the answer you're looking for? Browse other questions tagged or ask your own question.