1

In the function below my counter works fine as long as an item is found in $DT_FILES. If the find is empty for that folder the counter gives me a count of 1 instead of 0. I am not sure what I am missing.

What the script does here is 1) makes a variable containing all the parent folders. 2) Loop through each folder, cd inside each one and makes a list of all files that contain the string "-DT-". 3) If it finds a file that doesn't not end with ".tif", it then copy the DT files and put a .tif extension to it. Very simple.

I count the number of times the loop did create a new file with the ".tif" extension.

So I am not sure why I am getting a count of 1 at times.

function create_tifs()
{
    IFS=$'\n'

    # create list of main folders
    LIST=$( find . -maxdepth 1 -mindepth 1 -type d )

    for f in $LIST
    do
        echo -e "\n${OG}>>> Folder processed: ${f} ${NONE}"
        cd ${f}

            DT_FILES=$(find . -type f -name '*-DT-*' | grep -v '.jpg')
            if (( ${#DT_FILES} ))
            then
                count=0
                for b in ${DT_FILES}
                do
                    if  [[ "${b}" != *".tif" ]]
                    then
#                        cp -n "${b}" "${b}.tif"
                        echo -e "TIF created ${b} as ${b}.tif"
                        echo
                       ((count++))
                    else
                        echo -e "TIF already done ${b}"

                    fi
                done
            fi

            echo -e "\nCount = ${count}"
}
Robin
  • 339
  • 1
  • 9
  • Your code contains several syntax errors, and some weird indentation. After fixing those, I can't repro the problem. But overriding `IFS` for the whole script (or even the whole function) looks very suspicious; probably try to avoid that? – tripleee Jun 15 '22 at 04:21
  • @tripleee Could you tell me what are the syntax errors? – Robin Jun 15 '22 at 04:31
  • 1
    I'm sure your shell does too, but the missing `done` is the main one. – tripleee Jun 15 '22 at 04:39

2 Answers2

2

I can't repro your problem, but your code contains several dubious constructs. Here is a refactoring might coincidentally also remove whatever problem you were experiencing.

#!/bin/bash

# Don't use non-portable function definition syntax
create_tifs() {
    # Don't pollute global namespace; don't attempt to parse find output
    # See also https://mywiki.wooledge.org/BashFAQ/020
    local f
    for f in ./*/; do
        # prefer printf over echo -e
        # print diagnostic messages to standard error >&2
        # XXX What are these undeclared global variables?
        printf "\n%s>>> Folder processed: %s %s" "$OG" "$f" "$NONE" >&2

        # Again, avoid parsing find output
        find "$f" -name '*-DT-*' -not -name '*.jpg' -exec sh -c '
            for b; do
                if  [[ "${b}" != *".tif" ]]
                then
#                   cp -n "${b}" "${b}.tif"
                    printf "TIF created %s as %s.tif\n" "$b" "$b" >&2
                    # print one line for wc
                    printf ".\n"
                else
                    # XXX No newline, really??
                    printf "TIF already done %s" "$b" >&2
                fi
            done
        fi' _ {} +
    # Missing done!
    done |
    # Count lines produced by printf inside tif creation
    wc -l |
    sed 's/.*/Count = &/'
}

This could be further simplified by using find ./*/ instead of looping over f but then you don't (easily) get to emit a diagnostic message for each folder separately. Similarly, you could add -not -name '*.tif' but then you don't get to print "tif already done" for those.

Tangentially perhaps see also Correct Bash and shell script variable capitalization; use lower case for your private variables.

Printing a newline before your actual message (like in the first printf) is a weird antipattern, especially when you don't do that consequently. The usual arrangement would be to put a newline at the end of each emitted message.

tripleee
  • 175,061
  • 34
  • 275
  • 318
  • Thank you. I read all this. The missing done was my error in cutting and pasting. I wasn't aware of all these very rigid rules but I see why. This doesn't resolve the count issue but helped me to stop using find and use local variable, so i voted it up. – Robin Jun 15 '22 at 15:06
0

If you've got Bash 4.0 or later you can use globstar instead of (the error-prone) find. Try this Shellcheck-clean code:

#! /bin/bash -p

shopt -s dotglob extglob nullglob globstar

function create_tifs
{
    local dir dtfile
    local -i count
    for dir in */; do
        printf '\nFolder processed: %s\n' "$dir" >&2

        count=0
        for dtfile in "$dir"**/*-DT-!(*.jpg); do
            if [[ $dtfile == *.tif ]]; then
                printf 'TIF already done %s\n' "$dtfile" >&2
            else
                cp -v -n -- "$dtfile" "$dtfile".tif
                count+=1
            fi
        done
        printf 'Count = %d\n' "$count" >&2
    done

    return 0
}
  • shopt -s ... enables some Bash settings that are required by the code:
    • dotglob enables globs to match files and directories that begin with .. find shows such files by default.
    • extglob enables "extended globbing" (including patterns like !(*.jpg)). See the extglob section in glob - Greg's Wiki.
    • nullglob makes globs expand to nothing when nothing matches (otherwise they expand to the glob pattern itself, which is almost never useful in programs).
    • globstar enables the use of ** to match paths recursively through directory trees.
  • Note that globstar is potentially dangerous in versions of Bash prior to 4.3 because it follows symlinks, possibly leading to processing the same file or directory multiple times, or getting stuck in a cycle.
  • The -v option with cp causes it to print details of what it does. You might prefer to drop the option and print a different format of message instead.
  • See the accepted, and excellent, answer to Why is printf better than echo? for an explanation of why I used printf instead of echo.
  • I didn't use cd because it often leads to problems in programs.
pjh
  • 6,388
  • 2
  • 16
  • 17
  • Thank you for your solution. Very clean and simple. It resolves the count issue. I am getting a reality readjustment with my deliberate use of "find" and "echo" everywhere. I read all your link. Very interesting. Can you tell me what the -p on the shebang does? – Robin Jun 15 '22 at 15:08
  • One thing I like about echo is that you can add colors to better see what is happening when the script is running. Can you do the same thing with print or printf? – Robin Jun 15 '22 at 15:19
  • @Robin, the `-p` in the shebang prevents Bash from reading configuration files and environment variables that could change how it behaves (e.g. by defining functions that override standard utilities). It makes Bash programs more reliable, and reduces the "it works for me" effect. It may also avoid some security issues (see [Shell Script Security - Apple Developer](https://developer.apple.com/library/archive/documentation/OpenSource/Conceptual/ShellScripting/ShellScriptSecurity/ShellScriptSecurity.html)). – pjh Jun 15 '22 at 15:47
  • 1
    @Robin, `echo` has no built-in support for colorization. It requires terminal control characters to be explicitly output. That can just as easily be done with `printf`. – pjh Jun 15 '22 at 15:50
  • Thank you. Yes, that's what I meant but colorization. I am going to try with printf. – Robin Jun 15 '22 at 16:24
  • 1
    @Robin, [Using colors with printf](https://stackoverflow.com/q/5412761/4154375) looks potentially useful. – pjh Jun 15 '22 at 17:52
  • I really appreciate this help. printf does print terminal control characters totally fine. So I am using printf now. – Robin Jun 15 '22 at 20:54