bug-gnu-emacs
[Top][All Lists]
Advanced

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

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


From: Kim F. Storm
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
User-agent: Gnus/5.11 (Gnus v5.11) Emacs/22.0.50 (gnu/linux)

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





reply via email to

[Prev in Thread] Current Thread [Next in Thread]