Re: address@hidden: grep-tree doesn't shell-quote-argument]

[Kim F. Storm]

address@hidden (Kim F. Storm) writes:

> Richard Stallman <address@hidden> writes:
>
>>     > For now, I would suggest we consider making the new feature
>>     > grep-tree more convenient in a way that isn't incompatible for old
>>     > features such as grep-find.
>>
>>     The problem that was being addressed is that we already have to many
>>     similar features.  The patch was intended to obliterate "grep-tree" as
>>     a separate feature.
>>
>> Yes, it could be C-u M-x grep-find.
>
> How is that convenient?
>
> Here is a patch which make the improvement for both M-x grep and M-x 
> grep-find.

I worked some more on cleaning this up, consolidate some duplicate code, 
and removed the grep-ignore-case defcustom (and just obey case-fold-search,
unless there are upper-case letters in the regexp [like isearch].)

I still have a very strong belief that we MUST fix this issue before the 
release.

Note that major parts of this patch still need to be applied to fix problems
and short-comings in the current grep-tree implementation, and more of it must
be applied to integrate grep-tree into grep-find [in some form or another].

So I still think it would be better to make the whole [incompatible,
but much improved user interface] change now, so we don't end up in a
significant mess trying to revert/fix even more incompatibilities after
the release.

*** grep.el     16 Mar 2006 09:41:13 +0100      1.51
--- grep.el     25 Apr 2006 00:30:13 +0200      
***************
*** 108,113 ****
--- 108,126 ----
                 (const :tag "Not Set" nil))
    :group 'grep)
  
+ (defcustom grep-command-template nil
+   "The default find command for \\[grep-find].
+ The default value of this variable is set up by `grep-compute-defaults';
+ call that function before using this variable in your program.
+ The following place holders should be present in the string:
+  <C> - place to put -i if case insensitive grep.
+  <F> - file names and wildcards to search.
+  <R> - the regular expression searched for."
+   :type '(choice string
+                (const :tag "Not Set" nil))
+   :version "22.1"
+   :group 'grep)
+ 
  (defcustom grep-use-null-device 'auto-detect
    "If t, append the value of `null-device' to `grep' commands.
  This is done to ensure that the output of grep includes the filename of
***************
*** 130,137 ****
                 (const :tag "Not Set" nil))
    :group 'grep)
  
