Revised media ingest script

Question

link to original question: Simple media ingest script

As per answers I have refactored my code and would like to get feedback on the current structure.

The purpose of this tool is to allow someone to copy media from cards (SD cards, CFast, etc) using rsync and to also allow someone to complete incomplete transfer that they have initiated.

I've chosen to use prompts because it's more user friendly for the environment the script will be used in. The intended users are not command line tool friendly.

I'm not very familiar with bash and am quite out of practice with code in general so I would like to check behind myself. I received excellent feedback in the previous post and have addressed several of the points raised.

#!/usr/local/bin/bash
set -e
set -u
# prompt user for input
prompt_user(){
 while [[ -z "${!2:-}" ]]
 do
 read -r -p "1ドル" "2ドル"
 done
}
# prompt user for additional rsync options
add_rsync_options(){
 while true;
 do
 read -r -p "Add rsync options? [y/n] " add_options
 case $add_options in
 [Yy]* )
 read -r -p $'Enter additional rsync options:\n' rsync_options
 break;;
 [Nn]* )
 break;;
 *) echo "Please enter y or no!"
 esac
 done
}
# make target directory for transfer
make_directory(){
 echo $'Follow the prompt to create a project directory.\n'
 prompt_user $'Path of target directory?\n' target_directory
 prompt_user $'Brand Prefix?\n' brand_prefix
 prompt_user $'Project Name?\n' project_name
 prompt_user $'Media Type?\n' media_type
 prompt_user $'Location?\n' location
 prompt_user $'Employee?\n' employee
 destination_path=${target_directory}/$(date +'%Y%m%d')_${brand_prefix}_${project_name}_${media_type}_${location}_${employee}
 echo "Creating directory: ${destination_path}"
 mkdir -p "${destination_path}" "${destination_path}/logs"
}
# run rsync command
run_rsync(){
 echo $'Follow the prompt to complete the rsync command.\n'
 prompt_user $'Path to source media?\n' source_path
# if partial ingest indicate pre-existing target directory
 if [[ "$option" == "2" ]]; then
 prompt_user $'Target directory?\n' target_directory
 destination_path=$target_directory
 fi
 add_rsync_options
 echo $'Running rsync command:\n'
 rsync -r --info=progress2 --log-file="${destination_path}/logs/$(date +'%Y%m%d')_transfer_log.txt" ${rsync_options:-} "${source_path}/" "${destination_path}"
}
# clear terminal for readability
clear
# start
while true
do
# read user input and run appropriate functions
 read -r -p $'Enter [1] to start an ingest or [2] to complete a partial ingest.\n' option
 case $option in
 1 )
 make_directory
 run_rsync
 break;;
 2 )
 run_rsync
 break;;
 * )
 echo $'Please enter a valid option!\n';;
 esac
done

Error handling is at a minimum still I believe and I'm not too sure what would even be relevant to handle past what's implemented. I'm not very experienced in that regard so resources are appreciated!

Thanks!

Answer 1

What is $option in run_rsync?

option is read in the global scope to decide if user wants a full ingest or continue a partial ingest. In that loop that reads the prompt, the meaning of option is understandable.

Referring to $option in run_rsync is just too far away. It's not clear anymore where it comes from and what it means. It would be better if the prompt loop passed the decision to run_rsync as a parameter, to make it perfectly clear. For example:

while true
do
 read -r -p $'Enter [1] to start an ingest or [2] to complete a partial ingest.\n' option
 case $option in
 1 )
 make_directory
 run_rsync full
 break;;
 2 )
 run_rsync partial
 break;;
 * )
 echo $'Please enter a valid option!\n';;
 esac
done

And then in run_rsync:

run_rsync() {
 local ingestType=1ドル
 echo $'Follow the prompt to complete the rsync command.\n'
 prompt_user $'Path to source media?\n' source_path
 if [[ "$ingestType" = "partial" ]]; then
 prompt_user $'Target directory?\n' target_directory
 destination_path=$target_directory
 fi
 # ...

Inconsistent terminology

In make_directory, the term "target directory" refers to the base directory in which a timestamped sub-directory will be created, with brand name, project name, and so on, also in the name.

In run_rsync, in case of partial ingest the user is prompted for "target directory", but here it will mean the full destination path. If would be better to name it as such.

Inconsistent formatting

The code uses inconsistent indentation. For example the content of prompt_user is over-indented, compared to other functions. The body of the while loop in the global scope is over-indented, and also haphazardly indented. It would be easier to read if indenting was consistent throughout.

Unnecessary comments

Most comments in the program state the obvious. They are just noise, and it would be better to remove them.

• usulmuaddib: Excellent points! Thanks for the detailed response. Passing the ingest type to the function makes a lot of sense and did not occur to me at all.