Re: collect-string

[Tak Ota]

Stefan,

Thanks for your quick review.

Fri, 3 Dec 2010 11:17:50 -0800: Stefan Monnier <address@hidden> wrote:

> Thanks, that looks pretty good now.  A few more nitpicks while I'm here:
> 
> > + (defvar occur-collect-submatch-history '("1")
> > +   "The history of list of parenthesized expression numbers as a
> > +   string in occur's collect operation")
> 
> Usually history vars start at nil (and you can save them via savehist
> if you so prefer).

If a user takes the default, which is the first item in the history,
it makes no sense.  This value is asked only when there is at least
one subexpression so I thought having something would suggest the
usage to the user.

> 
> >   (defun occur-read-primary-args ()
> > !   (let ((regexp)
> > !   (do-collect (consp current-prefix-arg)))
> > !     (list (setq regexp
> > !           (read-regexp (if do-collect
> > !                            "Collect strings matching regexp"
> > !                          "List lines matching regexp")
> > !                        (car regexp-history)))
> 
> Always try to set the var directly in the let rather than via
> a separate setq:
> 
>   (let* ((do-collect (consp current-prefix-arg))
>          (regexp (read-regexp (if do-collect
>                                   "Collect strings matching regexp"
>                                 "List lines matching regexp")
>                               (car regexp-history))))
>     (list regexp ...))

This I realized after sent out the message and made exactly same
change as you suggest.

> 
> > !     (if do-collect
> > !         (if (zerop (regexp-opt-depth regexp))
> > !             ;; no subexpression so collect entire the match
> 
> Comments should start with a capital letter and end with a ".".

You are right.

> 
> > !             '(0)
> > !           ;; construct a list of subexpression integers
> > !           (mapcar 'string-to-number
> > !                   (split-string
> > !                    (let ((default (car occur-collect-submatch-history)))
> > !                      (read-from-minibuffer
> > !                       (format "Subexpressions to collect (default %s): " 
> > default)
> > !                       "" nil nil 'occur-collect-submatch-history default)
> > !                      (car occur-collect-submatch-history)))))
>                            ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
> 
> This should be `default', right?

No, this is the new input value as read-from-minibuffer prepends this
value to the history.

> > ! When NLINES is a list of integers or when the function is called
> > ! interactively with prefix argument without a number (`C-u' alone
> > ! as prefix) the matching strings are collected into the `*Occur*'
> > ! buffer.  The parenthesized match strings in REGEXP indicated by
> > ! the integers in NLINES are collected.  For example, providing
> > ! \"defun \\(\\S +\\)\" for REGEXP and (1) for NLINES collects all
> > ! the function names in a lisp program.  When there is no
> > ! parenthesized subexpressions in REGEXP the entire match is
> > ! collected."
> 
> Hmm... I didn't think of NLINES turning into a list of submatch-numbers,
> but that makes me think that maybe we should go one step further and
> turn it into a string, like the ones used in replace-match.

This is truly brilliant.  Thank you very much.  I love this idea
bouncing process.  See below.

-Tak

*** ../../../d/pub/emacs/emacs-23.2.90/lisp/replace.el  Thu Dec  2 16:33:19 2010
--- replace.el  Fri Dec  3 14:28:27 2010
***************
*** 527,532 ****
--- 527,535 ----
  Maximum length of the history list is determined by the value
  of `history-length', which see.")
  
+ (defvar occur-collect-regexp-history '("\\1")
+   "History of regexp for occur's collect operation")
+ 
  (defun read-regexp (prompt &optional default-value)
    "Read regexp as a string using the regexp history and some useful defaults.
  Prompt for a regular expression with PROMPT (without a colon and
***************
*** 1028,1037 ****
        (nreverse result))))
  
  (defun occur-read-primary-args ()
!   (list (read-regexp "List lines matching regexp"
!                    (car regexp-history))
!       (when current-prefix-arg
!         (prefix-numeric-value current-prefix-arg))))
  
  (defun occur-rename-buffer (&optional unique-p interactive-p)
    "Rename the current *Occur* buffer to *Occur: original-buffer-name*.
--- 1031,1056 ----
        (nreverse result))))
  
  (defun occur-read-primary-args ()
!   (let* ((perform-collect (consp current-prefix-arg))
!          (regexp (read-regexp (if perform-collect
!                                   "Collect strings matching regexp"
!                                 "List lines matching regexp")
!                               (car regexp-history))))
!     (list regexp
!         (if perform-collect
!             ;; Perform collect operation
!             (if (zerop (regexp-opt-depth regexp))
!                 ;; No subexpression so collect the entire match.
!                 "\\&"
!               ;; Get the regexp for collection pattern.
!               (let ((default (car occur-collect-regexp-history)))
!                 (read-from-minibuffer
!                  (format "Regexp to collect (default %s): " default)
!                  "" nil nil 'occur-collect-regexp-history default)
!                 (car occur-collect-regexp-history)))
!           ;; Otherwise normal occur takes numerical prefix argument.
!           (when current-prefix-arg
!             (prefix-numeric-value current-prefix-arg))))))
  
  (defun occur-rename-buffer (&optional unique-p interactive-p)
    "Rename the current *Occur* buffer to *Occur: original-buffer-name*.
***************
*** 1064,1070 ****
  \\<occur-mode-map>\\[describe-mode] in that buffer will explain how.
  
  If REGEXP contains upper case characters (excluding those preceded by `\\')
! and `search-upper-case' is non-nil, the matching is case-sensitive."
    (interactive (occur-read-primary-args))
    (occur-1 regexp nlines (list (current-buffer))))
  
--- 1083,1100 ----
  \\<occur-mode-map>\\[describe-mode] in that buffer will explain how.
  
  If REGEXP contains upper case characters (excluding those preceded by `\\')
! and `search-upper-case' is non-nil, the matching is case-sensitive.
! 
! When NLINES is a string or when the function is called
! interactively with prefix argument without a number (`C-u' alone
! as prefix) the matching strings are collected into the `*Occur*'
! buffer by using NLINES as a replacement regexp.  NLINES may
! contain \\& and \\N which convention follows `replace-match'.
! For example, providing \"defun\\s +\\(\\S +\\)\" for REGEXP and
! \"\\1\" for NLINES collects all the function names in a lisp
! program.  When there is no parenthesized subexpressions in REGEXP
! the entire match is collected.  In any case the searched buffers
! are not modified."
    (interactive (occur-read-primary-args))
    (occur-1 regexp nlines (list (current-buffer))))
  
***************
*** 1146,1165 ****
      (setq occur-buf (get-buffer-create buf-name))
  
      (with-current-buffer occur-buf
!       (occur-mode)
        (let ((inhibit-read-only t)
            ;; Don't generate undo entries for creation of the initial contents.
            (buffer-undo-list t))
        (erase-buffer)
!       (let ((count (occur-engine
!                     regexp active-bufs occur-buf
!                     (or nlines list-matching-lines-default-context-lines)
!                     (if (and case-fold-search search-upper-case)
!                         (isearch-no-upper-case-p regexp t)
!                       case-fold-search)
!                     list-matching-lines-buffer-name-face
!                     nil list-matching-lines-face
!                     (not (eq occur-excluded-properties t)))))
          (let* ((bufcount (length active-bufs))
                 (diff (- (length bufs) bufcount)))
            (message "Searched %d buffer%s%s; %s match%s for `%s'"
--- 1176,1218 ----
      (setq occur-buf (get-buffer-create buf-name))
  
      (with-current-buffer occur-buf
!       (if (stringp nlines)
!         (fundamental-mode) ;; This is for collect opeartion.
!       (occur-mode))
        (let ((inhibit-read-only t)
            ;; Don't generate undo entries for creation of the initial contents.
            (buffer-undo-list t))
        (erase-buffer)
!       (let ((count
!              (if (stringp nlines)
!                    ;; Treat nlines as a regexp to collect.
!                  (let ((bufs active-bufs)
!                        (count 0))
!                    (while bufs
!                      (with-current-buffer (car bufs)
!                        (save-excursion
!                          (goto-char (point-min))
!                          (while (re-search-forward regexp nil t)
!                              ;; Insert the replacement regexp.
!                            (let ((str (match-substitute-replacement nlines)))
!                              (if str
!                                  (with-current-buffer occur-buf
!                                    (insert str)
!                                    (setq count (1+ count))
!                                    (or (zerop (current-column))
!                                        (insert "\n"))))))))
!                        (setq bufs (cdr bufs)))
!                      count)
!                ;; Perform normal occur.
!                (occur-engine
!                 regexp active-bufs occur-buf
!                 (or nlines list-matching-lines-default-context-lines)
!                 (if (and case-fold-search search-upper-case)
!                     (isearch-no-upper-case-p regexp t)
!                   case-fold-search)
!                 list-matching-lines-buffer-name-face
!                 nil list-matching-lines-face
!                 (not (eq occur-excluded-properties t))))))
          (let* ((bufcount (length active-bufs))
                 (diff (- (length bufs) bufcount)))
            (message "Searched %d buffer%s%s; %s match%s for `%s'"