git/list[1] front-page[2] threads[3] people[4] search[5] about
 

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

From
Junio C Hamano <junkio@cox.net>
Date
Dec 18, 2005, 19:37 UTC
Message-ID
<7vr78amc19.fsf@assigned-by-dhcp.cox.net>
In-Reply-To
<20051218145621.GX22159@pasky.or.cz>
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.

Show 9 quoted lines
> @@ -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;
}
Previous: Petr BaudisNext: Linus Torvalds
Message 3 of 5 in “git-grep: convert from bash to sh”
  1. git-grep: convert from bash to shTimo Hirvonen, Dec 18, 2005
  2. Petr BaudisDec 18, 2005
  3. Junio C HamanoDec 18, 2005
  4. Linus TorvaldsDec 18, 2005
  5. Timo HirvonenDec 18, 2005

Read the whole thread, see it on lore, or plain text.

$ cat FOOTERMessages come from the public archive at lore.kernel.org/git, fetched every hour. The front page is chosen and written each morning by an AI editor and can be wrong; the threads themselves are the record. About and API. For agents: an MCP server at https://gitlist.dev/mcp, and any thread, story or person page as Markdown by adding .md to its URL (or sending Accept: text/markdown). Details in /llms.txt.