bug#5260: 23.1.90; zrgrep grep-files-aliases

bug#5260: 23.1.90; zrgrep grep-files-aliases

[jidanni]

Regarding

    (defun zrgrep (regexp &optional files dir confirm grep-find-template)
      "Recursively grep for REGEXP in gzipped FILES in tree rooted at DIR.
    Like `rgrep' but uses `zgrep' for `grep-program', sets the default
    file name to `*.gz', and sets `grep-highlight-matches' to `always'."

I object. The default should be ALL, not *.gz, as at least I want to
comb all files, compressed or not... (the main bonus of zgrep(1)).

      (interactive
       (let ((grep-program "zgrep")
             (grep-find-template nil)  ; output of `grep-compute-defaults'
             (grep-find-command nil)
             (grep-host-defaults-alist nil)
             (grep-files-aliases '(("*.gz" . "*.gz") ; for `grep-read-files'
                                   ("all" . "* .*"))))

all should be ALL as it is more clear... and in fact just plain * .* is
even clearer.  Anyway, ALL should be at front. And there should be a
variable so one can customize about what should be in front...

bug#5260: 23.1.90; zrgrep grep-files-aliases

[Juri Linkov]

>     (defun zrgrep (regexp &optional files dir confirm grep-find-template)
>       "Recursively grep for REGEXP in gzipped FILES in tree rooted at DIR.
>     Like `rgrep' but uses `zgrep' for `grep-program', sets the default
>     file name to `*.gz', and sets `grep-highlight-matches' to `always'."
>
> I object. The default should be ALL, not *.gz, as at least I want to
> comb all files, compressed or not... (the main bonus of zgrep(1)).

zgrep(1) says the purpose of zgrep is to uncompress if necessary and
feed to grep.  So I agree that `all' should be the default because zgrep
is intended to work on all files, not just compressed files.

>       (interactive
>        (let ((grep-program "zgrep")
>              (grep-find-template nil)  ; output of `grep-compute-defaults'
>              (grep-find-command nil)
>              (grep-host-defaults-alist nil)
>              (grep-files-aliases '(("*.gz" . "*.gz") ; for `grep-read-files'
>                                    ("all" . "* .*"))))
>
> all should be ALL as it is more clear... and in fact just plain * .* is
> even clearer.

Lower-case `all' is consistent with other values in `grep-files-aliases'
where all aliases are lower-case.  We could try to find a way to
emphasize that they are aliases and not real file names.  I don't think
that upper-case will do that because there may be upper-case file names.
Maybe something like enclosing them in square brackets will help like e.g.

  [all]
  [el]
  [ch]

or with quotes:

  `all'
  `el'
  `ch'

> Anyway, ALL should be at front. And there should be a
> variable so one can customize about what should be in front...

Better to avoid adding a customizable variable for such a minor feature.
Moreover, we are now in feature freeze.  Meanwhile, it is possible to override
the front default value with the following line in .emacs:

  (setq grep-files-history '("all"))

because the file-reading function takes the first element of the history
by default.

So now I'd like to install only the following patch that puts `all' in front
and also fixes a bug I just discovered: when `zrgrep' is run first before
normal `grep' commands then some grep variables get wrong default values.
A fix is to run `grep-compute-defaults' at the beginning of `zrgrep' like
all other grep commands do:

=== modified file 'lisp/progmodes/grep.el'
--- lisp/progmodes/grep.el	2009-12-07 17:35:47 +0000
+++ lisp/progmodes/grep.el	2009-12-28 10:42:23 +0000
@@ -996,12 +996,20 @@
 Like `rgrep' but uses `zgrep' for `grep-program', sets the default
 file name to `*.gz', and sets `grep-highlight-matches' to `always'."
   (interactive
+   (progn
+     ;; Compute standard default values.
+     (grep-compute-defaults)
+     ;; Compute the default zrgrep command running `grep-compute-defaults'
+     ;; for grep program "zgrep", but not changing global values.
    (let ((grep-program "zgrep")
-	 (grep-find-template nil)  ; output of `grep-compute-defaults'
+	   ;; Don't change global values for variables computed
+	   ;; by `grep-compute-defaults'.
+	   (grep-find-template nil)
 	 (grep-find-command nil)
 	 (grep-host-defaults-alist nil)
-	 (grep-files-aliases '(("*.gz" . "*.gz") ; for `grep-read-files'
-			       ("all" . "* .*"))))
+	   ;; Use for `grep-read-files'
+	   (grep-files-aliases '(("all" . "* .*")
+				 ("gz"  . "*.gz"))))
      ;; Recompute defaults using let-bound values above.
      (grep-compute-defaults)
      (cond
@@ -1015,7 +1023,7 @@
 		(dir (read-directory-name "Base directory: "
 					  nil default-directory t))
 		(confirm (equal current-prefix-arg '(4))))
-	   (list regexp files dir confirm grep-find-template))))))
+	     (list regexp files dir confirm grep-find-template)))))))
   ;; Set `grep-highlight-matches' to `always'
   ;; since `zgrep' puts filters in the grep output.
   (let ((grep-highlight-matches 'always))

-- 
Juri Linkov
http://www.jurta.org/emacs/

bug#5260: 23.1.90; zrgrep grep-files-aliases

[Juri Linkov]

> So now I'd like to install only the following patch that puts `all' in front
> and also fixes a bug I just discovered: when `zrgrep' is run first before
> normal `grep' commands then some grep variables get wrong default values.
> A fix is to run `grep-compute-defaults' at the beginning of `zrgrep' like
> all other grep commands do:

Fix installed.  More changes after feature freeze.

-- 
Juri Linkov
http://www.jurta.org/emacs/