Bash plugin to setup project for books

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 ..

Single responsibility principle

The input validating functions do two things:

• Read input (when invoked without parameters)

• 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.

StackExchange Code Review Q#105206, answer score: 5