* 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
* Re: grep.el bugs: grep-find-use-xargs handling, non-use of shell-quote-argument, find-grep and alternative find
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
1 sibling, 0 replies; 4+ messages in thread
From: Kim F. Storm @ 2006-08-22 22:28 UTC (permalink / raw)
Cc: bug-gnu-emacs
Tom Seddon <bug-gnu-emacs@tomseddon.plus.com> writes:
> Hi,
>
> I've included a patch to fix three grep.el bugs.
Thank you for the bug report.
I will install fixes for these problems shortly.
>
> 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.
--
Kim F. Storm http://www.cua.dk
^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: grep.el bugs: grep-find-use-xargs handling, non-use of shell-quote-argument, find-grep and alternative find
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
1 sibling, 1 reply; 4+ messages in thread
From: Kim F. Storm @ 2006-08-23 23:25 UTC (permalink / raw)
Cc: bug-gnu-emacs
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
^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: grep.el bugs: grep-find-use-xargs handling, non-use of shell-quote-argument, find-grep and alternative find
2006-08-23 23:25 ` Kim F. Storm
@ 2006-08-24 17:53 ` Tom Seddon
0 siblings, 0 replies; 4+ messages in thread
From: Tom Seddon @ 2006-08-24 17:53 UTC (permalink / raw)
Yes, that seems to have done it. Thanks!
--Tom
Kim F. Storm wrote:
> 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.
^ 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).