patternbashMinor
trash script to alias rm
Viewed 0 times
scripttrashalias
Problem
This script implements a command line level trashcan system.
Designed to work around
Appreciate any and all suggestions from formatting, expansion of the idea to glaring issues. Looking for an experienced eye to pin point potential issues.
/usr/local/bin/trash
/usr/local/bin/untrash
`#!/bin/bash
#
# Restore files from Trashcan
# alias rmoops=untrash
#
error() {
echo $1
exit 1
}
file="$1"
trash=~/.trash
keep=5
## bypass scenarios
# root user
[ "$(id -u)" = "0" ] && error "Please do not run as root user."
# any flags so rm -rf actually does it
case $file in
-*) error "Flags are not supp
Designed to work around
rm doing the bits needed to implement a trashcan system at command line level. Any user flags hands the request over to rm. This means rm -f will still delete a file with no recovery. Prevents execution by root as too many dependencies at superuser level. Deployment by adding aliases rm=trash and rmoops=untrash. Only implemented on files, does not cater for directories.Appreciate any and all suggestions from formatting, expansion of the idea to glaring issues. Looking for an experienced eye to pin point potential issues.
/usr/local/bin/trash
#!/bin/bash
#
# Trashcan script
# alias rm=trash
#
error() {
echo $1
exit 1
}
bypass() {
exec rm $*
}
file="$1"
trash=~/.trash
keep=5
## bypass scenarios
# root user
[ "$(id -u)" = "0" ] && error "Please do not run as root user."
# special cases
case $file in
-) bypass $ ;;
$trash/ ) bypass $ ;;
esac
# only single filename supported
[ $# -gt 1 ] && bypass $*
# can we access the file?
[ -f "$file" ] || error "File not specified."
# move existing versions out of the way
ls $trash/$file.version.* 2>/dev/null | tac | while read name; do
version=$((${name##*.}+1))
# keep the only the last number of versions
if [ $version -gt $keep ]; then
rm $name || error "Unable to remove file $name."
else
mv $name $trash/$file.version.$version || error "Unable to move version $version."
fi
done
# move file into trash (volume friendly)
cp --preserve $file $trash/$file.version.1 || error "Unable to trash your file."
rm -f $file || error "Unable to cleanup the file."
exit 0
/usr/local/bin/untrash
`#!/bin/bash
#
# Restore files from Trashcan
# alias rmoops=untrash
#
error() {
echo $1
exit 1
}
file="$1"
trash=~/.trash
keep=5
## bypass scenarios
# root user
[ "$(id -u)" = "0" ] && error "Please do not run as root user."
# any flags so rm -rf actually does it
case $file in
-*) error "Flags are not supp
Solution
Overall
I'm not really convinced of the value of this "trashcan" idea. If deleting files doesn't actually delete them, then what's the point? Trying to create free space would be ineffectual. Worse, encouraging users to lean on an "undelete" facility will likely leave them in trouble when they move to a conventional system without such a crutch. (Plus, it's a Good Thing to have an occasional minor accident - otherwise it's very easy to just assume that your backups are working, and never think to verify that you can actually restore from them! I'm certainly guilty of that...)
Quoting issues
Much of your code doesn't quote expanded variables when it should. One example:
This is unsafe, and needs to be written
There are many more similar instances; I won't list them all.
Testing for file-ness rather than existence
Do you really want this error when you ask to trash a symlink? In fact, I think the wording of the message is poor, even for the case you think you're testing.
Pathname matching
You test whether the argument is in the "trash" directory with
There's other ways to refer to the same directory as
Echo and read
This code:
is usually written
(I'm not sure that
I'm not really convinced of the value of this "trashcan" idea. If deleting files doesn't actually delete them, then what's the point? Trying to create free space would be ineffectual. Worse, encouraging users to lean on an "undelete" facility will likely leave them in trouble when they move to a conventional system without such a crutch. (Plus, it's a Good Thing to have an occasional minor accident - otherwise it's very easy to just assume that your backups are working, and never think to verify that you can actually restore from them! I'm certainly guilty of that...)
Quoting issues
Much of your code doesn't quote expanded variables when it should. One example:
case $file in
-*) bypass $* ;;
$trash/* ) bypass $* ;;
esacThis is unsafe, and needs to be written
case "$file" in
-*) bypass "$@" ;;
"$trash"/* ) bypass "$@" ;;
esacThere are many more similar instances; I won't list them all.
Testing for file-ness rather than existence
[ -f "$file" ] || error "File not specified."Do you really want this error when you ask to trash a symlink? In fact, I think the wording of the message is poor, even for the case you think you're testing.
Pathname matching
You test whether the argument is in the "trash" directory with
case $file in
$trash/* )
esacThere's other ways to refer to the same directory as
"$trash" - in particular, a relative name from the working directory. You'll need to canonicalise path name if you're doing this kind of comparison; possibly you might want to compare device and inode numbers for real robustness if you might have bind mounts and the like.Echo and read
This code:
echo -n "Which would you like to restore? [1]:"
read useris usually written
read -p "Which would you like to restore? [1]: " user(I'm not sure that
user is a good name for the version number, either).Code Snippets
case $file in
-*) bypass $* ;;
$trash/* ) bypass $* ;;
esaccase "$file" in
-*) bypass "$@" ;;
"$trash"/* ) bypass "$@" ;;
esac[ -f "$file" ] || error "File not specified."case $file in
$trash/* )
esacecho -n "Which would you like to restore? [1]:"
read userContext
StackExchange Code Review Q#157299, answer score: 2
Revisions (0)
No revisions yet.