I have the following code:


for f in `find . ! -type d`;
        line=`grep -i 'todo' $f | sed 's/^[ \t]*//'`
        if [ $line ]; then
                echo "$f:"
                echo "$line"

but the condition is not working as I would expect it, I need it to only work if something other than an empty string (or nothing) is returned by the command.

+1  A: 
if [ -n $line ]

Checks if $line is empty string.

Alberto Zaccagni
This did not work, I'm guessing that the value being returned is not empty, maybe null or whitespace.

This is one I like:

if [ ${PIPESTATUS[0]} -eq 0 ]; then echo lines found; fi

it takes the exit status of the grep. man page tells:

   Normally,  the  exit  status  is  0  if  selected lines are found and 1 otherwise.
This produced the same results as running the script without the condition.
sorry, doesn't work since the pipe is done in a sub-shell (because of the \`...\`). Too bad, go for one of the other options!
+3  A: 

As Alberto pointed out, it's common to use -n to check to see if a string has a non-zero length, but in the case where the string may be empty or contain only whitespace, you'll want to quote the variable:

if [ -n "$line" ] # ...

Similarly, you can check to see if it has a zero length with -z:

if [ -z "$line" ] # ...

Negation is also supported when it makes more sense:

if [ ! -z "$line" ] # ...

Edit: And since you're using bash, you can actually use some other nice features such as:

  1. Double brackets (e.g. [[ expr ]]), and
  2. Regular expressions (e.g. [[ cat =~ ca[bat] ]])

See the conditional constructs section in man bash for more information.

Kaleb Pederson
+3  A: 

Some ideas:

  • You need to quote your $f in the grep line to deal with files with spaces in their names.
  • Perhaps you should use -f rather than ! -d, otherwise your find may hang on fifos.
  • For me the \t escape didn't work as a tab match, it consumed a leading t in a test case file line, so I just inserted a raw tab character in the script instead.
  • Use the double square bracket test otherwise you need an operator in there, or at least some quotes.

A slight variation on your script would then be:

find . -type f | while read f
        # raw tab in here ------------------- v
        line=`grep -i 'todo' "$f" | sed 's/^[  ]*//'`
        if [[ $line ]]; then
                echo "$f:"
                echo "$line"
martin clayton
+1 for `while read`
`while read` will fail if the file name contains newlines.
If your shell's `read` builtin supports -r (raw mode) and -d (change delimiter), this should handle any bizarre filenames you throw at it: `find . -type f -print0 | while IFS="" read -r -d $'\000' f; do ...` (note that the IFS setting only applies to the `read` command, not to the contents of the loop... except on a few buggy shells).
Gordon Davisson
+2  A: 

The old school notation (usable with archaic things like Bourne Shell, and not so archaic things such as Korn shell) is:

for f in `find . ! -type d`
    line=`grep -i 'todo' $f | sed 's/^[ \t]*//'`
    if [ -n "$line" ]
            echo "$f:"
            echo "$line"

You could speed up the overall processing, though, by using:

find . ! -type d -print0 | xargs -0 grep -i 'todo' /dev/null |
perl -p -e '($name) = split /:/;
            if ($saved ne $name) { $saved = $name; print "$name:\n"; }

The find is modified to handle spaces in names; the xargs also handles spaces in names, and runs the grep command on /dev/null plus some files - which ensures that matching lines are preceded by the file name. The perl script splits the line at the first colon - so file names containing colons are not good news. It compares that name with what it had saved before; if they are different, then it prints out the new name, colon and newline to give you the file heading. Then it removes the file name and trailing spaces before (automatically) printing the line. (Don't use '-w' with Perl; it complains about $saved being uninitialized. If you must (it is good practice), add BEGIN { $saved = ""; }.)

This is likely to perform better than the shell loop because it invokes grep just a few times rather than once per file, and doesn't invoke the sed at all (instead of once per file).

Jonathan Leffler

Use POSIX character classes if you can:

# cf. http://en.wikipedia.org/wiki/Regular_expression#POSIX_character_classes

sed 's/[[:blank:]]/-wasTabOrSpace-/g' <<< $'abc\t def'

Since we have a subshell because of the `...` or $(...) construct respectively, we can exit the subshell like any other (parent) shell with a specified PIPESTATUS-based exit code! :-)

# simple test
var="$(echooo hello 2>/dev/null | cat)"
echo ${PIPESTATUS[0]}  # 0

var="$(echooo hello 2>/dev/null | cat; exit ${PIPESTATUS[0]})"
echo ${PIPESTATUS[0]}  # 127