7
\$\begingroup\$

This is a script (function) to be added to a bashrc. The purpose is to not actually remove files, but rather to send them to a trash folder for safe keeping. I was tired of deleting important files. Because files may have the same name, I chose to create subdirectories with the date and file name on them, for easy access and to prevent conflicts.

Thoughts? Improvements I could make? This is my first BASH script function, and I'd love to make it a standard part of my computers, so I want to make it good first!

function rm { 
    PREFIX="~/.Trash";
    for FILE in "$@" ; do 
        STARDATE=`date +%Y%m%d-%H%M`;
        PLACE="${STARDATE}-${FILE}" ; 
        mkdir -p "${PREFIX}/${PLACE}"; 
        mv $FILE $PREFIX/$PLACE/$FILE ; 
        echo "${FILE} moved to ${PREFIX}/${PLACE}" ; 
    done
}
\$\endgroup\$
2
  • \$\begingroup\$ Does this work if the file name has spaces in it? I suspect it doesn't, since $PREFIX/$PLACE/$FILE is not quoted. Also, why do you use ${foo} instead of just $foo? \$\endgroup\$ Commented Jun 8, 2014 at 21:00
  • \$\begingroup\$ What operating system? Have you also considered LVM snapshots (Linux) or ZFS snapshots (BSD/Solaris) or Time Machine backups (Mac OS)? \$\endgroup\$ Commented Jun 8, 2014 at 21:22

2 Answers 2

Reset to default
7
\$\begingroup\$

Interesting idea! I propose to rewrite the script this way:

rm() { 
    TRASH=~/.Trash
    for FILE; do
        DATE=$(date +%Y%m%d-%H%M)
        TARGETDIR="$TRASH/$DATE-$FILE"
        mkdir -p "$TARGETDIR"
        mv -v "$FILE" "$TARGETDIR/"
    done
}

What I changed and why:

  • for i; do is equivalent to for i in "$@"; do but shorter
  • In PREFIX="~/.Trash", the ~ does NOT expand to your home directory, so I believe this is a bug. And since there's nothing to quote in .Trash either, you can just write PREFIX=~/.Trash without any quoting
  • Instead of an echo after the mv, you can use the -v flag to make mv more verbose and speak for itself
  • Correctly quoted everywhere it's necessary
  • Changed the deprecated `cmd` style command substitution to the new recommended $(cmd) style
  • Renamed variables to be more intuitive, though I know this is subjective:
    • PREFIX to TRASH, to be more specific and clear
    • STARDATE to DATE, because I don't know what is "STAR" and DATE alone is already pretty clear
    • combined PREFIX + PLACE to TARGETDIR to reduce duplication
  • Changed function rm { to rm() {, because this seems to be the preferred way to declare functions in bash
  • Removed all the trailing ;, you don't need them
  • Use $VARNAME instead of ${VARNAME}, because I find that easier to read, but do as you like

Personally I would rename rm() to trash(), because most of the time I certainly don't want to backup everything, especially when deleting directories. trash also seems intuitive.

By the way, this is a great site to check your Bash scripts for common mistakes:

http://www.shellcheck.net/#

\$\endgroup\$
4
\$\begingroup\$

You should realize that this is not a drop-in replacement for /bin/rm. For example, this function won't handle options such as -f correctly.

You double-quoted several variable expansions, but you must make it a religious habit to always double-quote all such expansions. Otherwise, special characters in filenames will break your script in nasty ways.

Error handling is poor. You'll echo a confirmation even when mv fails. Therefore, you should join commands nontrivial with semicolons, but with &&. Also, errors should go to standard error instead of standard out, so use echo "…" >&2.

\$\endgroup\$

You must log in to answer this question.

Start asking to get answers

Find the answer to your question by asking.

Ask question

Explore related questions

See similar questions with these tags.