0

Why doesn't this script work? It always opens the default directory where the script sits in, can't open new ones

#!/bin/bash

read -p "choose a directory: " map list=$( ls ) for item in $list; do echo $item read -p "Make copy? " ans

if [ $ans = "yes" ]; then        
    cp $item > mkdir $map+copy
fi

done echo "Done"

Andy Dalton
  • 13,993
David
  • 9
  • Run the script through shellcheck.net and fix the errors it tells you about. (1) cp does not deal with redirects. (2) + is not a shell operator to concatenate strings. (3) mkdir runs on its own command line, and only needs to be run once (at most) because you only specify it once. (4) ls is not suitable for subsequent processing as it deals badly with spaces, special characters, and directories – Paul_Pedant Apr 02 '21 at 22:22
  • 1
    Does this answer your question? School task, bash scripting ---- This is a direct copy of your earlier question. Please either delete the old or this one and update the other appropriately. As for now one cannot tell what you are aiming for. – FelixJN Apr 02 '21 at 22:27
  • 1
    This line cp $item > mkdir $map+copy attempts to copy $item to $map+copy, writing any output of the cp command to file mkdir. I don't think this is your intention. I can guess what your script is supposed to accomplish, but to avoid misunderstandings and incorrect assumptions, please tell us what the desired result is. – berndbausch Apr 02 '21 at 23:49
  • 1
    Expanding on @ berndbausch comment, change it to "cp ./$item $map" - map is a directory. And you should test that the target directory of the copy exists, i.e., $map. And you need the ./$item since the files probably arn't in your $PATH. Further, if you want people to type "yes" - then you need to tell them to type "yes" or modity the test to include "y". – Cinaed Simson Apr 03 '21 at 01:21
  • I suggest that you try to manually run the commands that you're trying to script. If you cannot run it manually, it's not going to work in the script either. You need to understand what you're trying to accomplish and what sequence of commands will do what you're trying to accomplish before you start writing a script to do it. – Andy Dalton Apr 03 '21 at 01:47
  • 1
    If you've run this it will have given you errors. You haven't even mentioned these. Fix them, or if you're stuck ask about them. Even if someone here wrote your homework for you, what would that gain you, really? You won't have learned anything useful. Help you? Yes, certainly. Do your homework for you? Not really – Chris Davies Apr 03 '21 at 07:07
  • @David, oops, I didn't realize this was connected to the earlier question. – ilkkachu Apr 03 '21 at 12:45
  • @CinaedSimson: You're right that putting ./ before $item is a good idea, but you've got the wrong reason — it has nothing to do with $PATH.  Also, rather than saying "cp ./$item $map", you should say cp "./$item" "$map" (putting the variables in quotes). – Scott - Слава Україні Apr 12 '21 at 01:55

1 Answers1

1
cp $item > mkdir $map+copy

Ok, so, > redirects the output of a command, the part that's printed (usually to your terminal). E.g. ls outputs a list of files, so ls > list.txt would output that list to list.txt instead. But e.g. cp foo bar explicitly opens the files foo and bar, but doesn't output anything to the terminal.

Thus, the redirection here, gives you an empty file called mkdir, but the rest of the command cp $list $map+copy would copy the file named by $list to whatever $map+copy expands to (the contents of the variable $map and the fixed string +copy, concatenated together).

On the other hand, cat foo would open foo and print it out, and you could use cat foo > bar to direct that printout to a file called bar. Pretty much the same as doing cp foo bar, actually, except that cp has options to like -a and -p to also copy the owner and permission information.

And, in the shell, you can concatenate strings by just sticking them next to each other (without any whitespace in between). So, if you set the variables x=foo y=bar, all of these print foobar:

echo foobar
echo "foo""bar"
echo "$x$y"
echo "foo$y"
echo "${x}bar"

Which means that you can just do "${map}copy" or "$map""copy" to concatenate the two parts.

You do need to run mkdir separately from cp, though, so if $map contains foobar, and $item is hello.txt, this would create a directory foobarcopy, and copy hello.txt to that directory:

mkdir -p "${map}copy"
cp "$item" "${map}copy"

(-p tells mkdir not to error if the directory already exists.)

See:

for issues wrt. (not) double-quoting the variables.


Also, instead of:

list=$( ls )
for item in $list; do

You can have the shell produce a list of filenames without calling ls:

for item in ./*; do

See:

ilkkachu
  • 138,973
  • Thus, with the redirection here, you just get an empty file called mkdir. I think here, you'd both an empty file called mkdir and a copy of the source file in $map+copy (i.e., the same behavior as cp $item $map+copy > mkdir). – Andy Dalton Apr 03 '21 at 14:18
  • @AndyDalton, yes, bad phrasing on my part – ilkkachu Apr 03 '21 at 14:42
  • (1) Also, of course, "foo"bar and "$x"bar, and hence "$map"copy.  (2) Of course mkdir -p has an additional benefit, if the user enters something like a/b/c for map. – Scott - Слава Україні Apr 12 '21 at 02:13