3

I wrote a simple bash script (I'm pretty new to it, you know!) to accept a directory as its argument and print its listing, testing for files and directories. Here's how I approached it:

#!/bin/bash
# Lists all files and directories (non-recursively) in a specified directory

if [ $# -gt 1 ]; then
    echo "Error. Please specify only one directory. ($#)"
    exit
fi

if [ -z $1 ]; then
    echo "No directory specified. Exiting."
    exit
fi

echo "Listing for $1:"

$dirs=`ls $1`
echo "Dirs: $dirs" # Just to confirm if all is well

# Loop through and print
for i in $dirs;
do
    if [ -f $i ]; then
        echo "File: $i"
    elif [ -d $i ]; then
        echo "Directory: $i"
    fi
done

The problem is in my for loop. When I run this script and feed it my home directory, I get this error:

./list_files_and_dirs.sh: line 16: =Calibre: command not found

I know I'm making a mistake in command substitution involving variables, but I just don't know what. Someone please help!

================= Update =================

Here's the new (final section) code as per inputs from answers:

dirs=`ls "$1"`
#echo "Dirs: $dirs" # Just to confirm if all is well

IFS=$'\n'

# Loop through and print
for i in $dirs;
do
    if [ -f "$i" ]; then
        echo "File: $i"
    elif [ -d "$i" ]; then
        echo "Directory: $i"
    fi
done
  • You might want to add else echo "Other: $i" to handle things that aren't files or directories.  (Or do you want to just ignore them?) – G-Man Sep 15 '15 at 19:55
9

Note: I've assumed you're teaching yourself Bash. Do not use this code in production. find "$directory" -maxdepth 1 -type d will get you the directories, -type f will get you the files.

Since it is complaining about line 16, let's have a look at it:

$dirs=`ls $1`

If you want to assign to a variable, you should not include the $. I'm guessing you meant:

dirs=`ls $1`

What's happening now, is this:

  • $dirs is probably empty, so substituted with nothing.
  • The ls command is run, and its output substituted in the 'command'.
  • The first file in your directory is called Calibre, leaving you with the command: =Calibre x y z ...
  • =Calibre is not a valid command, thus the error.

However, we're not there yet: this will go wrong if there are spaces in any of the filenames. To fix that, you need to do more:

  • You need to include IFS=$'\n' somewhere before the for loop. This sets the fields separator to the linefeed character, which stops the for loop from splitting up the files on the spaces or tabs (it's technically possible for a file to have a linefeed in its filename as well: you're unlikely to ever encounter such a file and it won't cause serious problems in this case if you do run into such a file, but it's worth being aware of the possibility in case it ever does matter).
  • In order for a filename with spaces not to turn into several different arguments to -f and -d, you need to put quotes $i. (So: [ -f "$i" ] and [ -d "$i" ]. In order to support directories with spaces in them, you should do the same for $1 where it is used.
  • I put double quotes everywhere and set IFS='' before the for loop. Now my for loop prints nothing, though a simple echo "$i" shows the listing. Any idea what's wrong? Should I update the code in question or should I post a new question? – ankush981 Sep 13 '15 at 16:51
  • @dotslash: My mistake. It should be IFS=$'\n'. I've fixed it in the answer as well. – marinus Sep 13 '15 at 16:56
  • I did it but still no output. Please see the updated code in question. All I get is Listing for /home/ankush: and then nothing. – ankush981 Sep 13 '15 at 16:59
  • 1
    @dotslash: Don't quote $dirs in the for loop. In that case, you actually do want it to turn into separate arguments, because you want to consider each part separately. – marinus Sep 13 '15 at 17:00
  • Did that but still nothing. :( Mind running this on your system, please? :-) – ankush981 Sep 13 '15 at 17:03
5

Marinus already explained why you were getting that error. I would like to point out that there's no need for ls here. That way, you avoid all the problems that come with parsing its output. You could just do:

#!/bin/bash
# Lists all files and directories (non-recursively) in a specified directory

if [ "$#" -gt 1 ]; then
    echo "Error. Please specify only one directory. ($#)"
    exit
fi

if [ -z "$1" ]; then
    echo "No directory specified. Exiting."
    exit
fi

echo "Listing for $1:"

# Loop through and print
for i in "$1"/*;
do
    if [ -f "$i" ]; then
        echo "File: $i"
    elif [ -d "$i" ]; then
        echo "Directory: $i"
    fi
done
  • I'll be damned! What a neat trick! O.O – ankush981 Sep 13 '15 at 17:08
  • @G-Man lol, I should indeed. Thanks :) – terdon Sep 15 '15 at 21:42

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

Not the answer you're looking for? Browse other questions tagged or ask your own question.