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

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

[Timo Hirvonen]

sh does not support arrays so we have to use eval instead.

Signed-off-by: Timo Hirvonen <tihirvon@gmail.com>

---

 git-grep.sh |   14 +++++++-------
 1 files changed, 7 insertions(+), 7 deletions(-)

11c29a066288c5f05a67ff0d46e9ce17cd7a37da
diff --git a/git-grep.sh b/git-grep.sh
index 2ed8e95..2f0a297 100755
--- a/git-grep.sh
+++ b/git-grep.sh
@@ -8,21 +8,21 @@ SUBDIRECTORY_OK='Yes'
 . git-sh-setup
 
 pattern=
-flags=()
-git_flags=()
+flags=
+git_flags=
 while : ; do
 	case "$1" in
 	--cached|--deleted|--others|--killed|\
 	--ignored|--exclude=*|\
 	--exclude-from=*|\--exclude-per-directory=*)
-		git_flags=("${git_flags[@]}" "$1")
+		git_flags="$git_flags '$1'"
 		;;
 	-e)
 		pattern="$2"
 		shift
 		;;
 	-A|-B|-C|-D|-d|-f|-m)
-		flags=("${flags[@]}" "$1" "$2")
+		flags="$flags '$1' '$2'"
 		shift
 		;;
 	--)
@@ -31,7 +31,7 @@ while : ; do
 		break
 		;;
 	-*)
-		flags=("${flags[@]}" "$1")
+		flags="$flags '$1'"
 		;;
 	*)
 		if [ -z "$pattern" ]; then
@@ -46,5 +46,5 @@ done
 [ "$pattern" ] || {
 	usage
 }
-git-ls-files -z "${git_flags[@]}" "$@" |
-	xargs -0 grep "${flags[@]}" -e "$pattern"
+eval git-ls-files -z "$git_flags" '"$@"' |
+	eval xargs -0 grep "$flags" -e '"$pattern"'
-- 
0.99.9.GIT

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

[Petr Baudis]

Dear diary, on Sun, Dec 18, 2005 at 02:26:39PM CET, I got a letter
where Timo Hirvonen <tihirvon@gmail.com> said that...
> sh does not support arrays so we have to use eval instead.
> 
> Signed-off-by: Timo Hirvonen <tihirvon@gmail.com>

This version also makes it work properly with patterns containing quotes
and backslashes (not so unusual when you grep for C strings).

Signed-off-by: Petr Baudis <pasky@suse.cz>

---

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

diff --git a/git-grep.sh b/git-grep.sh
index 2ed8e95..7e9e5bf 100755
--- a/git-grep.sh
+++ b/git-grep.sh
@@ -8,21 +8,21 @@ SUBDIRECTORY_OK='Yes'
 . git-sh-setup
 
 pattern=
-flags=()
-git_flags=()
+flags=
+git_flags=
 while : ; do
 	case "$1" in
 	--cached|--deleted|--others|--killed|\
 	--ignored|--exclude=*|\
 	--exclude-from=*|\--exclude-per-directory=*)
-		git_flags=("${git_flags[@]}" "$1")
+		git_flags="$git_flags '$1'"
 		;;
 	-e)
 		pattern="$2"
 		shift
 		;;
 	-A|-B|-C|-D|-d|-f|-m)
-		flags=("${flags[@]}" "$1" "$2")
+		flags="$flags '$1' '$2'"
 		shift
 		;;
 	--)
@@ -31,7 +31,7 @@ while : ; do
 		break
 		;;
 	-*)
-		flags=("${flags[@]}" "$1")
+		flags="$flags '$1'"
 		;;
 	*)
 		if [ -z "$pattern" ]; then
@@ -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"'

-- 
				Petr "Pasky" Baudis
Stuff: http://pasky.or.cz/
VI has two modes: the one in which it beeps and the one in which
it doesn't.

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

[Junio C Hamano]

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;
}

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

[Linus Torvalds]

On Sun, 18 Dec 2005, Timo Hirvonen wrote:
> 
> sh does not support arrays so we have to use eval instead.

This seems horribly broken.

If I'm not mistaken, this breaks

	git grep "it's a happy coincidence"

badly. I didn't test, just looking at the patch.

Dammit, I'd rather depend on "bash"/"ksh" than have a broken "git grep".

Tell people to get a real shell.

		Linus

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

[Timo Hirvonen]

On Sun, 18 Dec 2005 11:57:26 -0800 (PST)
Linus Torvalds <torvalds@osdl.org> wrote:

> On Sun, 18 Dec 2005, Timo Hirvonen wrote:
> > 
> > sh does not support arrays so we have to use eval instead.
> 
> This seems horribly broken.
> 
> If I'm not mistaken, this breaks
> 
> 	git grep "it's a happy coincidence"
> 
> badly. I didn't test, just looking at the patch.

Actually it works:

/usr/src/linux-2.6: git grep "it's a"
Documentation/Changes:debugfs.  Obviously, it's a good idea to upgrade.
Documentation/CodingStyle:and NOT read it.  Burn them, it's a great
symbolic gesture.


but it doesn't work with backslashes.  Need to use \\

/usr/src/linux-2.6: git grep 'e.\\n"'
Documentation/DMA-mapping.txt:		       "mydev: No suitable DMA
available.\n");
Documentation/DMA-mapping.txt:		       "mydev: No suitable DMA
available.\n");

-- 
http://onion.dynserv.net/~timo/