unofficial mirror of bug-gnu-emacs@gnu.org 
 help / color / mirror / code / Atom feed
* grep.el bugs: grep-find-use-xargs handling, non-use of shell-quote-argument, find-grep and alternative find
@ 2006-08-22 19:17 Tom Seddon
  2006-08-22 22:28 ` Kim F. Storm
  2006-08-23 23:25 ` Kim F. Storm
  0 siblings, 2 replies; 4+ messages in thread
From: Tom Seddon @ 2006-08-22 19:17 UTC (permalink / raw)


[-- Attachment #1: Type: text/plain, Size: 2358 bytes --]

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


[-- Attachment #2: grep.el.patch --]
[-- Type: text/plain, Size: 4567 bytes --]

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

[-- Attachment #3: Type: text/plain, Size: 149 bytes --]

_______________________________________________
bug-gnu-emacs mailing list
bug-gnu-emacs@gnu.org
http://lists.gnu.org/mailman/listinfo/bug-gnu-emacs

^ permalink raw reply	[flat|nested] 4+ messages in thread

end of thread, other threads:[~2006-08-24 17:53 UTC | newest]

Thread overview: 4+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2006-08-22 19:17 grep.el bugs: grep-find-use-xargs handling, non-use of shell-quote-argument, find-grep and alternative find Tom Seddon
2006-08-22 22:28 ` Kim F. Storm
2006-08-23 23:25 ` Kim F. Storm
2006-08-24 17:53   ` Tom Seddon

Code repositories for project(s) associated with this public inbox

	https://git.savannah.gnu.org/cgit/emacs.git

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).