Re: display-completion-list should not strip text properties

[Kim F. Storm]

Chong Yidong <address@hidden> writes:

> The two changes that we have to reimplement are the following:
>
> 1997-03-24  Kevin Rodgers  <address@hidden>
>
>       * compile.el (grep-program): New variable.
>       (grep-command): Use it, and test whether it supports the -e
>       option; fix doc string (last command is stored in history variable).
>       (grep-null-device): Declare before grep-program and grep-command.
>       (grep-find-use-xargs, grep-find-command, grep-find-history):
>       New variables.
>       (grep-find): New command.
>       (grep): Only concatenate grep-null-device to COMMAND when it's
>       not nil (to support grep-find).


EXECUTIVE SUMMARY:
------------------

  We don't have to do anything about this.


ANALYSIS:
---------

I (and others) have already modified the code introduced by those
changes several times, so parts of it is no longer present -- or very
different from the original code.  However, there are still parts which
are quite close to Kevin's original code.


Let's go over those changes:

Index: compile.el
===================================================================
RCS file: /cvsroot/emacs/emacs/lisp/progmodes/compile.el,v
retrieving revision 1.158
retrieving revision 1.159
diff -c -r1.158 -r1.159
*** compile.el  21 Feb 1997 09:46:03 -0000      1.158
--- compile.el  24 Mar 1997 23:59:44 -0000      1.159
***************
*** 256,263 ****
  
! (defvar grep-command "grep -n "
!   "Last grep command used in \\[grep]; default for next grep.")
  
--- 256,307 ----
  
! ;; Use zgrep if available, to work nicely with compressed files.
! ;; Otherwise, use ordinary grep.
! (defvar grep-program
!   (if (equal (condition-case nil      ; in case "zgrep" isn't in exec-path
!                (call-process "zgrep" nil nil nil
!                              "foo" grep-null-device)
!              (error nil))
!            1)
!       "zgrep"
!     "grep")
!   "The default grep program for `grep-command' and `grep-find-command'.")


This change has been reverted - but Kevin's code is still included as a comment.
We could just remove the comment.


! ;; Use -e if grep supports it,
! ;; because that avoids lossage if the pattern starts with `-'.
! (defvar grep-command
!   (if (equal (condition-case nil      ; in case "grep" isn't in exec-path
!                (call-process grep-program nil nil nil
!                              "-e" "foo" grep-null-device)
!              (error nil))
!            1)
!       (format "%s -n -e " grep-program)
!     (format "%s -n " grep-program))
!   "The default grep command for \\[grep].")

The variable still exists, but the code to initialize it has been reworked,
and is no longer Kevin's work.

! (defvar grep-find-use-xargs
!   (if (equal (call-process "find" nil nil nil
!                          grep-null-device "-print0")
!            0)
!       'gnu)
!   "Whether \\[grep-find] uses the `xargs' utility by default.
! 
! If nil, it uses `grep -exec'; if `gnu', it uses `find -print0' and `xargs -0';
! if not nil and not `gnu', it uses `find -print' and `xargs'.
! 
! This variable's value takes effect when `compile.el' is loaded
! by influencing the default value for the variable `grep-find-command'.")

Ditto.  The current doc string is 50% similar to Kevin's work.

! 
! (defvar grep-find-command
!   (cond ((eq grep-find-use-xargs 'gnu)
!        (format "find . -type f -print0 | xargs -0 -e %s" grep-command))
!       (grep-find-use-xargs
!        (format "find . -type f -print | xargs %s" grep-command))
!       (t (cons (format "find . -type f -exec %s {} /dev/null \\;"
!                        grep-command)
!                (+ 22 (length grep-command)))))
!   "The default find command for \\[grep-find].")


The variable still exists, but otherwise, this part has been
completely reworked.


***************
*** 308,313 ****
--- 352,358 ----

+ (defvar grep-find-history nil)

Still exists -- but that's a trivial change (how else would you define
the var?)


***************
*** 392,402 ****
    ;; Setting process-setup-function makes exit-message-function work
    ;; even when async processes aren't supported.
    (let* ((compilation-process-setup-function 'grep-process-setup)
!        (buf (compile-internal (concat command-args " " grep-null-device)
                                "No more grep hits" "grep"
                                ;; Give it a simpler regexp to match.
                                nil grep-regexp-alist)))))
  
  (defun compile-internal (command error-message
                                 &optional name-of-mode parser regexp-alist
                                 name-function)
--- 437,464 ----
    ;; Setting process-setup-function makes exit-message-function work
    ;; even when async processes aren't supported.
    (let* ((compilation-process-setup-function 'grep-process-setup)
!        (buf (compile-internal (if grep-null-device
!                                   (concat command-args " " grep-null-device)
!                                 command-args)
                                "No more grep hits" "grep"
                                ;; Give it a simpler regexp to match.
                                nil grep-regexp-alist)))))


This part is also completely reworked.
  
+ 
+ ;;;###autoload
+ (defun grep-find (command-args)
+   "Run grep via find, with user-specified args, and 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
+    (list (read-from-minibuffer "Run find (like this): "
+                              grep-find-command nil nil 'grep-find-history)))
+   (let ((grep-null-device nil))               ; see grep
+     (grep command-args)))
+ 

The grep-find command is still present with the above code and doc string
mostly intact (the code has been extended in various ways - but you can still
find those 5 lines of code in the current version of the function.

However, Kevin's code and doc string is clearly derived from the
already existing grep defun:

(defun grep (command-args)
  "Run grep, with user-specified args, and collect output in a buffer.
While grep 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 grep command."
  (interactive
   (list (read-from-minibuffer "Run grep (like this): "
                               grep-command nil nil 'grep-history)))
  ;; Setting process-setup-function makes exit-message-function work
  ;; even when async processes aren't supported.
  (let* ((compilation-process-setup-function 'grep-process-setup)
         (buf (compile-internal (concat command-args " " grep-null-device)
                                "No more grep hits" "grep"
                                ;; Give it a simpler regexp to match.
                                nil grep-regexp-alist)))))



CONCLUSION:
-----------

Except for the commentary about zgrep [which I have just removed],
the relevant code has either been reworked already, or is trivially
derived from existing code.

To conclude, I don't think we need to do anything about the changes
related to grep-find functionality.

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