AndreasInfo
AndreasInfo

Reputation: 1227

Bash command does not work in script but in console

I have running the two commands in a script where I want to check if all files in a directoy are media:

1 All_LINES=$(ls -1 | wc -l)
2 echo "Number of lines: ${All_LINES}"
3
4 REACHED_LINES=$(ls -1 *(*.JPG|*.jpg|*.PNG|*.png|*.JPEG|*.jpeg) | wc -l)
5 echo "Number of reached lines: ${REACHED_LINES}"

if[...]

Running line 4 and 5 sequentially in a shell it works as expected, counting all files ending with .jpg, .JPG...

Running all together in a script gives the following error though:

Number of lines: 12
/home/andreas/.bash_scripts/rnimgs: command substitution: line 17: syntax error near unexpected token `('
/home/andreas/.bash_scripts/rnimgs: command substitution: line 17: `ls -1 *(*.JPG|*.jpg|*.PNG|*.png|*.JPEG|*.jpeg) | wc -l)'
Number of reached lines:

Could somebody explain this to me, please?

EDIT: This is as far as I got:

#!/bin/bash

# script to rename images, sorted by "type" and "date modified" and named by current folder

#get/set basename for files
CURRENT_BASENAME=$(basename "${PWD}")
echo -e "Current directory/basename is: ${CURRENT_BASENAME}\n"
read -e -p "Please enter basename: " -i "${CURRENT_BASENAME}" BASENAME
echo -e "\nNew basename is: ${BASENAME}\n"

#start
echo -e "START RENAMING"

#get nr of all files in directory
All_LINES=$(ls -1 | wc -l)
echo "Number of lines: ${All_LINES}"

#get nr of media files in directory
REACHED_LINES=$(ls -1 *(*.JPG|*.jpg|*.PNG|*.png|*.JPEG|*.jpeg) | wc -l)
echo "Number of reached lines: ${REACHED_LINES}"

EDIT1: Thanks again guys, this is my result so far. Still room for improvement, but a start and ready to test.

#!/bin/bash

#script to rename media files to a choosable name (default: ${basename} of current directory) and sorted by date modified

#config
media_file_extensions="(*.JPG|*.jpg|*.PNG|*.png|*.JPEG|*.jpeg)"

#enable option extglob (extended globbing): If set, the extended pattern matching features described above under Pathname Expansion are enabled.
#more info: https://askubuntu.com/questions/889744/what-is-the-purpose-of-shopt-s-extglob
#used for regex
shopt -s extglob

#safe and set IFS (The Internal Field Separator): IFS is used for word splitting after expansion and to split lines into words with the read builtin command.
#more info: https://bash.cyberciti.biz/guide/$IFS
#used to get blanks in filenames
SAVEIFS=$IFS;
IFS=$(echo -en "\n\b");

#get and print current directory
basedir=$PWD
echo "Directory:" $basedir