! (defcustom grep-tree-command nil
!   "The default find command for \\[grep-tree].
  The default value of this variable is set up by `grep-compute-defaults';
  call that function before using this variable in your program.
  The following place holders should be present in the string:
--- 143,150 ----
                 (const :tag "Not Set" nil))
    :group 'grep)
  
! (defcustom grep-find-command-template nil
!   "The default find command for \\[grep-find].
  The default value of this variable is set up by `grep-compute-defaults';
  call that function before using this variable in your program.
  The following place holders should be present in the string:
***************
*** 145,171 ****
    :version "22.1"
    :group 'grep)
  
! (defcustom grep-tree-files-aliases '(
        ("ch" . "*.[ch]")
        ("c" .  "*.c")
        ("h" .  "*.h")
-       ("m" .  "[Mm]akefile*")
        ("asm" . "*.[sS]")
!       ("all" . "*")
!       ("el" . "*.el")
        )
!   "*Alist of aliases for the FILES argument to `grep-tree'."
    :type 'alist
    :group 'grep)
  
! (defcustom grep-tree-ignore-case t
!   "*If non-nil, `grep-tree' ignores case in matches."
!   :type 'boolean
!   :group 'grep)
! 
! (defcustom grep-tree-ignore-CVS-directories t
!   "*If non-nil, `grep-tree' does no recurse into CVS directories."
!   :type 'boolean
    :group 'grep)
  
  (defcustom grep-error-screen-columns nil
--- 158,189 ----
    :version "22.1"
    :group 'grep)
  
! (defcustom grep-prompt-style nil
!   "*Prompt style used by `grep' and `grep-find'.
! Nil means to prompt for regexp, files, and directory.
! Value `shell' means to prompt for shell command instead.
! Value `post' means to post-edit the final shell command."
!   :type '(choice (const :tag "Standard" nil)
!                (const :tag "Shell Command" shell)
!                (const :tag "Post-edit Command" post))
!   :version "22.1"
!   :group 'grep)
! 
! (defcustom grep-files-aliases '(
!       ("el" . "*.el")
        ("ch" . "*.[ch]")
        ("c" .  "*.c")
        ("h" .  "*.h")
        ("asm" . "*.[sS]")
!       ("m" .  "[Mm]akefile*")
        )
!   "*Alist of aliases for the FILES argument to `grep' and `grep-find'."
    :type 'alist
    :group 'grep)
  
! (defcustom grep-find-ignored-directories '("CVS" ".hg" "{arch}")
!   "*List of names of sub-directories which `grep-find' shall not recurse 
into."
!   :type '(repeat string)
    :group 'grep)
  
  (defcustom grep-error-screen-columns nil
***************
*** 208,213 ****
--- 226,233 ----
        '("Compile..." . compile))
      (define-key map [menu-bar grep compilation-grep]
        '("Another grep..." . grep))
+     (define-key map [menu-bar grep compilation-grep-find]
+       '("Recursive grep..." . grep-find))
      (define-key map [menu-bar grep compilation-recompile]
        '("Repeat grep" . recompile))
      (define-key map [menu-bar grep compilation-separator2]
***************
*** 353,358 ****
--- 373,383 ----
  ;;;###autoload
  (defvar grep-find-history nil)
  
+ ;; History of grep and grep-find regexp and files args.
+ (defvar grep-regexp-history nil)
+ (defvar grep-files-history '("ch" "el"))
+ 
+ 
  ;;;###autoload
  (defun grep-process-setup ()
    "Setup compilation variables and buffer for `grep'.
***************
*** 402,461 ****
                      (looking-at
                       (concat (regexp-quote hello-file)
                               ":[0-9]+:English")))))))))
!   (unless grep-command
!     (setq grep-command
!         (let ((required-options (if grep-use-null-device "-n" "-nH")))
!           (if (equal (condition-case nil ; in case "grep" isn't in exec-path
!                          (call-process grep-program nil nil nil
!                                        "-e" "foo" null-device)
!                        (error nil))
!                      1)
!               (format "%s %s -e " grep-program required-options)
!             (format "%s %s " grep-program required-options)))))
!   (unless grep-find-use-xargs
!     (setq grep-find-use-xargs
!         (if (and
!                (equal (call-process "find" nil nil nil
!                                     null-device "-print0")
!                       0)
!                (equal (call-process "xargs" nil nil nil
!                                     "-0" "-e" "echo")
!                     0))
!             'gnu)))
!   (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
!                (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)))))))
!   (unless grep-tree-command
!     (setq grep-tree-command
!         (let* ((glen (length grep-program))
!                (gcmd (concat grep-program " <C>" (substring grep-command 
glen))))
!           (cond ((eq grep-find-use-xargs 'gnu)
!                  (format "%s <D> <X> -type f <F> -print0 | xargs -0 -e %s <R>"
!                          find-program gcmd))
!                 (grep-find-use-xargs
!                  (format "%s <D> <X> -type f <F> -print | xargs %s <R>"
!                          find-program gcmd))
!                 (t (format "%s <D> <X> -type f <F> -exec %s <R> {} %s \\;"
!                            find-program gcmd null-device))))))
!   (unless (or (not grep-highlight-matches) (eq grep-highlight-matches t))
!     (setq grep-highlight-matches
!         (with-temp-buffer
!           (and (equal (condition-case nil
!                           (call-process grep-program nil t nil "--help")
!                         (error nil))
!                       0)
!                (progn
!                  (goto-char (point-min))
!                  (search-forward "--color" nil t))
!                t)))))
  
  (defun grep-default-command ()
    (let ((tag-default
--- 427,491 ----
                      (looking-at
                       (concat (regexp-quote hello-file)
                               ":[0-9]+:English")))))))))
