Confusing syntax error with grep

gumpu has asked for the wisdom of the Perl Monks concerning the following question:

Hello fellow monks,

I am working on a little module to scan a whole tree of sourcefiles for use with Vim. One of the methods is:

sub parseDir {
    my $self    = shift;
    my $path    = shift;  # the path to the directroy we are
                          # currently scanning.
    my $out     = shift;  # the file we write the results to
    my $target  = shift;  # the thing we are searching for

    my $dir = IO::Dir->new($path) or die "can't open $path : $!";
    my @files = $dir->read();
    $dir->close();
    # Scan all interesting files
    grep(   ( -f "$path/$_" ) and 
            ( $_ =~ m/\.(java|xml|cpp|c|h|inc|pas)$/i ) and
            ( $self->Scan_File( "$path/$_", $out, $target ) )  , @file
+s );
    # Also scan any subdirectories.
    grep( ( ( -d "$path/$_" ) and 
            ( $_ !~ /^\.+$/ ) and # but avoid . and ..
            ( $self->parseDir( "$path/$_", $out, $target ) ) ), @files
+ );
}
[download]

The odd thing is that the second grep works fine while the first grep results in a: 'Not enough arguments for grep at scan.pl line 63, near "@files )"'. The only difference between the two is that the first has a '(' ')' pair less. I am quite puzzled why this makes a difference, since both

 
  ( expression ) and ( expression ) and ( expression)
[download]

and

 
  ( ( expression ) and ( expression ) and ( expression) )
[download]

seem to me to be syntactically correct expression.

Can anybody shed some light on this?

Have Fun

Comment on Confusing syntax error with grep Select or Download Code

Replies are listed 'Best First'.
Re: Confusing syntax error with grep by etcshadow (Priest) on Aug 12, 2004 at 14:15 UTC
Don't try to futz with `grep (expr), @list;` [download] or `grep ((expr), @list);` [download] or any such... you just confuse the hell out of the parser and out of someone trying to read it. Instead use this syntax for grep: `grep { block } @list;` [download] Those are curly braces setting off a block, not parenthesese setting off an expression. So your code would become: `grep { ( -f "$path/$_" ) and ( $_ =~ m/\.(java\|xml\|cpp\|c\|h\|inc\|pas)$/i ) and ( $self->Scan_File( "$path/$_", $out, $target ) } @files ); # Also scan any subdirectories. grep { ( ( -d "$path/$_" ) and ( $_ !~ /^\.+$/ ) and # but avoid . and .. ( $self->parseDir( "$path/$_", $out, $target ) ) } @files;` [download] However, it also doesn't make sense because you are using grep in a void context. grep is meant to filter a list (reduce some of the contents out of list1 into list2). If you just want to apply a block of code against every item in a list, think of using map or foreach. Most folks here will complain, likewise, if you use map in a void context (for the same reason, sort of, that I complain about you using grep... the purpose of map (in perl) is to mutate a list... although in some other language paradigms one might think of map as making sense in a void context... but that's another argument). `------------ :Wq Not an editor command: Wq` [download]	[reply] [d/l] [select]
Re^2: Confusing syntax error with grep by gumpu (Friar) on Aug 12, 2004 at 17:25 UTC
"However, it also doesn't make sense because you are using grep in a void context. grep is meant to filter a list (reduce some of the contents out of list1 into list2). If you just want to apply a block of code against every item in a list, think of using map or foreach. Most folks here will complain, likewise, if you use map in a void context" Guess I agree on using map instead of grep. But I find `foreach` ugly. `foreach ( @alist ) { if ( condition( $_ ) ) { do_something( $_ ); } }` [download] Is far less pretty to the eye than: `map { condition( $_ ) and do_something( $_ ) } @alist;` [download] OK, map ( or grep ) has to create a list which is not used. I used to worry about that too. However, given the size of an list in an average problem and the amount of memory that is nowadays present in a computer it is no longer relevant to worry about it. For instance in my case the list holds the names of all files in a single directory. That will be 1 to 1000 elements or so. Compared to the 256 Mbyte that is available in my computer this is peanuts. So I stopped worrying about it and instead worry about making it readable. Of course it would be even nicer if there was a construct in perl like: `{ do_something( $_) } forall ( @alist ) where { condition( $_ ) }` [download] Have Fun	[reply] [d/l] [select]
Re^3: Confusing syntax error with grep by ysth (Canon) on Aug 12, 2004 at 18:48 UTC
Of course it would be even nicer if there was a construct in perl like: `{ do_something( $_) } forall ( @alist ) where { condition( $_ ) }` You mean like: `do_something( $_ ) for grep condition( $_ ), @alist;` ? Update: `condition( $_ ) and do_something( $_ ) for @alist;` is going to be faster because it doesn't have to loop twice.	[reply] [d/l] [select]
Re^4: Confusing syntax error with grep by gumpu (Friar) on Aug 12, 2004 at 19:45 UTC
Re^3: Confusing syntax error with grep by gumpu (Friar) on Aug 12, 2004 at 18:38 UTC
Come to think of it: `foreach ( @alist ) { if ( condition( $_ ) ) { do_something( $_ ); } }` [download] can be written as `foreach ( @alist ) { condition( $_ ) and do_something( $_ ) }` [download] Which is as pretty as the `map` version. The point about worrying too much about memory still stands though. Have Fun	[reply] [d/l] [select]
Re^2: Confusing syntax error with grep by revdiablo (Prior) on Aug 12, 2004 at 16:42 UTC
However, it also doesn't make sense because you are using grep in a void context. Not necessarily. The first grep is in void context, but the last one may or may not be -- it depends on how the subroutine is called. ~~My guess is he took some working code and added a second `grep` at the end, without really realising how it worked.~~ Update: It's pointless to guess how it happened. Kindly ignore that sentence. :-)	[reply] [d/l]
Re: Confusing syntax error with grep by rsteinke (Scribe) on Aug 12, 2004 at 14:06 UTC
I believe 'and' actually has a lower operator precedence than the comma operator, so your first expression is parsing as `( expression ) and ( expression ) and ( expression, @files )` [download] Try replacing 'and' with '&&' and see if that helps. Ron Steinke <rsteinke@w-link.net>	[reply] [d/l]
Re^2: Confusing syntax error with grep by ikegami (Patriarch) on Aug 12, 2004 at 15:36 UTC
No, that would only be the case if the (last set of) parens weren't there.	[reply]
Re^3: Confusing syntax error with grep by gumpu (Friar) on Aug 12, 2004 at 16:07 UTC
Nope he is correct, it was a problem with 'and' having lower precedence than ',' `grep ( ( expression) and ( expression) and ( expression ) , @files )` [download] Is interpreted as `grep ( ( expression) and ( expression) and ( ( expression ) , @files ) + )` [download] According to `perldoc -f grep`, the construct `grep( expression, list )` is allowed. But it is indeed confusing to man and machine!. Have Fun	[reply] [d/l] [select]
Re^4: Confusing syntax error with grep by ikegami (Patriarch) on Aug 12, 2004 at 16:16 UTC


No such thing as a small change
	PerlMonks