Producing recursive directory listings

Question

This produces recursive directory listings(it's intended to do other things, so there will be something else instead of echo, but it isn't important), yet it isn't very intuitive. How can I improve it?

#! /bin/bash
deploy ()
{
    for file in $(ls $1); do
        if [ ! -d "$2$file" ]; then
            echo ${2}${file}
        else
            deploy $2$file "$2$file/"
        fi
    done;
}

deploy .

Output example:

tmp/kdecache-z/http/ba9c27404980248a61dcdadd35f5f52395b8bdde
tmp/kdecache-z/http/scoreboard
tmp/kdecache-z/icon-cache.kcache
tmp/kdecache-z/ksycoca4
tmp/kdecache-z/ksycoca4stamp
tmp/kdecache-z/plasma-svgelements-.customized1
tmp/kdecache-z/plasma-svgelements-internal-system-colors
tmp/kdecache-z/plasma_theme_.customized1.kcache
tmp/kdecache-z/plasma_theme_internal-system-colors.kcache
tmp/kdecache-z/Активные
tmp/kdecache-z/напоминанияrc
tmp/kdecache-z/Устаревшие
tmp/kdecache-z/напоминанияrc
tmp/kdecache-z/Шаблоны
tmp/kdecache-z/напоминанийrc
tmp/kde-z/xauth-1000-_0
tmp/ksocket-z/kdeinit4__0
tmp/ksocket-z/kio_http_cache_cleaner
tmp/ksocket-z/klauncherhX2255.slave-socket
tmp/ksocket-z/KSMserver__0
tmp/pulse-PKdhtXMmr18n/native
tmp/pulse-PKdhtXMmr18n/pid
tmp/qtsingleapp-homezS-8560-3e8
tmp/qtsingleapp-homezS-8560-3e8-lockfile
tmp/skype-2664/DbTemp./temp-KXp68f252Xx0kBMtqNdj7NFo
tmp/skype-2664/DbTemp./temp-NAailvY24hLGYNrZiJ99QIgH
tmp/sni-qt_skype_2664-v6Xyna/icons/hicolor/24x24/apps/skype_2664_2d1ee5482260fd9cd180b32787792683.png
tmp/sni-qt_skype_2664-v6Xyna/icons/hicolor/24x24/apps/skype_2664_37e170fc54e7355d9d298917e74f9ea9.png
tmp/sni-qt_skype_2664-v6Xyna/icons/hicolor/24x24/apps/skype_2664_602c84fa0f4d61c64f770495a500279e.png
tmp/sni-qt_skype_2664-v6Xyna/icons/hicolor/24x24/apps/skype_2664_f2fc4a539a7b9553f5b35241d1154e84.png
tmp/z.socket

P.S. It is working perfectly for me so I just want to make it more beautiful.

Answer 1

In fairness, this task is relatively simple, and your solution is actually pretty good.

Shell scripts are often just a hack to solve a specific task, and, your script is much better than many that I see that do that.

There are two things to consider in your script. The first is whether it can be improved. The second is whether there's a better way.

Now, about improving it. I would:

• remove the need for the second parameter
• name the parameters,
• include the / in a different place
• use quotes around variables in specific places.
• reverse the if-blocks to get rid of the 'negate' condition.

Consider the following:

deploy () {
    local dir="$1"
    for file in $(ls "$dir/" ); do
        # Fully qualified file
        fqfile="$dir/$file"
        if [ -d "$fqfile" ]; then
            deploy "$fqfile"
        else
            echo "$fqfile"
        fi
    done;
}

Introducing the fqfile allows the simplifcation of other variables too.

Now, having said all that, your command could easily be solved with the find command:

find . -type f -exec echo {} \;

The above command will recursively find all files below ., and echo them.

Answer 2

This on top of what @rolfl already said.

It's a bad practice to parse the output of ls. Iterating over the output of $(ls ...) falls under that category. For example, the script will not work for files with spaces in the name.

A better way would be using standard shell expansion, like this:

#!/bin/bash
deploy() {
    local dir="$1"
    for path in "$dir"/*; do
        if [ -d "$path" ]; then
            deploy "$path"
        elif [ -f "$path" ]; then
            echo "$path"
        fi
    done
}

Note that I had to add an extra elif, otherwise empty directories would be included too. But this extra condition is probably a good thing, as it seems you want to work with files anyway, and this code makes that point explicit.

Lastly, some minor coding style issues:

• Don't put a semicolon at the end lines (like you did in done;)
• Don't put a space after #! in the shebang line
• The common way to write function declarations is fname() { rather than fname () and a brace on the next line