!   (unless (and grep-command grep-command-template
!              grep-find-command grep-find-command-template)
!     (let ((required-options
!          (concat (if grep-use-null-device "-n" "-nH")
!                  (if (equal (condition-case nil ; in case "grep" isn't in 
exec-path
!                                 (call-process grep-program nil nil nil
!                                               "-e" "foo" null-device)
!                               (error nil))
!                             1) " -e" ""))))
!       (unless grep-command
!       (setq grep-command
!             (format "%s %s " grep-program required-options)))
!       (unless grep-command-template
!       (setq grep-command-template
!             (format "%s <C> %s <R> <F>" grep-program required-options)))
!       (unless grep-find-use-xargs
!       (setq grep-find-use-xargs
!             (if (and
!                  (equal (call-process "find" nil nil nil
!                                       null-device "-print0")
!                         0)
!                  (equal (call-process "xargs" nil nil nil
!                                       "-0" "-e" "echo")
!                         0))
!                 'gnu)))
!       (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
!                    (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)))))))
!       (unless grep-find-command-template
!       (setq grep-find-command-template
!             (let ((gcmd (format "%s <C> %s <R>"
!                                 grep-program required-options)))
!               (cond ((eq grep-find-use-xargs 'gnu)
!                      (format "%s <D> <X> -type f <F> -print0 | xargs -0 -e %s"
!                              find-program gcmd))
!                     (grep-find-use-xargs
!                      (format "%s <D> <X> -type f <F> -print | xargs %s"
!                              find-program gcmd))
!                     (t (format "%s <D> <X> -type f <F> -exec %s {} %s \\;"
!                                find-program gcmd null-device))))))
!       (unless (or (not grep-highlight-matches) (eq grep-highlight-matches t))
!       (setq grep-highlight-matches
!             (with-temp-buffer
!               (and (equal (condition-case nil
!                               (call-process grep-program nil t nil "--help")
!                             (error nil))
!                           0)
!                    (progn
!                      (goto-char (point-min))
!                      (search-forward "--color" nil t))
!                    t)))))))
  
  (defun grep-default-command ()
    (let ((tag-default
***************
*** 487,528 ****
                                   (file-name-extension buffer-file-name))))
        (replace-match tag-default t t grep-default 1))))
  
  ;;;###autoload
