1

I know for a fact this is going to be something completely simple and i'm going to have missed the most basic of things but I cannot get my head around this.

file=( $(find $DIRECTORY -maxdepth 1  -type f) )
total=$(find $DIRECTORY -type f | wc -l)
count=0
while [ "$count" -lt "$total" ] 
do
for f in "$file";
do
echo -n "Would you like to copy $file? "
read yn
case $yn in
Y | y )
 cp $f $TARGET
 chmod =r $TARGET/$(basename $file)
 count=$((count + 1))
  ;;
 N | n )
   echo "skipping"
   count=$((count + 1))
  ;;
* ) echo "Please enter Y or N"
    exit 1
  ;;
esac
done
done

(sorry about formatting).

basically it's doing this at the moment

you input the source file and target file. works fine

$ ./script.sh ~/work ~/folder
Created target directory.
Would you like to copy /home/USER/work/cash.sh?

then you either type y / n

either of them only gives the same response.

$ ./script.sh ~/work ~/folder
target directory exists, starting copy
Would you like to copy /home/USER/work/cash.sh? y
Would you like to copy /home/USER/work/cash.sh? y

basically it does copy that cash.sh file and it knows there are 2 files in the directory, it just doesn't skip to the next file and stays on the same one and i'm not sure how to fix that at all.

Honestly I've been staring at this for the past few hours and its driving me insane, anything would be of help. Thanks in advance.

Magnilex
  • 11,584
  • 9
  • 62
  • 84
user3381055
  • 53
  • 6
  • You need to add the -r flag in your copy if you want to copy ALL files in the directory. The -r flag stands for recrusive – ryekayo Jan 08 '15 at 16:24
  • I need to be able to copy them one by one and let the user decide if they want to skip one file or to copy it. Does this still apply? – user3381055 Jan 08 '15 at 16:26
  • Ahhh sorry no it doesnt in that case. The recursive flag would just simply copy everything in the directory. – ryekayo Jan 08 '15 at 16:27
  • use `set -vx` to turn on shell debug/trace. You'll be able to see the line that is going to be executed, then the line that is executed with all values substituted for variables is displayed, preceded with `>`. Good luck. – shellter Jan 08 '15 at 16:33
  • Shouldn't you copy the $f, not the $file? The $file is your list of files. The $f is a line item out of that. – suiterdev Jan 08 '15 at 16:35
  • Also, since it looks like you adapted your code based on my earlier answer, would you mind marking that question as answered at some point please? – suiterdev Jan 08 '15 at 16:35
  • Other comments: you can use just `((count++))`. but why are you counting? use a while loop feed from `find $DIR -maxdepth 1 -type f | while read f ; do ....` (for the pendantic), if you filenames might have spaces in them, then use `find $DIR -print0 -max ... | while IFS= read -r -d $'\0' ; do ...` Good luck! – shellter Jan 08 '15 at 16:36
  • That's not the right way to collect files from `find` into an array, btw -- look at what happens when you have filenames with spaces or newlines; they end up as separate array entries. See also entry #1 in http://mywiki.wooledge.org/BashPitfalls – Charles Duffy Jan 08 '15 at 16:40
  • Oh, I see the question I answered was deleted after I answered it for you. Thanks. You still need to use $f. – suiterdev Jan 08 '15 at 16:40
  • 1
    Forget `find`. Forget counting. Just loop over a glob. `for f in "$DIRECTORY/"*; do` (assuming `$DIRECTORY` is your source). – Etan Reisner Jan 08 '15 at 16:40
  • `$f` and `$file` in the loop here are the same thing. The quotes around `"$file"` see to that. – Etan Reisner Jan 08 '15 at 16:40
  • 1
    ...or `for f in "$DIRECTORY"/*`, to be robust against unusual directory names. – Charles Duffy Jan 08 '15 at 16:41
  • 1
    It would also be worth running this script -- as a whole -- through http://shellcheck.net/; there are a lot of little gotchas. – Charles Duffy Jan 08 '15 at 16:41
  • Charles, thanks for the tip on shellcheck.net, never knew about that site, bookmarked it. – suiterdev Jan 08 '15 at 16:59
  • @user3381055 Why did you edit your question to just include dots? I rolled it back one revision. – Magnilex Jan 08 '15 at 17:20

2 Answers2

1

First, don't use UPPER_CASE_VAR_NAMES. Here's why.

Second, quote all your variables, unless you know specifically when you don't want to. Here's why.

You're storing the find results in an array, so you don't need to run find twice:

file=( $(find "$directory" -maxdepth 1  -type f) )
total=${#file[@]}   # number of elements in the array

Note that any filenames containing spaces will be split into separate words. Assuming your filenames don't contain newlines, you could do

mapfile -t file < <(find "$directory" -maxdepth 1 -type f)

Then, to iterate over the array:

for f in "${file[@]}"; do

Another technique, making no assumptions about what characters are in your filenames:

find "$directory" -maxdepth 1 -type f -print0 | while IFS= read -d '' -r filename; do # ...
Community
  • 1
  • 1
glenn jackman
  • 238,783
  • 38
  • 220
  • 352
0

You want to change:

echo -n "Would you like to copy $file? "

to:

echo -n "Would you like to copy $f? "
Paul Evans
  • 27,315
  • 3
  • 37
  • 54