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: 
\$\endgroup\$
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
\$\endgroup\$
  • \$\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\$ – chris Feb 29 '16 at 14:07

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

Not the answer you're looking for? Browse other questions tagged or ask your own question.