From mboxrd@z Thu Jan 1 00:00:00 1970 Path: news.gmane.org!not-for-mail From: no-spam@cua.dk (Kim F. Storm) Newsgroups: gmane.emacs.bugs Subject: Re: grep.el bugs: grep-find-use-xargs handling, non-use of shell-quote-argument, find-grep and alternative find Date: Thu, 24 Aug 2006 01:25:44 +0200 Message-ID: References: <44EB5844.1020509@tomseddon.plus.com> NNTP-Posting-Host: main.gmane.org Mime-Version: 1.0 Content-Type: text/plain; charset=us-ascii X-Trace: sea.gmane.org 1156393207 22662 80.91.229.2 (24 Aug 2006 04:20:07 GMT) X-Complaints-To: usenet@sea.gmane.org NNTP-Posting-Date: Thu, 24 Aug 2006 04:20:07 +0000 (UTC) Cc: bug-gnu-emacs@gnu.org Original-X-From: bug-gnu-emacs-bounces+geb-bug-gnu-emacs=m.gmane.org@gnu.org Thu Aug 24 06:19:57 2006 Return-path: Envelope-to: geb-bug-gnu-emacs@m.gmane.org Original-Received: from lists.gnu.org ([199.232.76.165]) by ciao.gmane.org with esmtp (Exim 4.43) id 1GG6gG-0007Rd-2S for geb-bug-gnu-emacs@m.gmane.org; Thu, 24 Aug 2006 06:19:01 +0200 Original-Received: from localhost ([127.0.0.1] helo=lists.gnu.org) by lists.gnu.org with esmtp (Exim 4.43) id 1GG6gF-0002kX-5M for geb-bug-gnu-emacs@m.gmane.org; Thu, 24 Aug 2006 00:18:59 -0400 Original-Received: from mailman by lists.gnu.org with tmda-scanned (Exim 4.43) id 1GG27i-0000Vx-JN for bug-gnu-emacs@gnu.org; Wed, 23 Aug 2006 19:27:02 -0400 Original-Received: from exim by lists.gnu.org with spam-scanned (Exim 4.43) id 1GG27g-0000V1-Py for bug-gnu-emacs@gnu.org; Wed, 23 Aug 2006 19:27:02 -0400 Original-Received: from [199.232.76.173] (helo=monty-python.gnu.org) by lists.gnu.org with esmtp (Exim 4.43) id 1GG27g-0000Uy-Gw for bug-gnu-emacs@gnu.org; Wed, 23 Aug 2006 19:27:00 -0400 Original-Received: from [195.41.46.235] (helo=pfepa.post.tele.dk) by monty-python.gnu.org with esmtp (Exim 4.52) id 1GG2Fc-0005DR-6v for bug-gnu-emacs@gnu.org; Wed, 23 Aug 2006 19:35:12 -0400 Original-Received: from kfs-l.imdomain.dk.cua.dk (0x503e2644.bynxx3.adsl-dhcp.tele.dk [80.62.38.68]) by pfepa.post.tele.dk (Postfix) with SMTP id 6DFADFAC00C; Thu, 24 Aug 2006 01:26:58 +0200 (CEST) Original-To: Tom Seddon In-Reply-To: <44EB5844.1020509@tomseddon.plus.com> (Tom Seddon's message of "Tue, 22 Aug 2006 20:17:24 +0100") User-Agent: Gnus/5.11 (Gnus v5.11) Emacs/22.0.50 (gnu/linux) X-Mailman-Approved-At: Thu, 24 Aug 2006 00:18:32 -0400 X-BeenThere: bug-gnu-emacs@gnu.org X-Mailman-Version: 2.1.5 Precedence: list List-Id: "Bug reports for GNU Emacs, the Swiss army knife of text editors" List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Original-Sender: bug-gnu-emacs-bounces+geb-bug-gnu-emacs=m.gmane.org@gnu.org Errors-To: bug-gnu-emacs-bounces+geb-bug-gnu-emacs=m.gmane.org@gnu.org Xref: news.gmane.org gmane.emacs.bugs:15273 Archived-At: I have installed my changes. Please test to see if they fix the reported problems. Thanks again. Tom Seddon writes: > Hi, > > I've included a patch to fix three grep.el bugs. > > I'm using patched EmacsW32-1.06 (Emacs-22-CvsP060818-EmacsW32-1.06.exe) > but grep.el looks to be the same as the latest one in CVS. OS is Windows > XP, default shell is 4NT, and I'm using the GnuWin32 versions of find, > grep and xargs. > > Firstly, grep-find-use-xargs handling is slightly wrong. The docs state > that if grep-find-use-xargs is nil, xargs won't be used. But that isn't > quite true -- if grep-find-use-xargs is nil, grep-compute-defaults tries > to work out whether grep-find-use-xargs should actually be 'gnu. > > On my PC at least, 'gnu doesn't work (xargs gives the error "grep: > Invalid argument"). I didn't look into this bit, because it seemed like > you could make grep.el use find -exec instead. But that wasn't the case > -- since I actually have a GNU xargs.exe available in the PATH it was > impossible to make grep-compute-defaults set things up to use find -exec > rather than xargs. > > Judging by the docstring for grep-find-use-xargs, this was wrong. The > new behaviour is described in the new docstring. It should be > backwards-compatible for all sensible uses. In particular, if > grep-find-use-xargs is nil, the existing autodetection is still performed. > > (Perhaps I should just have figured out why xargs doesn't work...) > > The second bug is that grep.el doesn't use shell-quote-argument when > constructing the args for find. It just use "\\(", "\\)" and "\\;" > literally. This was causing errors on my PC: > > unixfind: paths must precede expression > Usage: unixfind [-H] [-L] [-P] [path...] [expression] > > Changing grep.el to use shell-quote-argument where appropriate fixed this. > > Finally, grep.el mishandles the case where find-program is something > other than "find". (Since Windows XP includes a "find.exe", I suppose > others might be changing find-program to point at an > alternatively-named version of GNU find.) See also: > > http://lists.gnu.org/archive/html/bug-gnu-emacs/2003-11/msg00029.html > > I've changed grep-compute-defaults accordingly, so that M-x find-grep > puts the cursor at the right point. > > > If Thunderbird has done the right thing, you should see the patch as > text below. It is also available from my web page: > > http://www.tomseddon.plus.com/emacs/grep/grep.el.patch > > A pre-patched grep.el is also available there: > > http://www.tomseddon.plus.com/emacs/grep/grep.el > > --Tom > > --- grep.el.orig 2006-08-22 18:24:34.734375000 +0100 > +++ grep.el 2006-08-22 19:55:58.703125000 +0100 > @@ -335,10 +335,18 @@ > (defvar grep-find-use-xargs nil > "Whether \\[grep-find] uses the `xargs' utility by default. > > -If nil, it uses `find -exec'; if `gnu', it uses `find -print0' and `xargs -0'; > -if not nil and not `gnu', it uses `find -print' and `xargs'. > +If `no', it uses `find -exec'; if `gnu', it uses `find -print0' > +and `xargs -0'; if `yes', it uses `find -print' and `xargs'. > > -This variable's value takes effect when `grep-compute-defaults' is called.") > +This variable's value takes effect when `grep-compute-defaults' > +is called. > + > +\(For backwards compatibility, if `grep-find-use-xargs' is nil, > +`grep-compute-defaults' will set it to one of `gnu' or `no'; or, > +if `grep-find-use-xargs' is non-nil, but not one of the valid > +settings, `grep-compute-defaults' will set it to `yes'.\) > + > +") > > ;; History of grep commands. > ;;;###autoload > @@ -382,6 +390,7 @@ > (error nil)) > (or result 0))) > > + > ;;;###autoload > (defun grep-compute-defaults () > (unless (or (not grep-use-null-device) (eq grep-use-null-device t)) > @@ -417,23 +426,39 @@ > (unless grep-template > (setq grep-template > (format "%s %s " grep-program grep-options))) > - (unless grep-find-use-xargs > - (setq grep-find-use-xargs > - (if (and > - (grep-probe find-program `(nil nil nil ,null-device "-print0")) > - (grep-probe "xargs" `(nil nil nil "-0" "-e" "echo"))) > - 'gnu))) > + ;; "Normalize" `grep-find-use-xargs'. Hardly ideal, but > + ;; preserves the previous behaviour (unless 'no was being as the > + ;; non-null value of course). > + ;; > + ;; Presumably keeping nil to mean 'auto-detect' is desirable? > + (setq grep-find-use-xargs > + (cond ((null grep-find-use-xargs) > + (if (and > + (grep-probe find-program `(nil nil nil ,null-device "-print0")) > + (grep-probe "xargs" `(nil nil nil "-0" "-e" "echo"))) > + 'gnu > + 'no)) > + ((not (or (eq grep-find-use-xargs 'gnu) > + (eq grep-find-use-xargs 'no))) > + 'yes) > + (t > + 'no))) > (unless grep-find-command > (setq grep-find-command > (cond ((eq grep-find-use-xargs 'gnu) > (format "%s . -type f -print0 | xargs -0 -e %s" > find-program grep-command)) > - (grep-find-use-xargs > + ((eq grep-find-use-xargs 'yes) > (format "%s . -type f -print | xargs %s" > find-program grep-command)) > - (t (cons (format "%s . -type f -exec %s {} %s \\;" > - find-program grep-command null-device) > - (+ 22 (length grep-command))))))) > + ((eq grep-find-use-xargs 'no) > + (let ((command (format "%s . -type f -exec %s {} %s %s" > + find-program grep-command null-device (shell-quote-argument ";")))) > + (cons command > + (+ (string-match (regexp-quote grep-command) > + command) > + (length grep-command) > + 1))))))) > (unless grep-find-template > (setq grep-find-template > (let ((gcmd (format "%s %s " > @@ -441,11 +466,12 @@ > (cond ((eq grep-find-use-xargs 'gnu) > (format "%s . -type f -print0 | xargs -0 -e %s" > find-program gcmd)) > - (grep-find-use-xargs > + ((eq grep-find-use-xargs 'yes) > (format "%s . -type f -print | xargs %s" > find-program gcmd)) > - (t (format "%s . -type f -exec %s {} %s \\;" > - find-program gcmd null-device)))))))) > + ((eq grep-find-use-xargs 'no) > + (format "%s . -type f -exec %s {} %s %s" > + find-program gcmd null-device (shell-quote-argument ";"))))))))) > (unless (or (not grep-highlight-matches) (eq grep-highlight-matches t)) > (setq grep-highlight-matches > (with-temp-buffer > @@ -736,18 +762,23 @@ > (let ((command (grep-expand-template > grep-find-template > regexp > - (concat "\\( -name " > + (concat (shell-quote-argument "(") > + " -name " > (mapconcat #'shell-quote-argument > (split-string files) > " -o -name ") > - " \\)") > + " " > + (shell-quote-argument ")")) > dir > (and grep-find-ignored-directories > - (concat "\\( -path '*/" > + (concat (shell-quote-argument "(") > + " -path '*/" > (mapconcat #'identity > grep-find-ignored-directories > "' -o -path '*/") > - "' \\) -prune -o "))))) > + "' " > + (shell-quote-argument ")") > + " -prune -o "))))); > (when command > (if current-prefix-arg > (setq command > _______________________________________________ > bug-gnu-emacs mailing list > bug-gnu-emacs@gnu.org > http://lists.gnu.org/mailman/listinfo/bug-gnu-emacs -- Kim F. Storm http://www.cua.dk