HiveBrain v1.2.0
Get Started
← Back to all entries
patternbashMinor

Keep track of Terminal access

Submitted by: @import:stackexchange-codereview··
0
Viewed 0 times
terminalaccesskeeptrack

Problem

I am fairly new to bash (I have about 3 months of experience), and now I've written my first real application (sort of).

It's a function inside of ~/.bash_profile, but it has an installer. It is called loginfo and it keeps track of when the terminal was used by the user, and allows them to view this data.

It comes with an installer, and a built-in uninstaller.

install.sh

#!/bin/bash

width=$(tput cols)
printSolid(){
printf "%${width}s\n" " " | tr " " "$1"
}

printCenter(){
textsize=${#1}
padding=$((($width+$textsize)/2))
printf "%${padding}s\n" "$1"
}

install_loginfo(){
touch ~/.bash_profile
if grep -q "#loginfo-->" ~/.bash_profile
then
    echo "[ER] Already installed 'loginfo'."
else
    loginfofile=$(echo "$(dirname $0)/loginfo.txt")
    test ! -e $loginfofile && (echo "[ER] Unable to locate '$loginfofile'")
    cat $loginfofile >> ~/.bash_profile
    test $? -eq 0 && (echo "[OK] Successfully appended '$loginfofile' to '~/.bash_profile'")
    test $? -ne 0 && (echo "[ER] Something went wrong appending '$loginfofile' to '~/.bash_profile'")
fi
}

printSolid "#"
printCenter "Installer for 'loginfo'"
printCenter "By 545H4"
printSolid "#"
echo "Would you like to install 'loginfo'? (y/n)"
while true
do
read -n 1 -r -s confirm
case $confirm in
y | Y)
    install_loginfo
    break
    ;;
n | N)
    echo "'loginfo' was not installed."
    break
    ;;
*)  echo "Please enter a valid option (y/n)"
    ;;
esac
done
echo "You have got to restart your terminal for any change to take place."
exit


loginfo.txt

```
#loginfo-->

currLogin=$(date '+%d/%m/%Y %H:%M:%S')
echo "$currLogin" >> terminallogins.txt

deltLoginfo(){
while true
do
echo "Remove 'loginfo'? (y/n)"
read -n 1 -r -s confirm
case $confirm in
y | Y)
echo "$(sed '/^#loginfo-->/,/^# ~/.bash_profile
test $? -eq 0 && (echo "[OK] Successfully removed 'loginfo' from '~/.bash_profile'")
test $? -ne 0 && (echo "[ER] Failed to remove 'loginfo' from '~/.bash_profile'")
touch ~/te

Solution


  • indent your code!



-
don't do this

printf "%${padding}s\n" "$1"


do this

printf "%*s\n" "$padding" "$1"


-
in this section:

cat $loginfofile >> ~/.bash_profile
test $? -eq 0 && (echo "[OK] Successfully appended '$loginfofile' to '~/.bash_profile'")
test $? -ne 0 && (echo "[ER] Something went wrong appending '$loginfofile' to '~/.bash_profile'")


the 2 test command is testing the exit status of the previous command, not the cat command. It's either testing the exit status of the previous test command or the first echo command. Either way, you get the result you want, but really by luck. You want

if cat $loginfofile >> ~/.bash_profile; then
    echo "[OK] Successfully appended '$loginfofile' to '~/.bash_profile'"
else
    echo "[ER] Something went wrong appending '$loginfofile' to '~/.bash_profile'"
fi


Be aware that the logic of A && B || C is different from if A; then B; else C; fi in one subtle way: what happens when B exits non-zero?

  • in if A; then B; else C; fi when A succeeds but B fails, C will NOT be executed.



  • in A && B || C when A succeeds but B fails, C WILL be executed -- the || operator detects the previous command in the pipeline (B) exits non-zero and will execute its right-hand side command.



-
I'd recommend using select instead of a while loop to get your answer

PS3="Would you like to install 'loginfo'? "
select ans in Yes No; do
    case $ans in
        Yes) install_loginfo
            break
            ;;
        No) echo "'loginfo' was not installed."
            break
            ;;
    esac
done

Code Snippets

printf "%${padding}s\n" "$1"
printf "%*s\n" "$padding" "$1"
cat $loginfofile >> ~/.bash_profile
test $? -eq 0 && (echo "[OK] Successfully appended '$loginfofile' to '~/.bash_profile'")
test $? -ne 0 && (echo "[ER] Something went wrong appending '$loginfofile' to '~/.bash_profile'")
if cat $loginfofile >> ~/.bash_profile; then
    echo "[OK] Successfully appended '$loginfofile' to '~/.bash_profile'"
else
    echo "[ER] Something went wrong appending '$loginfofile' to '~/.bash_profile'"
fi
PS3="Would you like to install 'loginfo'? "
select ans in Yes No; do
    case $ans in
        Yes) install_loginfo
            break
            ;;
        No) echo "'loginfo' was not installed."
            break
            ;;
    esac
done

Context

StackExchange Code Review Q#94992, answer score: 6

Revisions (0)

No revisions yet.