#get and print nr of files in current directory
all_files=( "$basedir"/* )
echo "Number of files in directory: ${#all_files[@]}"

#get and print nr of media files in current directory
media_files=( "$basedir"/*${media_file_extensions} )
echo -e "Number of media files in directory: ${#media_files[@]}\n"

#validation if #all_files = #media_files
if [ ${#all_files[@]} -ne ${#media_files[@]} ]
then
  echo "ABORT - YOU DID NOT REACH ALL FILES, PLEASE CHECK YOUR FILE ENDINGS"
  exit
fi

#make a copy
backup_dir="backup_95f528fd438ef6fa5dd38808cdb10f"
backup_path="${basedir}/${backup_dir}"
mkdir "${backup_path}"
rsync -r "${basedir}/" "${backup_path}" --exclude "${backup_dir}"
echo "BACKUP MADE"

echo -e "START RENAMING"

#set new basename
basename=$(basename "${PWD}")
read -e -p "Please enter file basename: " -i "$basename" basename
echo -e "New basename is: ${basename}\n"

#variables
counter=1;
new_name="";
file_extension="";

#iterate over files
for f in $(ls -1 -t -r *${media_file_extensions})
do
  #catch file name
  echo "Current file is:           $f"
  #catch file extension
  file_extension="${f##*.}";
  echo "Current file extension is: ${file_extension}"
  #create new name
  new_name="${basename}_${counter}.${file_extension}"
  echo "New name is:               ${new_name}";
  #rename file
  mv $f "${new_name}";
  echo -e "Counter is:                ${counter}\n"

  ((counter++))
done

#get and print nr of media files before
echo "Number of media files before: ${#media_files[@]}"
#get and print nr of media files after
media_files=( "$basedir"/*${media_file_extensions} )
echo -e "Number of media files after: ${#media_files[@]}\n"

#delete backup?
while true; do
    read -p "Do you wish to keep the result? " yn
    case $yn in
        [Yy]* ) rm -r ${backup_path}; echo "BACKUP DELETED"; break ;;
        [Nn]* ) rm -r !(${backup_dir}); rsync -r "${backup_path}/" "${basedir}"; rm -r ${backup_path}; echo "BACKUP RESTORED THEN DELETED"; break;; 
        * ) echo "Please answer yes or no.";;
    esac
done

#reverse IFS to default
IFS=$SAVEIFS;

echo -e "END RENAMING"

Upvotes: 1

Views: 350

Answers (3)

Paul Hodges
Paul Hodges

Reputation: 15246

As @chepner already pointed out in a comment, you likely need to explicitly enable extended globbing on your script. c.f. Greg's WIKI

It's also possible to condense that pattern to eliminate some redundancy and add mixed case if you like -

$: ls -1 *.*([Jj][Pp]*([Ee])[Gg]|[Pp][Nn][Gg])
a.jpg
b.JPG
c.jpeg
d.JPEG
mixed.jPeG
mixed.pNg
x.png
y.PNG

You can also accomplish this without ls, which is error-prone. Try this:

$: all_lines=(*)
$: echo ${#all_lines[@]}
55

$: reached_lines=( *.*([Jj][Pp]*([Ee])[Gg]|[Pp][Nn][Gg]) )
$: echo ${#reached_lines[@]}
8

c.f. this breakdown

If all you want is counts, but prefer not to include directories:

all_dirs=( */ )
num_files=$(( ${#all_files[@]} - ${#all_dirs[@]} ))

If there's a chance you will have a directory with a name that matches your jpg/png pattern, then it gets trickier. At that point it's probably easier to just use @markp-fuso's solution.

One last thing - avoid all-caps variable names. Those are generally reserved for system stuff.

edit

If you're using shopt for extglob anyway, you might as well use it.
shopt -s nocaseglob makes the whole case-insensitive pattern simpler: *.*(jp*(e)g|png) will do it.

Also, if you were going to something like

num_files=$(( ${#all_files[@]} - ${#all_dirs[@]} ))

then you should also probably use nullglob when loading the array so that zero is possible and accurate, or at least failglob. "Hidden" files and directories beginning with a dot are generally excluded, which might be a bug or a feature, according to your needs; if you want them included, add dotglob.

If for some reason you wanted to check arbitrary depth subdirectories as well, set globstar and use ** instead of just * for the directory pattern - e.g., $basedir/**/*.*(jp*(e)g|png) - and remember that *() in extended globbing is "zero or more", while +() is "one or more", ?() is "zero or one", etc.

Reads more here.

Upvotes: 3

markp-fuso
markp-fuso

Reputation: 33994

Assuming the OP wants to limit the counts to normal files (ie, exclude non-files like directories, pipes, symbolic links, etc), a solution based on find may provide more accurate counts.

Updating OP's original code to use find (ignoring dot files for now):

ALL_LINES=$(find . -maxdepth 1 -type f | wc -l)
echo "Number of lines: ${ALL_LINES}"

REACHED_LINES=$(find . -maxdepth 1 -type f \( -iname '*.jpg' -o -iname '*.png' -o -iname '*.jpeg' \) | wc -l)
echo "Number of reached lines: ${REACHED_LINES}"

Upvotes: 0

tripleee
tripleee

Reputation: 189317

You don't need to and don't want to use ls at all here. See https://mywiki.wooledge.org/ParsingLs

Also, don't use uppercase for your private variables. See Correct Bash and shell script variable capitalization

#!/bin/bash

shopt -s extglob

read -e -p "Please enter basename: " -i "$PWD" basedir
all_files=( "$basedir"/* )
echo "Number of files: ${#all_files[@]}"

media_files=( "$basedir"/*(*.JPG|*.jpg|*.PNG|*.png|*.JPEG|*.jpeg) )
echo "Number of media files: ${#media_files[@]}"

Upvotes: 3

Related Questions