3
\$\begingroup\$

I'm just getting into bash and sh scripting. I mostly just stub out little convenience or exercise scripts for myself, but I recognize I may be flaunting best practices at times.

My question is specifically related to best practices (or just common sense) when using rm inside a shell script. My googles have only turned up suggestions to alias rm to rm -i or somesuch.

Anyway, enough talk, here's the script (note: the rm appears at the very end of the script):

#!/bin/sh -e
# Format output of alias print dump into more readable format

if [ -f "TEMP.AWK" ]; then
    echo "Somehow 'TEMP.AWK' already exists"
    exit 1
fi


cat ${HOME}/.zshrc | grep -e '^alias' | awk -F'=' 'BEGIN {
        print "ALIAS                         | COMMAND";
        print "---------------------------------------";
    }
    {

        # replace all multi-spaces with a single space
        gsub(/\s+/, " ", $0);

        col1Len  = 20;
        col2Len  = 60;
        aliasLen = length($1);
        cmdLen   = length($2);

        tab1len  = col1Len - aliasLen;
        tab2len  = col2Len - cmdLen;

        printf " " $1 " "

        for (i = 0; i < tab1len; i++)
            printf "-"

        print " " $2

    } END {
        print "---------------------------------------";
    }' > TEMP.AWK

cat TEMP.AWK | grep --color -e '[#].*'

rm TEMP.AWK

Any other feedback would not go unappreciated.

\$\endgroup\$
1
  • 2
    \$\begingroup\$ Hi, welcome to Code Review! I hope you receive great answers! Whatever you do with rm inside a shell, make sure to use it carefully if you need -rf :) \$\endgroup\$ – Tunaki Apr 23 '16 at 18:56
2
\$\begingroup\$

The best practice for dealing with temporary files is simply not to use them in the first place. Many things can go wrong with temporary files:

  • Lack of permissions to write to a directory
  • Lack of disk space
  • Read-only filesystem
  • File or symlink already exists
  • File or symlink didn't exist when you checked, but got created immediately after, in a race condition
  • Script crashed, such that it created a temporary file but didn't get a chance to clean it up

Though there are ways to mitigate these risks, I see no reason to use a temporary file here at all. For that matter, cat and the first grep are also superfluous.

#!/bin/sh

awk -F= '
    BEGIN {
        TABLE_FMT = "%-18s | %-58s"
        row = sprintf(TABLE_FMT, "ALIAS", "COMMAND");
        print row;
        gsub(".", "-", row);
        print row;
    }
    END {
        print row;
    }

    /^alias/ {
        gsub(/\s+/, " ", $0);  # Condense consecutive whitespace
        printf TABLE_FMT "\n", $1, $2;
    }
' < "$HOME/.zshrc" | grep --color '#.*\|$'

I can't tell for sure, but it looks like the second grep is meant to highlight comments. Unfortunately, it actually ends up stripping out all uncommented lines, including the header and footer.


Instead of parsing the $HOME/.zshrc configuration file, you might want to query the live alias definitions in the current shell instead. In that case, you should write a shell function that processes the output of the bare alias command.

\$\endgroup\$
2
  • \$\begingroup\$ Thanks for the great reply! I totally missed my lack of \|$. I've included your suggestions in my script, and it indeed outputs as expected, and looks a lot cleaner. The reason I'm using .zshrc as a constant, is because I can't seem to print the output of the shell builtin alias from a script - only command line. I suppose if I wanted to make this script reusable, I could also do some checking based on the $SHELL env variable... Anyway, I digress! Your answer addresses my original question, and further, greatly improves the semantics of this script. So thank you again. :) \$\endgroup\$ – J.M. Janzen Apr 24 '16 at 14:26
  • 1
    \$\begingroup\$ Aliases are a feature in interactive shells, so they aren't available in scripts. Furthermore, even if they were available in scripts, you want to dump the aliases defined in the current shell, not of a subshell. That's why I recommended a shell function rather than a script for that. \$\endgroup\$ – 200_success Apr 24 '16 at 16:06

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.