! (defun grep (command-args &optional highlight-regexp)
!   "Run grep, with user-specified args, and collect output in a buffer.
  While grep runs asynchronously, you can use \\[next-error] (M-x next-error),
  or \\<grep-mode-map>\\[compile-goto-error] in the grep \
  output buffer, to go to the lines
  where grep found matches.
  
! This command uses a special history list for its COMMAND-ARGS, so you can
! easily repeat a grep command.
! 
! A prefix argument says to default the argument based upon the current
! tag the cursor is over, substituting it into the last grep command
! in the grep command history (or into `grep-command'
! if that history list is empty).
! 
! If specified, optional second arg HIGHLIGHT-REGEXP is the regexp to
! temporarily highlight in visited source lines."
    (interactive
     (progn
!      (unless (and grep-command
!                 (or (not grep-use-null-device) (eq grep-use-null-device t)))
!        (grep-compute-defaults))
!      (let ((default (grep-default-command)))
!        (list (read-from-minibuffer "Run grep (like this): "
!                                  (if current-prefix-arg
!                                      default grep-command)
!                                  nil nil 'grep-history
!                                  (if current-prefix-arg nil default))))))
! 
!   ;; Setting process-setup-function makes exit-message-function work
!   ;; even when async processes aren't supported.
!   (compilation-start (if (and grep-use-null-device null-device)
!                        (concat command-args " " null-device)
!                      command-args)
!                    'grep-mode nil highlight-regexp))
  
  ;;;###autoload
  (define-compilation-mode grep-mode "Grep"
--- 517,659 ----
                                   (file-name-extension buffer-file-name))))
        (replace-match tag-default t t grep-default 1))))
  
+ (defun grep-expand-template (template &optional regexp files dir excl)
+   "Patch grep COMMAND string replacing <C>, <D>, <F>, <R>, and <X>."
+   (let ((command template)
+       (case-fold case-fold-search)
+       (case-fold-search nil))
+     (if (string-match "<C>" command)
+       (setq command
+             (replace-match (if (and case-fold
+                                     (isearch-no-upper-case-p regexp t))
+                                "-i" "")
+                            t t command)))
+     (if (string-match "<D>" command)
+       (setq command
+             (replace-match (or dir ".")
+                            t t command)))
+     (if (string-match "<F>" command)
+       (setq command
+             (replace-match (or files "")
+                            t t command)))
+     (if (string-match "<N>" command)
+       (setq command
+             (replace-match (if (and grep-use-null-device null-device)
+                                null-device "")
+                            t t command)))
+     (if (string-match "<R>" command)
+       (setq command
+             (replace-match (shell-quote-argument (or regexp ""))
+                            t t command)))
+     (if (string-match "<X>" command)
+       (setq command
+         (replace-match (or excl "")
+                        t t command)))
+     command))
+ 
+ (defun grep-read-regexp ()
+   "Read regexp arg for interactive grep."
+   (let ((default
+         (or (funcall (or find-tag-default-function
+                          (get major-mode 'find-tag-default-function)
+                          'find-tag-default))
+             "")))
+     (read-string
+      (concat "Search for"
+            (if (and default (> (length default) 0))
+                (format " (default %s): " default) ": "))
+      nil 'grep-regexp-history default)))
+ 
+ (defun grep-read-files (regexp)
+   "Read files arg for interactive grep."
+   (let* ((default
+          (or (and (stringp (buffer-file-name))
+                   (let ((fn (file-name-nondirectory (buffer-file-name)))
+                         (aliases grep-files-aliases)
+                         alias)
+                     (while aliases
+                       (setq alias (car aliases)
+                             aliases (cdr aliases))
+                       (if (string-match (wildcard-to-regexp (cdr alias)) fn)
+                           (setq aliases nil)
+                         (setq alias nil)))
+                     (cdr alias)))
+              (car grep-files-history)))
+        (files (read-string
+                (concat "Search for \"" regexp
+                        "\" in files (default " default "): ")
+                nil 'grep-files-history default)))
+     (and files
+        (or (cdr (assoc files grep-files-aliases))
+            files))))
+ 
  ;;;###autoload
! (defun grep (regexp &optional files api)
!   "Run grep, searching for REGEXP in FILES in current directory.
! Collect output in a buffer.
! Interactively, prompt separately for each search parameter.
! The search is limited to file names matching shell pattern FILES.
! FILES may use abbreviations defined in `grep-files-aliases', e.g.
! entering `ch' is equivalent to `*.[ch]'.
! 
! With \\[universal-argument] prefix, prompt for shell command instead.
! With two \\[universal-argument] prefixes, prompt for search parameters,
! and allow user to edit the final shell command before it is submitted.
! Note that setting `grep-prompt-style' overrides any prefix arg.
! 
  While grep runs asynchronously, you can use \\[next-error] (M-x next-error),
  or \\<grep-mode-map>\\[compile-goto-error] in the grep \
  output buffer, to go to the lines
  where grep found matches.
  
! This command uses a special history list for its arguments, so you can
! easily repeat a grep command."
    (interactive
     (progn
!      (grep-compute-defaults)
!      (cond
!       ((or (eq grep-prompt-style 'shell)
!          (equal current-prefix-arg '(4)))
!        (if grep-command
!          (list (read-from-minibuffer "Run grep (like this): "
!                                      grep-command nil nil
!                                      'grep-history)
!                nil current-prefix-arg)
!        ;; No default was set
!        (read-string
!         "grep.el: No `grep-command' available. Press RET.")
!        (list nil nil nil)))
!       ((not grep-command-template)
!        (read-string
!       "grep.el: No `grep-command-template' available. Press RET.")
!        (list nil nil nil))
!       (t
!        (let* ((regexp (grep-read-regexp))
!             (files (grep-read-files regexp)))
!        (list regexp files current-prefix-arg))))))
!   (when regexp
!     (let (command)
!       (if (or (null files)  ;; backwards compatible non-interactive call
!             (eq grep-prompt-style 'shell)
!             (equal current-prefix-arg '(4)))
!         (setq command regexp)
!       (setq command (grep-expand-template
!                      grep-command-template
!                      regexp
!                      files))
!       (when command
!         (if (or (eq grep-prompt-style 'post)
!                 (equal current-prefix-arg '(16)))
!             (setq command
!                   (read-from-minibuffer "Confirm: "
!                                         command nil nil 'grep-find-history))
!           (push command grep-history))))
!       (when command
!       ;; Setting process-setup-function makes exit-message-function work
!       ;; even when async processes aren't supported.
!       (compilation-start (if (and grep-use-null-device null-device)
!                              (concat command " " null-device)
!                            command) 'grep-mode)))))
  
  ;;;###autoload
  (define-compilation-mode grep-mode "Grep"
***************
*** 537,649 ****
    (set (make-local-variable 'compilation-disable-input) t))
  
  ;;;###autoload
! (defun grep-find (command-args)
!   "Run grep via find, with user-specified args COMMAND-ARGS.
! Collect output in a buffer.
! While find runs asynchronously, you can use the \\[next-error] command
! to find the text that grep hits refer to.
! 
! This command uses a special history list for its arguments, so you can
! easily repeat a find command."
!   (interactive
!    (progn
!      (unless (and grep-command
!                 (or (not grep-use-null-device) (eq grep-use-null-device t)))
!        (grep-compute-defaults))
!      (if grep-find-command
!        (list (read-from-minibuffer "Run find (like this): "
!                                    grep-find-command nil nil
!                                      'grep-find-history))
!        ;; No default was set
!        (read-string
!         "compile.el: No `grep-find-command' command available. Press RET.")
!        (list nil))))
!   (when (and grep-find-command command-args)
!     (let ((null-device nil))          ; see grep
!       (grep command-args))))
! 
! ;;;###autoload
! (defalias 'find-grep 'grep-find)
! 
! (defun grep-expand-command-macros (command &optional regexp files dir excl 
case-fold)
!   "Patch grep COMMAND replacing <D>, etc."
!   (setq command
!       (replace-regexp-in-string "<D>"
!                                 (or dir ".") command t t))
!   (setq command
!       (replace-regexp-in-string "<X>"
!                                 (or excl "") command t t))
!   (setq command
!       (replace-regexp-in-string "<F>"
!                                 (or files "") command t t))
!   (setq command
!       (replace-regexp-in-string "<C>"
!                                 (if case-fold "-i" "") command t t))
!   (setq command
!       (replace-regexp-in-string "<R>"
!                                 (or regexp "") command t t))
!   command)
! 
! (defvar grep-tree-last-regexp "")
! (defvar grep-tree-last-files (car (car grep-tree-files-aliases)))
! 
! ;;;###autoload
! (defun grep-tree (regexp files dir &optional subdirs)
!   "Grep for REGEXP in FILES in directory tree rooted at DIR.
  Collect output in a buffer.
  Interactively, prompt separately for each search parameter.
- With prefix arg, reuse previous REGEXP.
  The search is limited to file names matching shell pattern FILES.
! FILES may use abbreviations defined in `grep-tree-files-aliases', e.g.
  entering `ch' is equivalent to `*.[ch]'.
  
  While find runs asynchronously, you can use the \\[next-error] command
  to find the text that grep hits refer to.
  
  This command uses a special history list for its arguments, so you can
! easily repeat a find command.
! 
! When used non-interactively, optional arg SUBDIRS limits the search to
! those sub directories of DIR."
    (interactive
!    (let* ((regexp
!          (if current-prefix-arg
!              grep-tree-last-regexp
!            (let* ((default (current-word))
!                   (spec (read-string
!                          (concat "Search for"
!                                  (if (and default (> (length default) 0))
!                                      (format " (default %s): " default) ": 
")))))
!              (if (equal spec "") default spec))))
!         (files
!          (read-string (concat "Search for \"" regexp "\" in files (default "  
 grep-tree-last-files  "): ")))
!         (dir
!          (read-directory-name "Base directory: " nil default-directory t)))
!      (list regexp files dir)))
!   (unless grep-tree-command
!     (grep-compute-defaults))
!   (unless (and (stringp files) (> (length files) 0))
!     (setq files grep-tree-last-files))
!   (when files
!     (setq grep-tree-last-files files)
!     (let ((mf (assoc files grep-tree-files-aliases)))
!       (if mf
!         (setq files (cdr mf)))))
!   (let ((command-args (grep-expand-command-macros
!                      grep-tree-command
!                      (setq grep-tree-last-regexp regexp)
!                      (and files (concat "-name '" files "'"))
!                      (if subdirs
!                          (if (stringp subdirs)
!                              subdirs
!                            (mapconcat 'identity subdirs " "))
!                        nil)  ;; we change default-directory to dir
!                      (and grep-tree-ignore-CVS-directories "-path '*/CVS' 
-prune -o ")
!                      grep-tree-ignore-case))
!       (default-directory (file-name-as-directory (expand-file-name dir)))
!       (null-device nil))              ; see grep
!     (grep command-args regexp)))
  
  
  (provide 'grep)
  
--- 668,747 ----
    (set (make-local-variable 'compilation-disable-input) t))
  
  ;;;###autoload
! (defun grep-find (regexp &optional files dir api)
!   "Recusively grep for REGEXP in FILES in directory tree rooted at DIR.
  Collect output in a buffer.
  Interactively, prompt separately for each search parameter.
  The search is limited to file names matching shell pattern FILES.
! FILES may use abbreviations defined in `grep-files-aliases', e.g.
  entering `ch' is equivalent to `*.[ch]'.
  
+ With \\[universal-argument] prefix, prompt for shell command instead.
+ With two \\[universal-argument] prefixes, prompt for search parameters,
+ and allow user to edit the final shell command before it is submitted.
+ Note that setting `grep-prompt-style' overrides any prefix arg.
+ 
  While find runs asynchronously, you can use the \\[next-error] command
  to find the text that grep hits refer to.
  
  This command uses a special history list for its arguments, so you can
! easily modify or repeat a find command."
    (interactive
!    (progn
!      (grep-compute-defaults)
!      (cond
!       ((or (eq grep-prompt-style 'shell)
!          (equal current-prefix-arg '(4)))
!        (if grep-find-command
!          (list (read-from-minibuffer "Run find (like this): "
!                                      grep-find-command nil nil
!                                      'grep-find-history)
!                nil nil current-prefix-arg)
! 
!        ;; No default was set
!        (read-string
!         "grep.el: No `grep-find-command' available. Press RET.")
!        (list nil nil nil nil)))
!       ((not grep-find-command-template)
!        (read-string
!       "grep.el: No `grep-find-command-template' available. Press RET.")
!        (list nil nil nil nil))
!       (t
!        (let* ((regexp (grep-read-regexp))
!             (files (grep-read-files regexp))
!             (dir (read-directory-name "Base directory: "
!                                       nil default-directory t)))
!        (list regexp files dir current-prefix-arg))))))
!   (when regexp
!     (if (or (null files)  ;; backwards compatible non-interactive call
!           (eq grep-prompt-style 'shell)
!           (equal current-prefix-arg '(4)))
!       ;; REGEXP is command line
!       (compilation-start regexp 'grep-mode)
!       (let ((command (grep-expand-template
!                     grep-find-command-template
!                     regexp
!                     (and files (concat "-name "
!                                        (shell-quote-argument files)))
!                     nil  ;; we change default-directory to dir
!                     (and grep-find-ignored-directories
!                          (concat "\\( -path '*/"
!                                  (mapconcat #'identity
!                                             grep-find-ignored-directories
!                                             "' -o -path '*/")
!                                  "' \\) -prune -o "))))
!           (default-directory (file-name-as-directory (expand-file-name dir))))
!       (when command
!         (if (or (eq grep-prompt-style 'post)
!                 (equal current-prefix-arg '(16)))
!             (setq command
!                   (read-from-minibuffer "Confirm: "
!                                         command nil nil 'grep-find-history))
!           (push command grep-find-history))
!         (compilation-start command 'grep-mode))))))
  
+ ;;;###autoload
+ (defalias 'find-grep 'grep-find)
  
  (provide 'grep)
  

-- 
Kim F. Storm <address@hidden> http://www.cua.dk