Re: [PATCH] git-grep: convert from bash to sh

[Junio C Hamano <junkio@cox.net>]

Petr Baudis <pasky@suse.cz> writes:

> I'm kind of sensitive to this stuff. I still passionately hate scp
> making me to double-quote remote filenames. It's just evil.

OK.

>  	--ignored|--exclude=*|\
>  	--exclude-from=*|\--exclude-per-directory=*)
> -		git_flags=("${git_flags[@]}" "$1")
> +		git_flags="$git_flags '$1'"

SQs in --exclude= or --exclude-from= parameter?  E.g.

	git grep --exclude-from="/cygdrive/c/pasky's patterns/cvsignore" \
		-e foobar

>  	-A|-B|-C|-D|-d|-f|-m)
> -		flags=("${flags[@]}" "$1" "$2")
> +		flags="$flags '$1' '$2'"
>  		shift

SQs in $2 for -f.  About other flags, -[ABCm] take only numbers
and -[Dd] take read/skip/recurse, so as long as your user does
not screw up you are OK.  And malicious users are shooting
themselves in the foot here so we might not care too much being
loose.

> @@ -46,5 +46,6 @@ done
>  [ "$pattern" ] || {
>  	usage
>  }
> -git-ls-files -z "${git_flags[@]}" "$@" |
> -	xargs -0 grep "${flags[@]}" -e "$pattern"
> +pattern="$(echo "$pattern" | sed 's/[\\"]/\\&/g')"
> +eval git-ls-files -z "$git_flags" '"$@"' |
> +	eval xargs -0 grep "$flags" -e '"$pattern"'

You are not expanding $pattern in the outer shell that builds
eval arguments (letting the inner shell expand "$pattern" and
use it), so I do not think you need that sed script to muck with
it there.  The eval'ed string is literally "$pattern" including
double quotes because you have sq around the last parameter to
"eval" command, and eval would do the right thing, no?

Actually the use of sed script there is actively wrong.  When a
file contains these lines:

        printf("%s is not there\n");
        printf("\"%s\" is not there\n");

the command to pick up the second line should be:

	$ git-grep -e '\\"%s\\"'

but I think your version passes \\\\\"%s\\\\\ to underlying
grep.  Removing that single line would make it do the right
thing, I think.


Enough nitpicking.  If you want to stay in shell and want to
avoid shell array, you would either need to be a bit more
careful if you are going to use eval, or do something like what
I originally did, which made Linus' brain shut down and had him
gouge his eyes with a spoon.  It is the one that this one is a
response to:

	http://marc.theaimsgroup.com/?l=git&m=112656882627760

Not that I am suggesting the latter is better than a bit more
careful 'eval' construction ;-).

I am not happy with that [@] thing either. I have the attached
laying around in my working tree --- I do not remember if I
finished it or not; it's a WIP when I thought about this issue
last time and wondered if we might be better off rewriting the
whole thing.

-- >8 --
#!/usr/bin/perl
#
# Copyright (c) 2005 Junio C Hamano
#

use strict;
my (@git_flag, @grep_flag, $pattern);
my $nargs = 100;

my (%git_ls_files_flag) =
    map { '--' . $_ => 1 }
    qw{cached deleted others kiled ignored
       exclude= exclude-from= exclude-per-directory=};

sub is_git_flag {
    my ($arg) = @_;
    $arg =~ s/^([^=]*)=/$1/;
    return (exists $git_ls_files_flag{$arg});
}

while (@ARGV) {
    my ($arg) = shift @ARGV;
    if (is_git_flag($arg)) {
	push @git_flag, $arg;
    }
    elsif ($arg eq '-e') {
	$pattern = shift @ARGV;
    }
    elsif ($arg =~ /^-[ABCDdfm]$/) {
	push @grep_flag, $arg, (shift @ARGV);
    }
    elsif ($arg =~ /^--$/) {
	shift @ARGV;
	last;
    }
    elsif ($arg =~ /^-/) {
	push @grep_flag, $arg;
    }
    elsif (! defined $pattern) {
	$pattern = $arg;
	last;
    }
    else {
	last;
    }
}

my $ih;
open $ih, '-|', qw(git-ls-files -z), @git_flag, @ARGV;
$/ = "\0";
my @args = ();
while (<$ih>) {
    chomp;
    push @args, $_;
    if ($nargs <= @args) {
	system 'grep', @grep_flag, '-e', $pattern, @args;
	@args = ();
    }
}
close $ih;
if (@args) {
    system 'grep', @grep_flag, '-e', $pattern, @args;
}