Bash Shell Script Inner loop of Nested Loops not working

Question

In attempting to rename a set of files with variations of (1-26), Left/Right and png/bmp I have the following script

#!/bin/bash

NUMBER_LIST=$(seq -f "%03g" 1 26)
EXT_LIST=("png" "bmp")
SIDE_LIST=("right" "left")
for NUMBER in $NUMBER_LIST
do
    for SIDE in $SIDE_LIST
    do
        for EXT in $EXT_LIST
        do  
            OLD="${SIDE}_${NUMBER}.${EXT}"
            NEW="${NUMBER}_${SIDE}.${EXT}"
            mv $OLD $NEW

        done
    done
done

Only files that are designated right and bmp get renamed. So it looks like it is only iterating through the outermost loop, and not iterating through the inner loops (just uses first element).

I've looked elsewhere on the net but am having trouble finding anything relevant.

Any guesses as to what might be wrong?

Thanks, Jeff

You're creating arrays (except for $NUMBER_LIST), so you must base your loops on arrays, e.g., for SIDE in "${SIDE_LIST[@]}". Additionally, I suggest not using all-uppercase variable names. Referencing an array variable as if it were a scalar - e.g., $SIDE_LIST - implicitly returns the array element with index 0, so you're looping over 1 value only. — mklement0
– mklement0, Commented Feb 5, 2016 at 19:28
I agreed with @mklement0 on all accounts, should resolve your issue. — jkdba
– jkdba, Commented Feb 5, 2016 at 19:34
@mklement0 That works. I'm not sure I understand how the case defines it as a scalar. Could you elaborate? Does the array declaration (ie $SIDE_LIST) need to be lowercase? The element declaration (ie for SIDE) need to be lowercase? Or the element reference (ie $SIDE)? — WoodMath
– WoodMath, Commented Feb 5, 2016 at 19:40
It's just best practice to reserve upper case variables for the shell, lower case for users. — miken32
– miken32, Commented Feb 5, 2016 at 19:43
@WoodMath: The case issue is unrelated, as implied by miken32's comment. It's how you reference an array variable that determines whether a scalar is returned: $SIDE_LIST is a scalar reference, whereas "${SIDE_LIST[@]}" returns the entire array. — mklement0
– mklement0, Commented Feb 5, 2016 at 19:46

chepner · Accepted Answer · 2016-02-05 19:59:54Z

Your first assignment

NUMBER_LIST=$(seq -f "%03g" 1 26)

assigns the string created from the output of seq to NUMBER_LIST. Your next two assignments

EXT_LIST=("png" "bmp")
SIDE_LIST=("right" "left")

create arrays. With an array variable, $EXT_LIST is equivalent to ${EXT_LIST[0]}, that is, it's just the first element of the array. To iterate over all the values of an array, use

for EXT in "${EXT_LIST[@]}"

Note that your outermost loop works because you are iterating over the whitespace-separated strings stored in NUMBER_LIST. That is, if NUMBER_LIST is 0 1 2 3, then NUMBER is set in order to 0, 1, 2, then 3. The same would apply to your faulty attempt at a loop. Consider

EXT_LIST=("png bmp" "gif tif")
for EXT in $EXT_LIST; do

EXT_LIST would expand to png bmp, and since the expansion is not quoted, EXT would be assigned png, then bmp. If you incorrectly wrote

for EXT in ${EXT_LIST[@]}; do

then EXT would take png, bmp, gif, then tif as its values. The equally incorrect

for EXT in "$EXT_LIST"; do

would produce a single iteration, with EXT assigned the value png bmp gif tif.

Jeff Y · Accepted Answer · 2016-02-05 20:02:26Z

0

Even better, because simpler and because arrays are not needed here, is changing the vars' values and leaving the dereferences as-is:

EXT_LIST="png bmp"
SIDE_LIST="right left"

answered Feb 5, 2016 at 20:02

Jeff Y

2,4661 gold badge14 silver badges18 bronze badges

1 Comment

mklement0 Over a year ago

That works in this case, but is generally not advisable, because the words in the variable values are subject to pathname expansion (globbing) when used in a for loop. Using arrays is the robust choice.

Walter A · Accepted Answer · 2016-02-06 11:57:13Z

Another way, avoiding the loops, is starting with filenames that are correct. Writing regex for 001 .. 026 is also possible, but i only check digits in the first find and check the number later.

regex_side="\(left\|right\)"
regex_ext="\(bmp\|png\)"
find . -maxdepth 1 -regextype sed -regex ".*/${regex_side}_[0-9]\{3\}\.${regex_ext}" | while read -r f; do
   filename="${f##*/}"
   side_number="${filename%.*}"
   side="${side_number%_*}"
   number="${side_number##*_}"
   if [ ${number} -lt 26 ] && [ ${number} -gt 0 ]; then
      printf "%s\n" "mv ${f} ${f%/*}/${number}_${side}.${f##*.}"
      # When you like the mv statement, delete the # on the next line
      # mv "${f}" "${f%/*}/${number}_${side}.${f##*.}"
   fi
done

EDIT: Added solution with sed.

You can avoid find with a sed solution. This will be slower, since bash will need to call sed for every file. I think the sed solution is easier to understand:

regex_side="\(left\|right\)"
regex_ext="\(bmp\|png\)"
for file in *; do
   # When your shell supports it: newname=$(sed -e "..." <<< "${file}")
   newname=$(echo "${file}" |
             sed -e "s/^${regex_side}_\([0-9]\{3\}\)\.${regex_ext}$/\2_\1.\3/")
   if [ "${file}" != "${newname}" ]; then
      number="${newname%%_*}"
      if [ ${number} -lt 26 ] && [ ${number} -gt 0 ]; then
         printf "%s\n" "mv ${file} ${newname}"
         # When you like the mv statement, delete the # on the next line
         # mv "${file}" "${newname}"
      fi
   fi
done

Collectives™ on Stack Overflow

Bash Shell Script Inner loop of Nested Loops not working

3 Answers 3

Comments

1 Comment

Comments

Your Answer

Hot Network Questions

Collectives™ on Stack Overflow

3 Answers 3

Comments

1 Comment

Comments

Your Answer

Sign up or log in

Post as a guest

Related