grep command not working in conditional statement

Question

I've a bash script that's supposed to go over all the filenames and print them only those that contain a certain letter (let's say it's 'o').

What I've done:

command=`ls -l`
for file in $command
do
  if $file | grep 'o' ;
  then
  echo "$file"
  fi
done

This doesn't work, it says 'command not found' on the line:

if $file | grep 'o' ;

I looked up several other answers here and they're doing the same thing I am, i.e. no parentheses for the if-statement, same syntax etc.

What am I overlooking?

Answer 1

$file | grep -o executes the command specified by the value of file and pipes its output to grep. But that's clearly not what you wanted.

If you want to list files that contain o

You meant the value of file to be an input file for grep, not a command to execute. So you need an input redirection, not a pipe.

if grep 'o' <"$file"

grep reads from standard input if it doesn't receive a file name on its command line. You can also pass a file name on the command line; this does the same thing.

if grep 'o' -- "$file"

For why the double quotes, and why the -- in the second method, see Why does my shell script choke on whitespace or other special characters?

To suppress the output from grep, use the -q option, or redirect the output to /dev/null.

if grep -q 'o' <"$file"

or

if grep 'o' <"$file" >/dev/null

Another problem with your script is that command=`ls -l` sets the variable command to the output of the command ls -l, which is not a list of file names. If you recall Why does my shell script choke on whitespace or other special characters?, you'll remember that it's impossible to store a list of file names in a string variable: all the file names are jumbled together. Here, it's even worse: there's junk like permissions, dates, etc. The way to get a list of file names in a shell script is to use a wildcard pattern.

for file in *
do
  if grep -q 'o' <"$file"
  then
    echo "$file"
  fi
done

All of this is equivalent to

grep -l 'o' -- *

If you want to list files whose name contains o

This feature is built into the shell, with wildcard patterns. The pattern *o* matches file names that contain o, so to list them, you only need

ls -l -- *o*

The -- is necessary in case one of the file names begins with -, otherwise ls might interpret the file name as an option.

If this is a learning exercise on the use of grep, and you want to pass the file name as input to grep, you need to use the echo command.

echo "$file" | grep 'o'

The echo command might mangle some file names. To be safe, use printf. For more on this, and on the use of double quotes, see Why does my shell script choke on whitespace or other special characters?

printf '%s\n' "$file" | grep 'o'

As above, don't parse the output of ls.

for file in *
do
  if printf '%s\n' "$file" | grep -q 'o'
  then
    printf '%s\n'
  fi
done

If your file names don't contain any newlines, you could reduce this to.

for file in *
do
  printf '%s\n' "$file" | grep 'o'
done

Once again, the loop is pointless here and you can just use

printf '%s\n' *o*

to print the matching file names on separate lines. The use of grep would only be justified if you needed a regular expression that your shell's wildcard patterns aren't powerful enough to express.

Answer 2

You're trying to execute $file. Instead, you must echo it:

# ...
if echo "$file" | grep 'o' ;
# ...

Note that the grep will already print the filename, so you should silence it (e.g. grep -q 'o' or grep 'o' >/dev/null).

You're also passing -l to ls, which you don't want to do. ls -l prints the file name and attributes, and you'll be matching against them.

Also, you might want to use quotes if you expect it to work with any file, but that's not the source of the problem.

P.S.: As others have pointed out, you shouldn't use ls to begin with.

Answer 3

You should not rely on ls output at all. ls is meant to show human friendly output and not to be used within scripts. See the following example;

for f in ./*
  do 
   if [[ $f = *o* ]]; then 
    printf '%s\n' "$f"
   fi
 done

Answer 4

First off, that isn't how you pipe something to grep. Also, it is generally considered better form to separate commands from the conditional and just check return codes.

Something like the following works perfectly:

command=`ls -l`
for file in "$command"
do
  echo "$file" | grep 'o'
  if [ $? -eq 0 ]; then
    echo "$file"
  fi
done