Bash script menu

Question

Any comments on this?
Like there is a way to get rid of all the if-statements or a better way to # check model and build (which actually just prints the info out, so the comment may be a bit confusing)
feel free to tell me what you think.

MENU_MAIN() { clear PRINT_BANNER_S PRINT_MAIN_MENU # check model and build while read -r line; do [[ $line =~ ^(ro.product.model=) ]] && r="${line#*${BASH_REMATCH[1]}}"$' / Build: '"$r" [[ $line =~ ^(ro.build.version.incremental=) ]] && r="${line#*${BASH_REMATCH[1]}}" done < $ACTIVE_DB/rom/system/build.prop # hu display echo " -> $ACTIVE_DB (Model: $r )" echo -n "db : " if CHECK_DB ; then echo " [YES]" else echo " [NO]" fi echo -n "rom : " if CHECK_SYS ; then if CHECK_SYS_NEMPTY ; then echo -n " [SYS] " fi else echo -n " [NO SYS] " fi if CHECK_DATA; then if CHECK_DATA_NEMPTY ; then echo "[DATA]" fi else echo "[NO DATA]" fi # menu options if CHECK_DB ; then PRINT_LINE3 else PRINT_LINE3 echo "pre - prepare" PRINT_LINE fi if CHECK_DB ; then echo "find - find deps" fi if CHECK_DB ; then if CHECK_DB_SIZE ; then PRINT_LINE3 echo "obj - objects" echo "sym - symbols" echo "dep - dependencies" echo "pro - providings" PRINT_LINE echo "rm - what else can be removed" PRINT_LINE3 else PRINT_LINE3 echo "no content in database" PRINT_LINE3 fi else PRINT_LINE3 fi if CHECK_LOG_NEMPTY ; then echo "log - view logs" if CHECK_DB && CHECK_DB_SIZE ; then PRINT_LINE echo "re - reset db" PRINT_LINE3 else PRINT_LINE3 fi fi echo "man - view manual" echo "set - settings" echo "x - exit" PRINT_LINE3 read -p "CHOICE: " CHOICE case "$CHOICE" in pre) AUTO_PREP ;; mp) PAGE_PREP ;; f|find) CHECK_ALL ;; mf) PAGE_FIND ;; la) ALL_LIST ;; pa) ALL_PRINT ;; o|obj) PAGE_OBJ ;; s|sym) PAGE_SYM ;; d|dep) PAGE_DEP ;; p|pro) PAGE_PROV ;; rm) PAGE_RM ;; l|log) PAGE_LOG ;; m|man) PAGE_MAN ;; re) echo "todo" ;; set) PAGE_SETTINGS ;; x) exit 0 esac } 

---

This is the output

_____________________________________________________________________________________________________________________________ _____________________________________________________________________________________________________________________________ _______________________/\/\__________________________/\/\/\/\/\/\__/\/\/\/\/\____/\/\/\/\/\/\__/\/\/\/\/\/\__________________ _______________________/\/\____/\/\/\____/\/\/\/\________/\/\______/\/\____/\/\__/\/\__________/\/\__________________________ ___________________/\/\/\/\__/\/\/\/\/\__/\/\__/\/\______/\/\______/\/\/\/\/\____/\/\/\/\/\____/\/\/\/\/\____________________ _________________/\/\__/\/\__/\/\________/\/\/\/\________/\/\______/\/\__/\/\____/\/\__________/\/\__________________________ __________________/\/\/\/\____/\/\/\/\___/\/\____________/\/\______/\/\____/\/\__/\/\/\/\/\/\__/\/\/\/\/\/\__________________ _________________________________________/\/\________________________________________________________________________________ _____________________________________________________________________________________________________________________________ ----------------------------------------------------------------------------------------------------------------------------- main menu ----------------------------------------------------------------------------------------------------------------------------- -> DB_45763 (Model: SM-G900F / Build: G900FXXU1BOK6 ) db : [YES] rom : [SYS] [NO DATA] +-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+ find - find deps +-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+ obj - objects sym - symbols dep - dependencies pro - providings ----------------------------------------------------------------------------------------------------------------------------- rm - what else can be removed +-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+ log - view logs ----------------------------------------------------------------------------------------------------------------------------- re - reset db +-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+ man - view manual set - settings x - exit +-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+ CHOICE: 

Answer 1

Possible bug

Consider this code:

echo -n "rom : " if CHECK_SYS ; then if CHECK_SYS_NEMPTY ; then echo -n " [SYS] " fi else echo -n " [NO SYS] " fi if CHECK_DATA; then if CHECK_DATA_NEMPTY ; then echo "[DATA]" fi else echo "[NO DATA]" fi # ... PRINT_LINE3 

The above code is responsible for printing something like this:

 rom : [SYS] [NO DATA] +-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+ 

But it seems that in some cases the output might be broken, in the case when the condition chain on CHECK_DATA doesn't print a newline character. That is, when CHECK_DATA is true but CHECK_DATA_NEMPTY is false, the rom line will not be terminated with a newline character, and the +-+-+ line will be printed starting from the middle of the rom line. It would be better to write like this:

if CHECK_DATA; then if CHECK_DATA_NEMPTY ; then echo -n "[DATA]" fi else echo -n "[NO DATA]" fi echo # ... PRINT_LINE3 

Use printf instead of echo -n

The various flags of echo are not portable, as they may behave differently depending on the system. To print something without a terminating newline, it's safer to use printf.

Maintainability

Two points make the menu difficult to maintain.

When printing the list of commands, the command names are padded with space to align the description. That is, to get this kind of output:

 log - view logs ----------------------------------------------------------------------------------------------------------------------------- re - reset db 

You have this kind of code:

echo "log - view logs" if CHECK_DB && CHECK_DB_SIZE ; then PRINT_LINE echo "re - reset db" # ... 

The problem here is if you ever need to change the padding width, you will have to make parallel changes at many places. In the code snippet above, I actually broke the padding of one of the commands, can you tell me which one? The answer is hardly obvious.

Instead of hard-coded padding in the statements, you could encapsulate the padding logic in a function:

print_command() { command=$1 description=$2 printf '%-8s - %s' $command "$description" } 

And then replace the above statements with:

print_command log 'view logs' # ... print_command re 'reset db' 

The other problem is the duplication of command names when printing the menu and when evaluating the choices. It would be better to put the values in variables and use the variables everywhere instead of hardcoded strings.

Naming

Using all caps for function names is a bit unusual. All caps are commonly used for environment variable names or to imply constants. I suggest renaming the functions to lowercase.

Paths with spaces

Unless you have ironclad guarantees that $ACTIVE_DB will never contain spaces, it would be better to double-quote this path variable here:

done < $ACTIVE_DB/rom/system/build.prop