current community

your communities

more stack exchange communities

Stack Exchange
tour help
Take the 2-minute tour ×
Unix & Linux Stack Exchange is a question and answer site for users of Linux, FreeBSD and other Un*x-like operating systems. It's 100% free, no registration required.
up vote 2 down vote favorite

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
share|improve this question

2 Answers 2

up vote 8 down vote accepted

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.
share|improve this answer
    
Great answer! Methodical and clear. :) –  dotslash yesterday
    
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? –  dotslash yesterday
    
@dotslash: My mistake. It should be IFS=$'\n'. I've fixed it in the answer as well. –  marinus yesterday
    
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. –  dotslash yesterday
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 yesterday
up vote 5 down vote

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
share|improve this answer
    
I'll be damned! What a neat trick! O.O –  dotslash yesterday

Your Answer

 
discard

By posting your answer, you agree to the privacy policy and terms of service.

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