new to bash scripting

peok · 07-14-2006, 06:45 PM

I couldn't find a program to do what I wanted so I decided to take the plunge and learn bash scripting! The script works just fine for me, but I was hoping perhaps I could get some comments on if I did things the "right" way. Do you know what I mean? Did I use spacing approriately, did I use the right methods, did I go overboard on *'s, any unforseen problems, etc.

The script scans for wavpack (*.wv) files in subdirectories from the script and uses the program wvgain (part of the wavpack package) to determine if a song is missing replaygain tag information, and, if so, regenerates all of the replaygain tags (album mode) for all the *.wv files in the same folder as the guilty file.

Code:

#!/bin/bash
IFS=$'\n'
for X in `find . -name *.wv`
do
	if [ `wvgain -qs $X | grep "no ReplayGain values found"` ]
	then
		echo $'\n\n\n'******************************$'\n'ReplayGain tags missing in `basename $X`$'\n'Now removing all ReplayGain tags from all WavPack files in `dirname $X`/$'\n'******************************
		wvgain -c `dirname $X`/*.wv
		echo $'\n\n\n'******************************$'\n'Now adding ReplayGain tags to all WavPack files in `dirname $X`/$'\n'******************************
		wvgain -a `dirname $X`/*.wv
	else 
		echo "Tags OK: "$X
	fi
done

jlinkels · 07-14-2006, 09:44 PM

Peok,

If you had been a real Bash programmer, you'd program all this in a single line with lots of $, %, &, <, /, |, \, (), {} and ~. And I can read the script, so that is a bad sign.

Just kidding, I don't see anything wrong, this is the way I write it myself.

In addition I would have used:

$fname instead of $X as it is more descriptive
echo "now processing file $fname"
WVGAIN=/usr/bin/wvgain and then
$WVGAIN -a `dirname $X`/*.wv
to be able to replace this by WVGAIN=echo during debugging
log_out=/dev/stderr
to be able to change log_out for /dev/null or ~/mylog
and use debug statements like
echo "this is a debug statement" > $log_out

But that is pedantic and way beyond what ordinarily is used.

You might want to check "Programming with Style" in the Advanced Bash Scripting guide. The link is in my sig.

jlinkels

spirit receiver · 07-15-2006, 02:46 AM

There's something I find a little strange.
You iterate over all files that end with ".wv". Say there are two files: 1.wv, 2.wv. In the loop, we have X=./1.wv first, and "wvgain -qs ./1.wv" gives "no ReplayGain values found". Then you pass *.wv as an agument to "wvgain -c" and "wvgain -a", so they'll process both 1.wv and 2.wv now.
Then you proceed to the next iteration, where "wvgain -qs ./2.wv" won't trigger the test as that file was processed during the first iteration already. So this is fine in principle, but it still looks strange to iterate over the filnames and then process all files within the same iteration.
To me, one of the following procedures seems more natural:

Iterate as above, but use "wvgain -c $X" and "wvgain -a $X". This would be a proper iteration over each file.
Iterate over the directories only using "find -type d", and then use "wvgain -qs $X/*.wv" (and similarly with the other invokations of wvgain) to process one directory at a time.