Re: grep.el bugs: grep-find-use-xargs handling, non-use of shell-quote-argument, find-grep and alternative find

[no-spam@cua.dk (Kim F. Storm)]

I have installed my changes.
Please test to see if they fix the reported problems.

Thanks again.



Tom Seddon <bug-gnu-emacs@tomseddon.plus.com> 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 <C> %s <R> <F>" 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 <C> %s <R>"
> @@ -441,11 +466,12 @@
>  		(cond ((eq grep-find-use-xargs 'gnu)
>  		       (format "%s . <X> -type f <F> -print0 | xargs -0 -e %s"
>  			       find-program gcmd))
> -		      (grep-find-use-xargs
> +		      ((eq grep-find-use-xargs 'yes)
>  		       (format "%s . <X> -type f <F> -print | xargs %s"
>  			       find-program gcmd))
> -		      (t (format "%s . <X> -type f <F> -exec %s {} %s \\;"
> -				 find-program gcmd null-device))))))))
> +		      ((eq grep-find-use-xargs 'no)
> +		       (format "%s . <X> -type f <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