2

This script should copy all files of which the names are provided as arguments on the command line to the users home directory. If no files have been provided, the script should use read to ask for file names, and copy all file names provided in the answer to the users home directory.

if [ -z $1 ]
then
    echo provide filenames
    read $FILENAMES
else
    FILENAMES="$@"
fi

echo the following filenames have been provided: $FILENAMES
for i in $FILENAMES
do
    cp $i $HOME
done

If I provide arguments as string - it works. But if I provide as "read $FILENAMES" - it has not working.

Mentor shows same solution in his lessons but did not shows how it works. enter image description here

UPDATE After I entered the filenames as arguments it gives empty strings and did not copy files to $HOME location

[dmytro@oc1726036122 ~]$ cd Desktop/
[dmytro@oc1726036122 Desktop]$ . totmp 
provide filenames
one two
the following filenames have been provided:
the following filenames have been provided:
[dmytro@oc1726036122 Desktop]$
  • 1
    Please explain what 'it's not working' means, are you getting an error? Are the files not getting moved? Do they end up somewhere else? – Panki Jun 3 at 11:35
  • 5
    The solution looks incorrect to me - should be read FILENAMES (no $). Aside from the error, there are a number of other issues with the mentor's solution - it will fail for filenames that contain whitespace for example – steeldriver Jun 3 at 11:35
  • Aside from the readline issue, cp $i $HOME should read cp "$i" "$HOME" to make sure you handle whitespace correctly. Always try to quote Bash variables. – Edward Jun 3 at 11:39
  • After I entered the filenames as arguments it gives empty strings and did not copy files to $HOME location – Smenov Jun 3 at 11:40
  • 1
    @Edward unfortunately simply quoting the filenames isn't sufficient - the for loop will already have tokenized the $FILENAMES string on whitespace – steeldriver Jun 3 at 11:45
2

The read declares variables rather than reading them. Simply put, remove the $ from read and you're good to go.

if [ -z $1 ]
then
    echo provide filenames
    read FILENAMES
else
    FILENAMES="$@"
fi

echo the following filenames have been provided: $FILENAMES
for i in $FILENAMES
do
    cp $i $HOME
done

EDIT: I see that you use the source (.) command to run a script.

[dmytro@oc1726036122 Desktop]$ . totmp

It might be fine for this particular script, but never do that for complex scripts. Otherwise you source any variable or function from that script into your shell. Just use bash totmp

  • Thank you! That's the solution. And I will use your advise about use "bash" not "." Thanks. – Smenov Jun 3 at 12:57
2

The issue that seems to be blocking you is that the read command is incorrect, the argument passed to it should be a variable name and so it should be passed without a $ (the $ will expand the contents of the variable, which will be empty at that point, so the result is a read without any variable names passed to it.)

read FILENAMES

There's another problem with the check for absence of the first command-line argument. If it's not present, then $1 will expand to nothing (not to an empty string), which might cause trouble with the [ command, since [ -z ] is not really supposed to be valid, [ -z "" ] is what you'd expect to check in that case. In short, you need to quote that variable:

if [ -z "$1" ]

As you're using bash, you could also use [[ ... ]], which is generally better since it's an internal command (in that case, this command should work without quotes, but keeping the quotes doesn't hurt and looks good.)

(P.S.: There's so much more that's wrong with this script, it's so far from best practices, I'm really appalled at seeing someone teach this. Unfortunately, it seems the bar is really low for teaching bash and the manuals are really complex until you actually understand it well, so I don't know that I'd have a better recommendation on how to learn it properly either.) ☹️

1

The read command takes the variable, but you already are referring to the values of the variable:

 #correct syntax
 read variable
 #wrong syntax
 read $variable

$variable is the value of variable and in the beginning of the script this is unset/empty.

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

Not the answer you're looking for? Browse other questions tagged or ask your own question.