Re: Patch that adds help to isearch

[Lennart Borgman]

Richard M. Stallman wrote:

>    Some months ago we discussed help for isearch-forward/backward. I tried 
>    several alternatives at that time, but did not send in any patch. The 
>    attached patch is a simple way to give help. It simply defines C-h and 
>    f1 to call isearch-mode-help
>
> I don't want to make such a change now, and perhaps not ever.  Too
> many control characters are special already in isearch.  (I turned off
> one of them a couple of months ago.)
>  
I read in the code

   ;; Turned off because I find I expect to get the global definition--rms.
   ;; ;; Instead bind C-h to special help command for isearch-mode.
   ;; (define-key map "\C-h" 'isearch-mode-help)

I started working on that some months ago but gave up. It was rather 
complicated to stay in isearch-mode and do this and take care of 
different window configurations etc. I thought it was not worth the 
trouble and in the case of isearch-mode I also found it more helpful 
with the isearch-mode-help.

The change I proposed is simple and I do not beleive it will be any 
problem to remove it later if we decide so. However for all Emacs users 
that do not know the details of isearch-mode (like myself) I think it is 
very useful. Some keys are however used:

1) The help keys (C-h and f1). This of course blocks the global 
definition, but is this a problem here?

2) Scroll other window up or down. Keys bound to those keys are used for 
scrolling the help window, but only if the other window is displaying 
the help buffer.

I do not want to argue but I want to be clear.


>    I also found some small errors which I tried to correct. This was 
>    something with the isearch-ring but I can not remember the details right 
>    now. I included that also in the patch however.
>
> Can you please separate those bug fixes out
> and explain the bugs they fix?
>  
I looked at my old bug fixes but these are some months old. What I 
wanted to fix was the history handling for isearch edit. My old fix did 
not work any more so I have made a new one which I attach. This does a 
few things:

1) M-p shows the last used value saved in the history list for isearch 
(called `search-ring') when entering minibuff editing.
2) M-n shows an empty string in this case.
3) The default for editing is just the empty string, not the first 
string in the history list like now.

I might have misunderstood something here, so please test.

I believe there was a typo too. A lonely (quit ...).

>    I renamed search-ring-update to isearch-ring-update which seems more 
>    consistent. Maybe that should not be done if that breaks something?
>
> I don't want to change these names now.
>  
Ok. Could the isearch-mode-help be changed a bit instead? Currently it 
tells you to do

  M-x apropos RET, search-.* RET

That gives a lot of unwanted hits. I suggest to give a hint to use

  M-x customize-group RET, isearch RET

instead.

>    BTW I think there may be some useful key definitions missing in the 
>    help. Could you for example not switch the direction with C-r? Should 
>    not that be added to the help?
>
> Could you be more specific about the change you mean?
>  
Sorry, my mistake. It is actually the second time I do this I remember 
now. I did not find C-r in the help text because is it not in the left 
margin, but it is there.



Index: isearch.el
===================================================================
RCS file: /cvsroot/emacs/emacs/lisp/isearch.el,v
retrieving revision 1.276
diff -c -r1.276 isearch.el
*** isearch.el  24 Nov 2005 09:59:11 -0000      1.276
--- isearch.el  27 Nov 2005 22:28:09 -0000
***************
*** 1035,1041 ****
                         (isearch-message-prefix nil nil isearch-nonincremental)
                         isearch-string
                         minibuffer-local-isearch-map nil
!                        (if isearch-regexp 'regexp-search-ring 'search-ring)
                         nil t)
                      isearch-new-message
                      (mapconcat 'isearch-text-char-description
--- 1035,1043 ----
                         (isearch-message-prefix nil nil isearch-nonincremental)
                         isearch-string
                         minibuffer-local-isearch-map nil
!                        (cons 
!                         (if isearch-regexp 'regexp-search-ring 'search-ring)
!                         1)
                         nil t)
                      isearch-new-message
                      (mapconcat 'isearch-text-char-description
***************
*** 1054,1068 ****
                  isearch-word isearch-new-word))
  
          ;; Empty isearch-string means use default.
!         (if (= 0 (length isearch-string))
!             (setq isearch-string (or (car (if isearch-regexp
!                                               regexp-search-ring
!                                             search-ring))
!                                      "")
! 
!                   isearch-message
!                   (mapconcat 'isearch-text-char-description
!                              isearch-string ""))
            ;; This used to set the last search string,
            ;; but I think it is not right to do that here.
            ;; Only the string actually used should be saved.
--- 1056,1071 ----
                  isearch-word isearch-new-word))
  
          ;; Empty isearch-string means use default.
!           ;; Why should it mean default?
! ;;      (if (= 0 (length isearch-string))
! ;;          (setq isearch-string (or (car (if isearch-regexp
! ;;                                            regexp-search-ring
! ;;                                          search-ring))
! ;;                                   "")
! 
! ;;                isearch-message
! ;;                (mapconcat 'isearch-text-char-description
! ;;                           isearch-string ""))
            ;; This used to set the last search string,
            ;; but I think it is not right to do that here.
            ;; Only the string actually used should be saved.
***************
*** 1081,1091 ****
              ;; The search done message is confusing when the string
              ;; is empty, so erase it.
              (if (equal isearch-string "")
!                 (message "")))))
  
!     (quit  ; handle abort-recursive-edit
!      (isearch-abort)  ;; outside of let to restore outside global values
!      )))
  
  (defun isearch-nonincremental-exit-minibuffer ()
    (interactive)
--- 1084,1094 ----
              ;; The search done message is confusing when the string
              ;; is empty, so erase it.
              (if (equal isearch-string "")
!                 (message ""))))
  
!         (quit  ; handle abort-recursive-edit
!          (isearch-abort)  ;; outside of let to restore outside global values
!          ))))
  
  (defun isearch-nonincremental-exit-minibuffer ()
    (interactive)
***************
*** 1833,1842 ****
        ()
        (set yank-pointer-name
           (setq yank-pointer
!                (mod (+ (or yank-pointer 0)
!                        (if advance -1 1))
!                     length)))
!       (setq isearch-string (nth yank-pointer ring)
            isearch-message (mapconcat 'isearch-text-char-description
                                       isearch-string "")))))
  
--- 1836,1847 ----
        ()
        (set yank-pointer-name
           (setq yank-pointer
!                  (if yank-pointer
!                      (mod (+ yank-pointer
!                              (if advance -1 1))
!                           length)
!                    (if advance -1 0))))
!       (setq isearch-string (if (< yank-pointer 0) "" (nth yank-pointer ring))
            isearch-message (mapconcat 'isearch-text-char-description
                                       isearch-string "")))))