Bash plugin to setup project for books

Question

I'm writing a bash plugin to help people create books in a specific way. It's an ordinary shell script to setup the project and create its subresources. I'm using a loop to create barebones assets and then seek to checks-in the project into a repository as well.

The plugin will eventually provide simple commands such as $ bookiza insert [insert_at] [number of pages], $ bookiza remove [page_no] to do a few repetive tasks for an author and give a little control over the project.

Here's what the first cut of my bash script looks like:

# -------------------------------------------------#
# SUPERBOOK WRITERS #
# -------------------------------------------------#
# Add `~/bin` to the `$PATH`
export PATH="$HOME/bin:$PATH";
# TODO: Split methods below into logical files and include alongside `$PATH`.
for file in ~/bookiza/.{path}; do
 [ -r "$file" ] && [ -f "$file" ] && source "$file";
done;
unset file;
# ---------- BOOKIZA INITIALIZER --------- #
bookiza() {
 case "1ドル" in
 new)
 new "$@"
 ;; 
 insert)
 insert "$@"
 ;; 
 add)
 add "$@"
 ;; 
 remove)
 remove "$@"
 ;;
 length)
 cd "manuscript"
 getLength
 cd ".."
 ;;
 server)
 stop
 serve
 ;; 
 check)
 check
 ;; 
 help)
 help 
 ;; 
 *)
 echo $"Usage: 0ドル { new | insert | length | remove | server | check | help}"
 echo $"Try: $ bookiza help"
 esac
}
#--------- NEW PROJECT ---------#
new() {
 args=("$@")
 echo Number of arguments passed =: $#
 echo "Type: ${args[0]}, Project: ${args[1]} Booklength: ${args[2]}"
 PROJECTNAME=${args[1]}
 if [ ${PROJECTNAME:+x} ] ; then
 echo "Proceeding ........"
 else 
 echo "Halting ..........."
 validateProjectName $PROJECTNAME
 fi
 setupProject $PROJECTNAME
 BOOKLENGTH=${args[2]}
 if [ ${BOOKLENGTH:+x} ] ; then
 echo "Proceeding ........"
 validateNumeric $BOOKLENGTH
 else 
 echo "Halting ..........."
 validateBookLength $BOOKLENGTH
 fi
 createPages $BOOKLENGTH
 setupGitRepository 
}
###### Validations ######
validateProjectName() {
 if [[ $# -eq 0 ]] ; then
 echo "Project name not supplied. (HINT: My-New-Book-Name i.e. use hypens!)"
 read PROJECTNAME
 if [ ${PROJECTNAME:+x} ] ; then
 return 
 else 
 echo "Halting ..."
 validateProjectName $PROJECTNAME
 fi
 exit
 fi 
}
validateBookLength() {
 if [[ $# -eq 0 ]] ; then
 echo "Book length not supplied. (HINT: Must be even number i.e. 6, 12, 24!)"
 read BOOKLENGTH
 if [ ${BOOKLENGTH:+x} ] ; then
 validateNumeric $BOOKLENGTH
 return $BOOKLENGTH
 else 
 echo "Halting ..."
 validateBookLength $BOOKLENGTH
 fi
 exit
 fi 
}
validateNumeric() {
 BOOKLENGTH=1ドル
 reg='^[0-9]+$'
 if ! [[ $BOOKLENGTH =~ $reg ]] ; then
 echo "Error: Argument not a number, try again:" >&2;
 read BOOKLENGTH
 validateNumeric $BOOKLENGTH
 else
 validateEven $BOOKLENGTH
 fi
}
validateEven() {
 BOOKLENGTH=1ドル
 echo "Testing if ${BOOKLENGTH} is even now"
 if [ $((BOOKLENGTH%2)) -eq 0 ] ; then
 echo "Ok ....... Proceeding"
 echo "Setting book length = $BOOKLENGTH"
 return $BOOKLENGTH
 else
 echo "Error: Not an even number, try again:" >&2;
 read BOOKLENGTH
 validateEven $BOOKLENGTH
 fi 
}
setupProject() {
 echo "Setting up $PROJECTNAME now ..."
 mkdir -p "1ドル" && cd "1ドル" && touch README.md license.txt .gitignore && mkdir "trash" "cover" "templates" "images" "manuscript" || return $?
 echo "# 1ドル" >> README.md
 cd "templates" && touch template.html head.html template.css template.js && cd ".."
}
createPages() {
 PAGES=1ドル
 cd "manuscript"
 p=0
 while [ "$p" -lt "$PAGES" ]; do
 p=$((p+1))
 mkdir -p "page-$p"
 cd "page-$p"
 touch "body.html"
 touch "style.css"
 echo "body{background:rgba(200, 235, 255, 0.99); margin:0 0; overflow:hidden;}" >> style.css
 cd ".."
 done 
 echo "Done!" && cd ".." #Head back to root
}
setupGitRepository() {
 git init
 git add . -A
 git commit -am "First commit: Setup new book project" --quiet
 echo "Provide GITHUB URL:"
 read REPO_URL
 if [ ${REPO_URL:+x} ] ; then
 git remote add origin "$REPO_URL"
 git push -u origin master
 echo "Project ready! Stacked ${PAGES} blank pages inside /manuscript correctly." 
 else
 echo "Error: Argument not supplied, try again!"
 read REPO_URL
 fi
}
# ---------- INSERT PAGES --------- #
insert() {
 args=("$@")
 echo Number of arguments passed = $#
 echo Type: ${args[0]}, INSERT_AT: "${args[1]}", [ No. of pages: "${args[2]}"]
 INSERT_AT=${args[1]}
 if [ ${INSERT_AT:+x} ] ; then
 echo "Ok ........ Proceeding"
 validateNumericalInsertAt $INSERT_AT
 else 
 echo "Halting ..........."
 validateInsertAt $INSERT_AT
 fi
 NUMBER_OF_PAGES=${args[2]}
 if [ ${NUMBER_OF_PAGES:+x} ] ; then
 validateNumberOfPages $NUMBER_OF_PAGES
 else
 NUMBER_OF_PAGES=2
 fi
 generatePages $INSERT_AT $NUMBER_OF_PAGES
 echo "DONE:)"
}
validateInsertAt() {
 if [[ $# -eq 0 ]] ; then
 echo "INSERT_AT: not supplied. (HINT: Must be integer!)"
 read INSERT_AT
 if [ ${INSERT_AT:+x} ] ; then
 validateNumericalInsertAt $INSERT_AT
 return $INSERT_AT
 else 
 echo "Halting ..."
 validateInsertAt $INSERT_AT
 fi
 exit
 fi 
}
validateNumericalInsertAt() {
 INSERT_AT=1ドル
 reg='^[0-9]+$'
 if ! [[ $INSERT_AT =~ $reg ]] ; then
 echo "Error: Argument not a valid number, try again:" >&2;
 read INSERT_AT
 validateNumericalInsertAt $INSERT_AT
 else
 return $INSERT_AT
 fi
}
validateNumberOfPages() {
 NUMBER_OF_PAGES=1ドル
 reg='^[0-9]+$'
 if ! [[ $NUMBER_OF_PAGES =~ $reg ]] ; then
 echo "Error: Argument not a valid number, try again:" >&2;
 read NUMBER_OF_PAGES
 validateNumberOfPages $NUMBER_OF_PAGES
 else
 return $NUMBER_OF_PAGES
 fi
}
generatePages() {
 args=("$@")
 echo INSERT_AT: "${args[0]}", [ Number of pages: "${args[1]}" ]
 p="$INSERT_AT"
 cd "manuscript"
 if [ -d "page-$p" ]; then
 getLength
 q="$BOOKLENGTH"
 while [ "$q" -ge "$p" ]; do
 mv "page-$((q))" "page-$((q+NUMBER_OF_PAGES))"
 q=$((q-1))
 done
 q=0
 while [ "$q" -lt "$NUMBER_OF_PAGES" ]; do
 mkdir -p "page-$((p+q))"
 cd "page-$((p+q))"
 touch "body.html"
 touch "style.css"
 q=$((q+1))
 cd ".."
 done
 fi
 cd ".."
 getLength
}
# ---------- ADD PAGES --------- #
add() {
 args=("$@")
 echo Number of arguments passed =: $#
 echo "Type: ${args[0]}, Number of pages: ${args[1]}"
 cd "manuscript"
 NUMBER_OF_PAGES=${args[1]}
 if [ ${NUMBER_OF_PAGES:+x} ] ; then
 validateNumberOfPages $NUMBER_OF_PAGES
 else
 NUMBER_OF_PAGES=2
 fi
 getLength
 echo "SHOUT AT ME = ${NUMBER_OF_PAGES}"
 q=1
 while [ "$q" -le "$NUMBER_OF_PAGES" ]; do
 mkdir -p "page-$((BOOKLENGTH+q))"
 cd "page-$((BOOKLENGTH+q))"
 touch "body.html"
 touch "style.css"
 q=$((q+1))
 cd ".."
 done
 getLength
 cd ".."
}
remove() {
 args=("$@")
 echo Number of arguments passed =: $#
 echo "Type: ${args[0]}, Page number: ${args[1]}"
 cd "manuscript"
 PAGE_NO="${args[1]}"
 if [ ${PAGE_NO:+x} ] ; then
 trashPage $PAGE_NO
 else
 validatePageNo $PAGE_NO
 fi
 getLength
 cd ".."
}
trashPage() {
 args=("$@")
 echo PAGE_NO: "${args[0]}", [ Blah: "${args[1]}" ]
 PAGE_NO="${args[0]}"
 if [ -d "page-$PAGE_NO" ]; then
 mv "page-$PAGE_NO" "../trash/page-$PAGE_NO"
 m="$PAGE_NO"
 echo "Starting directory reduction"
 m=$((m+1))
 until [ ! -d "page-$m" ]; do
 mv "page-$((m))" "page-$((m-1))"
 m=$((m+1))
 done
 fi
}
# ---------- BOOKLENGTH --------- #
getLength() {
 BOOKLENGTH=1
 echo "Measuring book length ..."
 until [ ! -d "page-$((BOOKLENGTH+1))" ]; do
 BOOKLENGTH=$((BOOKLENGTH+1))
 done
 echo Book length is: $BOOKLENGTH pages
 return
}
# ---------- LOCAL SERVER --------- #
serve() {
 echo "Server node server here?"
}
# ---------- LOCAL SERVER --------- #
check() { 
 echo "Check if project has even number of pages" 
}
# ---------- LOCAL SERVER --------- #
help() {
 echo "Help guide here" 
} 

Here's the source on Github.

Before proceeding further into this work I want to have my code evaluated. DRY it up. Any tips or advice on better patterns and/or glaring mistakes (if any)? Given that return $variable doesn't take the value back to the function calling it, is it appropriate to write return $variable (readability) at all?

Answer 1

Single responsibility principle

The input validating functions do two things:

1. Read input (when invoked without parameters)
2. Validate input

It would be better to make a function do just one thing

Faulty input validation

The input validating functions don't validate at all when you pass parameters to them. As such these function names are misleading.

It seems you rely on that for example when calling validateProjectName $PROJECTNAME where the value of $PROJECTNAME is empty, then inside the function $# will have 0 as the value. This usage is misleading. If I rewrite the call as validateProjectName "$PROJECTNAME" then it will happily return from it (= valid project name), which is not right.

Furthermore, notice the duplicated validation logic: the half-baked [[ $# -eq 0 ]] is intended as the [ ${PROJECTNAME:+x} ]. The two tests look different but are designed for the same thing. It's a duplication that can be eliminated.

Broken input validation

If a parameter is not supplied, this function will behave very strange:

validateProjectName() {
 if [[ $# -eq 0 ]] ; then
 echo "Project name not supplied. (HINT: My-New-Book-Name i.e. use hypens!)"
 read PROJECTNAME
 if [ ${PROJECTNAME:+x} ] ; then
 return 
 else 
 echo "Halting ..."
 validateProjectName $PROJECTNAME
 fi
 exit
 fi 
}

It will keep asking for input until you give something to it, in each step printing "Halting...". When finally you enter something non-empty, the script will halt. Why doesn't it halt immediately then? This looks like a bug.

Pattern: input reading with retries

It's a common pattern to retry reading input until user enters something valid. But it's not common and not recommended to use recursion to repeat, because it can be confusing, and with enough failures a stack overflow can occur.

Consider this alternative:

isValidProjectName() {
 test "1ドル"
}
readValidProjectName() {
 PROJECTNAME=1ドル
 while ! isValidProjectName "$PROJECTNAME"; do
 echo "Please enter project name (HINT: My-New-Book-Name i.e. use hyphens!)"
 read PROJECTNAME
 done
}
readValidProjectName "$PROJECTNAME"

Points of interest:

• Each function has precisely one purpose
• Repeatedly asks for input in a loop until valid
• The loop condition in readValidProjectName uses the exit code of isValidProjectName
• The emptiness check is simplified by simply enclosing the variable within "..."

I suggest to follow this pattern in your other functions too.

Encapsulation

In the bookiza function, most actions call a dedicated function, which is nice, except length, in which the main task is wrapped within a cd "manuscript"; ...; cd ... The fact that it needs to go inside "manuscript" is an implementation detail that would be nicer to hide. I suggest to move these details inside a function, to handle this action uniformly like the others.

Moving into sub-directories

In general it's not recommended to change directories in scripts. It's easy to make a mistake, and if something goes wrong, the script might end up in the wrong directory.

Another issue with wrapping commands within a cd $somewhere; ...; cd .. is that if ever $somewhere will be more than one level deep, you'll have to remember to update cd .. accordingly.

A better and simpler way is to wrap within (...), for example:

(cd manuscript; getLength)

Notice that there's no cd .. at the end. There's no need: when the (...) exits, the script will be back in its original working directory.

Another alternative is using pushd and popd:

pushd manuscript; getLength; popd

Unnecessary double-quotes

It's good practice to double-quote path variables. On the other hand, when a directory name is a literal string with no special characters, then quoting is unnecessary, for example:

cd "manuscript"
cd ".."

These can be simply:

cd manuscript
cd ..

Minor things

Although your writing style is pretty clean, ShellCheck does pick up a few issues. It's a great site, I suggest copy-pasting your shell scripts in there to catch common mistakes and bad practices.