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

[Tom Seddon]

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