Shell Programming and Scripting

Read the comments I posted earlier.

Answer the questions I posed.

Why are you using:

Code:

for i in `ls -1 | grep $1 | grep  $2`

That is almost certainly the wrong way to do whatever you are trying to do.

Where are you compiling a list? All I see is the display of the individual commands, and nowhere do you put them into a list.

Of course.

Make sure the code you use is correct; in the script you posted, it was incomplete.

Just saying you made a change is not enough; I need to see the code you are trying to execute.

The case statment does not carry out the action; it makes a decision about which action is to be carried out. You then need the code to carry out the action.

You might find it helpful to put sections of your code into functions so that they can be tested idependently.

For example, you might want these functions: build_list, show_list, ask_user and move_files.

After you have written and tested the functions, the rest of your script could look like this:

Code:

build_list <file pattern>
show_list
if ask_user
then
   move_files
else
   echo "No changes made ..."
fi

I didn't suggest anything. I posted an example of what might be a better way of doing it. I don't know whether it is correct or not because you haven't explained the file selection criteria.

Does it do what you want? Does the wildcard pattern match the files you want?

What values are $1 and $2 likely to contain?

Does the following code display the files you want to work with?

Code:

printf "%s\n" *$1*$2*

It is explained in the "parameter expansion" section of your shell's man page. To see what the expansion does, try printing the individual parts:

Code:

printf "%s\n" "$i" "${i%%.Sent*}" "${i#*.Sent}"

If you had tested it first, you would have found most of the errors.

If you cannot tell us how it is not working, how do you expect us to help?

function build_list

That is not wrong in ksh and bash, but it is not the standard syntax for defining a function and will not work in other shells. Standard is:

Code:

build_list()

{
for i in *$1*$2*
do
x=`echo $i | sed 's/\.Sent/\.Done/g'`
done
}

Tha function does not build a list; only the last filename that matched the pattern would be stored in $x.

function show_list
{
echo mv $i DONE/$x
}

It makes your script more undertandable if you use meaningful names; that function doesn't show a list, it moves files.

...and it will fail if any filenames contain spaces or other pathological characters.

..and there is no logic behind that code. Think about what you are doing.

echo "Is this Ok?"
read user_response
if [$user_response="y"|"Y"]

There are five errors in the line above.

then
 mv $i DONE/$x;;
else
if [$user_response="n"|"N"]
then
echo "No changes made ...";;
fi
fi