Re: Bash/Shell scripting practices/conventions
- From: "Chris F.A. Johnson" <cfajohnson@xxxxxxxxx>
- Date: Fri, 28 Mar 2008 19:17:15 +0000
On 2008-03-28, Tobias Nissen wrote:
I just finished my first bigger bash script and I want to upload it to
my homepage. I'm putting it up for discussion here, because I want to
hear from experienced bash programmers what they would have done in
another way.
If you like, you can have a look at the script under this address:
http://movb.de/tmp/drotate
The script is called "directory rotate". Its intended use is to archive
files and directories older than a given number of days and after
succesful archiving to delete those files and directories -- hence the
"rotate". I am using it to archive my Mail (which is stored in MH
format) and to archive daily generated statistical data which is needed
in direct access for half a year only and can then be archived. When I
was searched for something usable some time ago, I didn't find anything
that matched my criteria.
I don't elaborate the design goals, because I mostly care about comments
on the actual code. In the following I will list some of the things I
find worth discussing.
* To check whether a command (here 'fold') is executable, I'm doing
this:
if [ "$($FOLD --version 1>/dev/null 2>&1 && echo 1)" !=3D 1 ]; then
echo "fold not executable"
fi
Since fold is a standard command, why bother to test for it? If
you are going to do that, you might as well test for cat, ls,
etc..
If you do want to test for a command, use the built-in 'command':
if command -v fold >/dev/null
then
FOLD="fold -sw"
else
FOLD=cat
fi
That way, you don't need the wrap function.
* Informational output that shall only be displayed if the user
requested so (-v flag) is handled by vecho:
vecho ()
{
if [ $VERBOSE -eq 1 ]; then
echo $@
It's better to use printf:
printf "%s\n" "$*"
fi
}
* Line-wrapping: To prevent line-breaks within word-boundaries I pipe
outputs that might get longer than the terminal width through 'wrap',
defined as follows (FOLD is either 'fold' or 'cat' and is set in the
initialisation stage):
wrap ()
{
if [ "$FOLD" == 'cat' ]; then
That works in bash, but is non standard. Preferable is:
if [ "$FOLD" = 'cat' ]; then
cat
else
$FOLD -sw $COLUMNS
fi
}
See above.
* In order to make the line wrapping work, I'm getting the number of
columns like this:
COLUMNS=$(tput cols)
That should be unnecessary. Test for the existence of $COLUMNS
before calling an external command:
: ${COLUMNS:=$(tput cols)}
if [ $? != 0 ]; then
COLUMNS=72
vecho "INFO: Error getting terminal width via '$GETCOLUMNS' so I assume=
$COLUMNS columns." 1>&2
fi
* If a config file exists at the default position, it is (always) read
at startup. If the user specifies an alternative config file, this is
also read (afterwards). This way I can have one master config file and
several special config files. Is it OK to do so?
That's a good way to do it.
* I am saving a list of filenames to a file named $FILELIST, whereas
filenames are delimited by the NULL-character \0. I'm getting the
number of filenames with
tr \\000 \\n < $FILELIST | wc -l
I'm outputting the list with
tr \\000 \\n < $FILELIST
Somehow I find this very ugly.
Why not save them as a newline-separated list? You are converting
them to that.
If the reason for using a NUL-separated list is because there are
filenames containing newlines (you should fix that!), converting
to a newline-separated list will break it anyway.
* 0==TRUE vs 1==TRUE: Is it too inconsistent if my flag variables h=
old
1 if the flag is supposed to be set and 0 otherwise? I check them
like this:
if [ "$FLAG" -eq 1 ]; then echo flag is set; fi
That's logical.
After all in "if list1; then list2; fi" list2 is executed when list1
returned 0. Or is it OK, because 0 just means "last command finished
with no errors" and true and false are something completely
different?
In what context? There are commands true and false which return
zero (success) and non-zero (failure) respectively.
....
From the script on your web site:
STARTDIR=$(pwd)
....
ARCHDIR=$(pwd)
The current directory is in $PWD; there's no need to use command
substitution (which is slow). Even if it wasn't, you wouldn't need
to use pwd twice.
GETCOLUMNS='tput cols'
Why? Use $COLUMNS.
--
Chris F.A. Johnson, author <http://cfaj.freeshell.org/shell/>
Shell Scripting Recipes: A Problem-Solution Approach (2005, Apress)
===== My code in this post, if any, assumes the POSIX locale
===== and is released under the GNU General Public Licence
.
- Follow-Ups:
- Re: Bash/Shell scripting practices/conventions
- From: Icarus Sparry
- Re: Bash/Shell scripting practices/conventions
- From: Tobias Nissen
- Re: Bash/Shell scripting practices/conventions
- References:
- Bash/Shell scripting practices/conventions
- From: Tobias Nissen
- Bash/Shell scripting practices/conventions
- Prev by Date: Bash/Shell scripting practices/conventions
- Next by Date: Detect ssh in bash
- Previous by thread: Bash/Shell scripting practices/conventions
- Next by thread: Re: Bash/Shell scripting practices/conventions
- Index(es):
Relevant Pages
|
