{"thread":{"id":"2868","subject":"[PATCH] git-grep: convert from bash to sh","startedAt":"2005-12-18T13:26:39Z","lastAt":"2005-12-18T20:18:00Z","messageCount":5,"participants":["Timo Hirvonen","Petr Baudis","Junio C Hamano","Linus Torvalds"],"isPatch":true,"patchVersion":1,"patchTotal":null},"messages":[{"id":"13782","messageId":"20051218152639.5c14bc26.tihirvon@gmail.com","threadId":"2868","inReplyTo":null,"subject":"[PATCH] git-grep: convert from bash to sh","fromName":"Timo Hirvonen","fromEmail":"tihirvon@gmail.com","sentAt":"2005-12-18T13:26:39Z","receivedAt":"2005-12-18T13:26:39Z","isPatch":true,"sender":{"key":"tihirvon@gmail.com","avatar":null},"body":"\nsh does not support arrays so we have to use eval instead.\n\nSigned-off-by: Timo Hirvonen <tihirvon@gmail.com>\n\n---\n\n git-grep.sh |   14 +++++++-------\n 1 files changed, 7 insertions(+), 7 deletions(-)\n\n11c29a066288c5f05a67ff0d46e9ce17cd7a37da\ndiff --git a/git-grep.sh b/git-grep.sh\nindex 2ed8e95..2f0a297 100755\n--- a/git-grep.sh\n+++ b/git-grep.sh\n@@ -8,21 +8,21 @@ SUBDIRECTORY_OK='Yes'\n . git-sh-setup\n \n pattern=\n-flags=()\n-git_flags=()\n+flags=\n+git_flags=\n while : ; do\n \tcase \"$1\" in\n \t--cached|--deleted|--others|--killed|\\\n \t--ignored|--exclude=*|\\\n \t--exclude-from=*|\\--exclude-per-directory=*)\n-\t\tgit_flags=(\"${git_flags[@]}\" \"$1\")\n+\t\tgit_flags=\"$git_flags '$1'\"\n \t\t;;\n \t-e)\n \t\tpattern=\"$2\"\n \t\tshift\n \t\t;;\n \t-A|-B|-C|-D|-d|-f|-m)\n-\t\tflags=(\"${flags[@]}\" \"$1\" \"$2\")\n+\t\tflags=\"$flags '$1' '$2'\"\n \t\tshift\n \t\t;;\n \t--)\n@@ -31,7 +31,7 @@ while : ; do\n \t\tbreak\n \t\t;;\n \t-*)\n-\t\tflags=(\"${flags[@]}\" \"$1\")\n+\t\tflags=\"$flags '$1'\"\n \t\t;;\n \t*)\n \t\tif [ -z \"$pattern\" ]; then\n@@ -46,5 +46,5 @@ done\n [ \"$pattern\" ] || {\n \tusage\n }\n-git-ls-files -z \"${git_flags[@]}\" \"$@\" |\n-\txargs -0 grep \"${flags[@]}\" -e \"$pattern\"\n+eval git-ls-files -z \"$git_flags\" '\"$@\"' |\n+\teval xargs -0 grep \"$flags\" -e '\"$pattern\"'\n-- \n0.99.9.GIT\n"},{"id":"13784","messageId":"20051218145621.GX22159@pasky.or.cz","threadId":"2868","inReplyTo":"20051218152639.5c14bc26.tihirvon@gmail.com","subject":"Re: [PATCH] git-grep: convert from bash to sh","fromName":"Petr Baudis","fromEmail":"pasky@suse.cz","sentAt":"2005-12-18T14:56:21Z","receivedAt":"2005-12-18T14:56:21Z","isPatch":true,"sender":{"key":"pasky@ucw.cz","avatar":"https://avatars.githubusercontent.com/u/18439?v=4"},"body":"Dear diary, on Sun, Dec 18, 2005 at 02:26:39PM CET, I got a letter\nwhere Timo Hirvonen <tihirvon@gmail.com> said that...\n> sh does not support arrays so we have to use eval instead.\n> \n> Signed-off-by: Timo Hirvonen <tihirvon@gmail.com>\n\nThis version also makes it work properly with patterns containing quotes\nand backslashes (not so unusual when you grep for C strings).\n\nSigned-off-by: Petr Baudis <pasky@suse.cz>\n\n---\n\nI'm kind of sensitive to this stuff. I still passionately hate scp\nmaking me to double-quote remote filenames. It's just evil.\n\ndiff --git a/git-grep.sh b/git-grep.sh\nindex 2ed8e95..7e9e5bf 100755\n--- a/git-grep.sh\n+++ b/git-grep.sh\n@@ -8,21 +8,21 @@ SUBDIRECTORY_OK='Yes'\n . git-sh-setup\n \n pattern=\n-flags=()\n-git_flags=()\n+flags=\n+git_flags=\n while : ; do\n \tcase \"$1\" in\n \t--cached|--deleted|--others|--killed|\\\n \t--ignored|--exclude=*|\\\n \t--exclude-from=*|\\--exclude-per-directory=*)\n-\t\tgit_flags=(\"${git_flags[@]}\" \"$1\")\n+\t\tgit_flags=\"$git_flags '$1'\"\n \t\t;;\n \t-e)\n \t\tpattern=\"$2\"\n \t\tshift\n \t\t;;\n \t-A|-B|-C|-D|-d|-f|-m)\n-\t\tflags=(\"${flags[@]}\" \"$1\" \"$2\")\n+\t\tflags=\"$flags '$1' '$2'\"\n \t\tshift\n \t\t;;\n \t--)\n@@ -31,7 +31,7 @@ while : ; do\n \t\tbreak\n \t\t;;\n \t-*)\n-\t\tflags=(\"${flags[@]}\" \"$1\")\n+\t\tflags=\"$flags '$1'\"\n \t\t;;\n \t*)\n \t\tif [ -z \"$pattern\" ]; then\n@@ -46,5 +46,6 @@ done\n [ \"$pattern\" ] || {\n \tusage\n }\n-git-ls-files -z \"${git_flags[@]}\" \"$@\" |\n-\txargs -0 grep \"${flags[@]}\" -e \"$pattern\"\n+pattern=\"$(echo \"$pattern\" | sed 's/[\\\\\"]/\\\\&/g')\"\n+eval git-ls-files -z \"$git_flags\" '\"$@\"' |\n+\teval xargs -0 grep \"$flags\" -e '\"$pattern\"'\n\n-- \n\t\t\t\tPetr \"Pasky\" Baudis\nStuff: http://pasky.or.cz/\nVI has two modes: the one in which it beeps and the one in which\nit doesn't.\n"},{"id":"13788","messageId":"7vr78amc19.fsf@assigned-by-dhcp.cox.net","threadId":"2868","inReplyTo":"20051218145621.GX22159@pasky.or.cz","subject":"Re: [PATCH] git-grep: convert from bash to sh","fromName":"Junio C Hamano","fromEmail":"junkio@cox.net","sentAt":"2005-12-18T19:37:54Z","receivedAt":"2005-12-18T19:37:54Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Petr Baudis <pasky@suse.cz> writes:\n\n> I'm kind of sensitive to this stuff. I still passionately hate scp\n> making me to double-quote remote filenames. It's just evil.\n\nOK.\n\n>  \t--ignored|--exclude=*|\\\n>  \t--exclude-from=*|\\--exclude-per-directory=*)\n> -\t\tgit_flags=(\"${git_flags[@]}\" \"$1\")\n> +\t\tgit_flags=\"$git_flags '$1'\"\n\nSQs in --exclude= or --exclude-from= parameter?  E.g.\n\n\tgit grep --exclude-from=\"/cygdrive/c/pasky's patterns/cvsignore\" \\\n\t\t-e foobar\n\n>  \t-A|-B|-C|-D|-d|-f|-m)\n> -\t\tflags=(\"${flags[@]}\" \"$1\" \"$2\")\n> +\t\tflags=\"$flags '$1' '$2'\"\n>  \t\tshift\n\nSQs in $2 for -f.  About other flags, -[ABCm] take only numbers\nand -[Dd] take read/skip/recurse, so as long as your user does\nnot screw up you are OK.  And malicious users are shooting\nthemselves in the foot here so we might not care too much being\nloose.\n\n> @@ -46,5 +46,6 @@ done\n>  [ \"$pattern\" ] || {\n>  \tusage\n>  }\n> -git-ls-files -z \"${git_flags[@]}\" \"$@\" |\n> -\txargs -0 grep \"${flags[@]}\" -e \"$pattern\"\n> +pattern=\"$(echo \"$pattern\" | sed 's/[\\\\\"]/\\\\&/g')\"\n> +eval git-ls-files -z \"$git_flags\" '\"$@\"' |\n> +\teval xargs -0 grep \"$flags\" -e '\"$pattern\"'\n\nYou are not expanding $pattern in the outer shell that builds\neval arguments (letting the inner shell expand \"$pattern\" and\nuse it), so I do not think you need that sed script to muck with\nit there.  The eval'ed string is literally \"$pattern\" including\ndouble quotes because you have sq around the last parameter to\n\"eval\" command, and eval would do the right thing, no?\n\nActually the use of sed script there is actively wrong.  When a\nfile contains these lines:\n\n        printf(\"%s is not there\\n\");\n        printf(\"\\\"%s\\\" is not there\\n\");\n\nthe command to pick up the second line should be:\n\n\t$ git-grep -e '\\\\\"%s\\\\\"'\n\nbut I think your version passes \\\\\\\\\\\"%s\\\\\\\\\\ to underlying\ngrep.  Removing that single line would make it do the right\nthing, I think.\n\n\nEnough nitpicking.  If you want to stay in shell and want to\navoid shell array, you would either need to be a bit more\ncareful if you are going to use eval, or do something like what\nI originally did, which made Linus' brain shut down and had him\ngouge his eyes with a spoon.  It is the one that this one is a\nresponse to:\n\n\thttp://marc.theaimsgroup.com/?l=git&m=112656882627760\n\nNot that I am suggesting the latter is better than a bit more\ncareful 'eval' construction ;-).\n\nI am not happy with that [@] thing either. I have the attached\nlaying around in my working tree --- I do not remember if I\nfinished it or not; it's a WIP when I thought about this issue\nlast time and wondered if we might be better off rewriting the\nwhole thing.\n\n-- >8 --\n#!/usr/bin/perl\n#\n# Copyright (c) 2005 Junio C Hamano\n#\n\nuse strict;\nmy (@git_flag, @grep_flag, $pattern);\nmy $nargs = 100;\n\nmy (%git_ls_files_flag) =\n    map { '--' . $_ => 1 }\n    qw{cached deleted others kiled ignored\n       exclude= exclude-from= exclude-per-directory=};\n\nsub is_git_flag {\n    my ($arg) = @_;\n    $arg =~ s/^([^=]*)=/$1/;\n    return (exists $git_ls_files_flag{$arg});\n}\n\nwhile (@ARGV) {\n    my ($arg) = shift @ARGV;\n    if (is_git_flag($arg)) {\n\tpush @git_flag, $arg;\n    }\n    elsif ($arg eq '-e') {\n\t$pattern = shift @ARGV;\n    }\n    elsif ($arg =~ /^-[ABCDdfm]$/) {\n\tpush @grep_flag, $arg, (shift @ARGV);\n    }\n    elsif ($arg =~ /^--$/) {\n\tshift @ARGV;\n\tlast;\n    }\n    elsif ($arg =~ /^-/) {\n\tpush @grep_flag, $arg;\n    }\n    elsif (! defined $pattern) {\n\t$pattern = $arg;\n\tlast;\n    }\n    else {\n\tlast;\n    }\n}\n\nmy $ih;\nopen $ih, '-|', qw(git-ls-files -z), @git_flag, @ARGV;\n$/ = \"\\0\";\nmy @args = ();\nwhile (<$ih>) {\n    chomp;\n    push @args, $_;\n    if ($nargs <= @args) {\n\tsystem 'grep', @grep_flag, '-e', $pattern, @args;\n\t@args = ();\n    }\n}\nclose $ih;\nif (@args) {\n    system 'grep', @grep_flag, '-e', $pattern, @args;\n}\n"},{"id":"13789","messageId":"Pine.LNX.4.64.0512181155360.4827@g5.osdl.org","threadId":"2868","inReplyTo":"20051218152639.5c14bc26.tihirvon@gmail.com","subject":"Re: [PATCH] git-grep: convert from bash to sh","fromName":"Linus Torvalds","fromEmail":"torvalds@osdl.org","sentAt":"2005-12-18T19:57:26Z","receivedAt":"2005-12-18T19:57:26Z","isPatch":true,"sender":{"key":"torvalds@linux-foundation.org","avatar":"https://avatars.githubusercontent.com/u/1024025?v=4"},"body":"\n\nOn Sun, 18 Dec 2005, Timo Hirvonen wrote:\n> \n> sh does not support arrays so we have to use eval instead.\n\nThis seems horribly broken.\n\nIf I'm not mistaken, this breaks\n\n\tgit grep \"it's a happy coincidence\"\n\nbadly. I didn't test, just looking at the patch.\n\nDammit, I'd rather depend on \"bash\"/\"ksh\" than have a broken \"git grep\".\n\nTell people to get a real shell.\n\n\t\tLinus\n"},{"id":"13791","messageId":"20051218221800.744e4565.tihirvon@gmail.com","threadId":"2868","inReplyTo":"Pine.LNX.4.64.0512181155360.4827@g5.osdl.org","subject":"Re: [PATCH] git-grep: convert from bash to sh","fromName":"Timo Hirvonen","fromEmail":"tihirvon@gmail.com","sentAt":"2005-12-18T20:18:00Z","receivedAt":"2005-12-18T20:18:00Z","isPatch":true,"sender":{"key":"tihirvon@gmail.com","avatar":null},"body":"On Sun, 18 Dec 2005 11:57:26 -0800 (PST)\nLinus Torvalds <torvalds@osdl.org> wrote:\n\n> On Sun, 18 Dec 2005, Timo Hirvonen wrote:\n> > \n> > sh does not support arrays so we have to use eval instead.\n> \n> This seems horribly broken.\n> \n> If I'm not mistaken, this breaks\n> \n> \tgit grep \"it's a happy coincidence\"\n> \n> badly. I didn't test, just looking at the patch.\n\nActually it works:\n\n/usr/src/linux-2.6: git grep \"it's a\"\nDocumentation/Changes:debugfs.  Obviously, it's a good idea to upgrade.\nDocumentation/CodingStyle:and NOT read it.  Burn them, it's a great\nsymbolic gesture.\n\n\nbut it doesn't work with backslashes.  Need to use \\\\\n\n/usr/src/linux-2.6: git grep 'e.\\\\n\"'\nDocumentation/DMA-mapping.txt:\t\t       \"mydev: No suitable DMA\navailable.\\n\");\nDocumentation/DMA-mapping.txt:\t\t       \"mydev: No suitable DMA\navailable.\\n\");\n\n-- \nhttp://onion.dynserv.net/~timo/\n"}]}