2
\$\begingroup\$

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: 
asked Feb 29, 2016 at 12:05
\$\endgroup\$

1 Answer 1

2
\$\begingroup\$

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
answered Feb 29, 2016 at 13:26
\$\endgroup\$
1
  • \$\begingroup\$ "worlds most awesome bash script menu" would have been sufficient^^ now seriously, the possible bug you pointed out, after posting here i changed to code to if CHECK_SYS && CHECK_SYS_NEMPTY ; then...else...fi . I tested what happens without any of the folders, and what happens if they exist but are empty. Works as intended. As to Maintainability and Paths with spaces , take the point, it's yours. The all-caps naming i simply took from another bash script, but will consider changing this. printf instead of echo, i think i will give this a shot. Thanks for the feedback \$\endgroup\$ Commented Feb 29, 2016 at 14:07

Your Answer

Draft saved
Draft discarded

Sign up or log in

Sign up using Google
Sign up using Email and Password

Post as a guest

Required, but never shown

Post as a guest

Required, but never shown

By clicking "Post Your Answer", you agree to our terms of service and acknowledge you have read our privacy policy.

Start asking to get answers

Find the answer to your question by asking.

Ask question

Explore related questions

See similar questions with these tags.