help with a ksh script

I am trying to substitute words within multiple files...I posted the first script (http://www.unix.com/showthread.php?t=37505\) in tcsh and still for some reason getting nowhere so I tried it in ksh.

#!/bin/ksh
pattern1=$1
pattern2=$2

if [[ $# != 2 ]]
then
print "Usage: $0 $pattern1 $pattern2"
exit
fi


for file in $(ls)
do
sed "s/$pattern1/$pattern2/g" $file > $file
done

The problem is that when I run the script, it seems it gets to here

if [[ $# != 2 ]]
then
print "Usage: $0 $pattern1 $pattern2"
exit
fi

and never makes it to

for file in $(ls)
do
sed "s/$pattern1/$pattern2/g" $file > $file
done

because I don't see any changes made to the .txt files when I do

more test1.txt

cause I still see the old word there.

you have to use a temp file

sed "s/$pattern1/$pattern2/g" $file > /tmp/file
mv /tmp/file $file

or something like that

Use -ne for numeric comparisons, and don't use the non-standard [[...]]:

if [ $# -ne 2 ]

Not only is ls unnecessary, but it will break your script if any filenames contain spaces or other pathological characters. Use:

for file in *

You're lucky to see anything at all. Normally, the shell truncates the file before the command is even started.

The standard method is to save the output to a temporary file, then move or copy it over the original file if the command succeeded:

sed -e '...' FILE > tempfile && mv tempfile FILE

Or copy it first and send the output to the original file:

cp FILE tempfile
sed -e '...' tempfile > FILE

Some version of sed have an option, -i, that edits the file in situ.

A trick that allows redirection to the same file is:

{ rm FILE; sed -e '...' > FILE; } < FILE

SO I don't have to use "$" for FILE right?

If it's a variable, of course you do.

In the example, it is just a generic filename. You replace it with the name of your file. If you use a variable, quote it.

Thanks again! I will try that and let you know what happens.

cfajohnson or anyone else,

personal opinion...would you use this in the script

if [[ $# != 2 ]]
then
print "Usage: $0 $pattern1 $pattern2"
exit
fi

cause I think it don't have to be there...I would thought the the do statement is the important part of the script.

I wouldn't use [[ ... ]] or !=; I'd use [ $# -ne 2 ].

But, yes, I would use something there or the script might fail if the wrong number of arguments are given.

Alrighty...here's what I got (with your help...hopefully I did this right...just learning this stuff 2 weeks ago)

#!/bin/ksh
pattern1=$1
pattern2=$2

if [ $# -ne 2 ]
then
print "Usage: $0 $pattern1 $pattern2"
exit
fi

for file in *
do
cp $3 /tmp/$3
sed -e 's/$pattern1/$pattern2/g'/tmp/$3 > $3
done

Does that look about right?

BTW In case you wondering why I using $3. $3 is the name of the file in which to make the substitution.

It looks good, apart from a few problems:

  • You should quote variables. Your script will fail if $3 contains a space, for example.
  • Use double quotes, or the pattern variables will not be expanded
  • If the patterns may contain slashes, change the delimiter or escape the slashes
  • Note that the replacement is a string, not a pattern
cp "$3" "/tmp/$3"
sed -e "s/$pattern1/$pattern2/g" "/tmp/$3" > "$3"

i have always been more fond of the following:

for file in *; do
sed "s/$pattern1/$pattern2/g" < ${file} > ${file}.$$ && {
mv ${file}.$$ ${file}
}
done

so sed dumps into file.$$ ($$ = pid of script) then upon success (&&) it moves the new file to the original name

Why bother bracing a single command?

if the command fails you do not over write the original file and can see the output of the command by looking at the temp file

The braces have nothing to do with that, the && is sufficient.

i see what you are saying, i use the braces out of habit and they are not needed for single commands

I would also suggest that it is a bad way to write code for multiple commands, a proper if statement will be much more readable and easier to maintain.

i respectfully disagree. i have many scripts that use that type of logic and have never had a problem. i also have many scripts that use if/then with $?.

i typically use if/then on scripts that perform multiple or complex tasks, mostly (as you said) to make them easier to read.

You are of course entitled to do so, there is nothing syntactically or logically wrong with it, but I would suggest still that it is a bad way to code becasue you lose the structure and flow of the code.

My rationale:

Embedding programming logic in lines of code makes the program difficult to follow, the logic harder to find and debug. From experience this can cost money because it takes longer for someone else to learn/fix/extend the code which is always a probable situation in a professional setting.

If on the other hand it's for personal use, then anything goes.

each to his own i suppose - but as with any script small or large - it should be well commented. it is the documentation that makes it professional (given that it functions and is error correcting as well)

If you use if ... then ... else ... fi, you don't need to comment to the same degree. Self documenting code is always preferable.

There are cases with && and || which can trip up the unwary. For example:

COMMAND1 && COMMAND2 || COMMAND3

Is not the same as:

if COMMAND1
then
   COMMAND2
else
   COMMAND3
fi

It is the same as:

if COMMAND1 && COMMAND2
then
   :
else
   COMMAND3
fi

In other words, COMMAND3 will be executed if either COMMAND1 or COMMAND2 fails.