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:
1 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
-
\$\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 ofecho
, i think i will give this a shot. Thanks for the feedback \$\endgroup\$chris– chris2016年02月29日 14:07:18 +00:00Commented Feb 29, 2016 at 14:07