GNU bug report logs - #16035
24.3.50; Custom isearch broken on trunk (e.g. change-log-mode)

Previous Next

To add a comment to this bug, you must first unarchive it, by sending
a message to control AT debbugs.gnu.org, with unarchive 16035 in the body.
You can then email your comments to 16035 AT debbugs.gnu.org in the normal way.

Toggle the display of automated, internal messages from the tracker.

Message #5 received at submit <at> debbugs.gnu.org (full text, mbox):

From: Anders Lindgren <andlind <at> gmail.com>
To: bug-gnu-emacs <at> gnu.org
Subject: 24.3.50; Custom isearch broken on trunk (e.g. change-log-mode)
Date: Tue, 3 Dec 2013 10:38:50 +0100

[Message part 1 (text/plain, inline)]

Recent changes in the isearch package has made packages that customize
isearch fail.

Case 1:

change-log-mode customizes isearch so that you could search multiple files.

    Emacs -Q lisp/ChangeLog
    C-s refresh-d
    C-s
    C-a

Here, the symbol "font-lock-refresh-defaults" in ChangeLog.16 has been
found. C-a typically exits isearch, however, now it doesn't. You can see
this on the fact that the isearch highlight is still active and when more
characters are typed, they are added to the search string.

This broke on revision 114586.

Log for revision 114586:

fixes bug: http://debbugs.gnu.org/15200

committer: Juri Linkov <juri <at> jurta.org>

branch nick: trunk

timestamp: Wed 2013-10-09 02:20:12 +0300

message:

  * lisp/isearch.el (isearch-help-map, isearch-mode-map): Don't bind [t]

  to isearch-other-control-char.

  (isearch-mode): Add isearch-pre-command-hook to pre-command-hook

  and isearch-post-command-hook to post-command-hook.

  (isearch-done): Remove isearch-pre-command-hook from pre-command-hook

  and isearch-post-command-hook from post-command-hook.

  (isearch-unread-key-sequence)

  (isearch-reread-key-sequence-naturally)

  (isearch-lookup-scroll-key, isearch-other-control-char)

  (isearch-other-meta-char): Remove functions.

  (isearch-pre-command-hook, isearch-post-command-hook):

  New functions based on isearch-other-meta-char rewritten

  relying on the new behavior of overriding-terminal-local-map

  that does not replace the local keymaps any more.

Case 2:

isearch in the popular third-part package "folding" is broken, same
symptoms as above. (If you don't have it I can supply a copy.)

    ./nextstep/Emacs.app/Contents/MacOS/Emacs -Q folding.el -l folding.el
    y
    M-x folding-mode RET
    C-s defvar
    C-a

This broke in revision 114633.

Log for 114633:

committer: Stefan Monnier <monnier <at> iro.umontreal.ca>

branch nick: trunk

timestamp: Fri 2013-10-11 21:10:25 -0400

message:

  * lisp/isearch.el (isearch-pre-command-hook): Don't build in knowledge
about

  internals of universal-argument.

    -- Anders Lindgren





In GNU Emacs 24.3.50.1 (x86_64-apple-darwin13.0.0, NS apple-appkit-1265.00)
 of 2013-12-03 on macpro.lan
Bzr revision: 115300 rgm <at> gnu.org-20131130084228-ifjblwsxjxkeeta7
Windowing system distributor `Apple', version 10.3.1265
Configured using:
 `configure --with-ns'

Important settings:
  value of $LC_CTYPE: UTF-8
  locale-coding-system: utf-8-unix
  default enable-multibyte-characters: t

Major mode: Change Log

Minor modes in effect:
  bug-reference-mode: t
  tooltip-mode: t
  electric-indent-mode: t
  mouse-wheel-mode: t
  tool-bar-mode: t
  menu-bar-mode: t
  file-name-shadow-mode: t
  global-font-lock-mode: t
  font-lock-mode: t
  blink-cursor-mode: t
  auto-composition-mode: t
  auto-encryption-mode: t
  auto-compression-mode: t
  line-number-mode: t
  transient-mark-mode: t

Recent input:
C-s r e f r e s h - d C-s C-a <up> d C-g C-g C-g C-h
v e m a c z - b <backspace> <backspace> <backspace>
s - b z <tab> <return> C-x 1 <menu-bar> <help-menu>
<send-emacs-bug-report>

Recent messages:
For information about GNU Emacs and the GNU system, type C-h C-a.
Quit [2 times]
Type C-x 1 to delete the help window.

Load-path shadows:
None found.

Features:
(shadow sort gnus-util mail-extr emacsbug message format-spec rfc822 mml
mml-sec mm-decode mm-bodies mm-encode mail-parse rfc2231 mailabbrev
gmm-utils mailheader sendmail rfc2047 rfc2045 ietf-drums mm-util
mail-prsvr mail-utils help-mode easymenu help-fns misearch multi-isearch
vc-bzr bug-reference add-log time-date tooltip electric uniquify
ediff-hook vc-hooks lisp-float-type mwheel ns-win tool-bar dnd fontset
image regexp-opt fringe tabulated-list newcomment lisp-mode prog-mode
register page menu-bar rfn-eshadow timer select scroll-bar mouse
jit-lock font-lock syntax facemenu font-core frame cham georgian
utf-8-lang misc-lang vietnamese tibetan thai tai-viet lao korean
japanese hebrew greek romanian slovak czech european ethiopic indian
cyrillic chinese case-table epa-hook jka-cmpr-hook help simple abbrev
minibuffer nadvice loaddefs button faces cus-face macroexp files
text-properties overlay sha1 md5 base64 format env code-pages mule
custom widget hashtable-print-readable backquote make-network-process
cocoa ns multi-tty emacs)

[Message part 2 (text/html, inline)]

Message #8 received at 16035 <at> debbugs.gnu.org (full text, mbox):

From: Juri Linkov <juri <at> jurta.org>
To: Anders Lindgren <andlind <at> gmail.com>
Cc: 16035 <at> debbugs.gnu.org
Subject: Re: bug#16035: 24.3.50;
 Custom isearch broken on trunk (e.g. change-log-mode)
Date: Wed, 04 Dec 2013 02:18:04 +0200

> Case 2:
>
> isearch in the popular third-part package "folding" is broken, same
> symptoms as above. (If you don't have it I can supply a copy.)
>
>     ./nextstep/Emacs.app/Contents/MacOS/Emacs -Q folding.el -l folding.el
>     y
>     M-x folding-mode RET
>     C-s defvar
>     C-a

folding.el performs such initialization in `folding-isearch-hook-function':

          (funcall (symbol-function 'set)
                   'overriding-terminal-local-map folding-isearch-mode-map)

and this condition in `isearch-pre-command-hook' fails:

     ((not (eq overriding-terminal-local-map isearch-mode-map)))

Maybe this condition should be checked only when handling a scrolling function
or prefix argument like in the patch that I posted in bug#15568.

Message #11 received at 16035 <at> debbugs.gnu.org (full text, mbox):

From: Juri Linkov <juri <at> jurta.org>
To: Anders Lindgren <andlind <at> gmail.com>
Cc: 16035 <at> debbugs.gnu.org
Subject: Re: bug#16035: 24.3.50;
 Custom isearch broken on trunk (e.g. change-log-mode)
Date: Wed, 04 Dec 2013 02:14:01 +0200

> Case 1:
>
> change-log-mode customizes isearch so that you could search multiple files.
>
>     Emacs -Q lisp/ChangeLog
>     C-s refresh-d
>     C-s
>     C-a
>
> Here, the symbol "font-lock-refresh-defaults" in ChangeLog.16 has been
> found. C-a typically exits isearch, however, now it doesn't. You can see
> this on the fact that the isearch highlight is still active and when more
> characters are typed, they are added to the search string.

Thanks for the bug report.  I fixed this now.

Message #14 received at 16035 <at> debbugs.gnu.org (full text, mbox):

From: Juri Linkov <juri <at> jurta.org>
To: Anders Lindgren <andlind <at> gmail.com>
Cc: 16035 <at> debbugs.gnu.org
Subject: Re: bug#16035: 24.3.50;
 Custom isearch broken on trunk (e.g. change-log-mode)
Date: Wed, 04 Dec 2013 02:37:41 +0200

>      ((not (eq overriding-terminal-local-map isearch-mode-map)))
>
> Maybe this condition should be checked only when handling a scrolling function
> or prefix argument like in the patch that I posted in bug#15568.

Actually this doesn't help: adding this condition where prefix argument
is handled still doesn't exit Isearch.  Currently I have no more ideas.

Message #17 received at 16035 <at> debbugs.gnu.org (full text, mbox):

From: Stefan Monnier <monnier <at> iro.umontreal.ca>
To: Juri Linkov <juri <at> jurta.org>
Cc: 16035 <at> debbugs.gnu.org, Anders Lindgren <andlind <at> gmail.com>
Subject: Re: bug#16035: 24.3.50;
 Custom isearch broken on trunk (e.g. change-log-mode)
Date: Tue, 03 Dec 2013 23:31:38 -0500

> folding.el performs such initialization in `folding-isearch-hook-function':
>
>           (funcall (symbol-function 'set)
>                    'overriding-terminal-local-map folding-isearch-mode-map)

Maybe folding.el is out of luck and will need to be updated.

An alternative is to make isearch record the
overriding-terminal-local-map that was set "at the beginning" (which
should be folding-isearch-mode-map for the folding-isearch and
isearch-mode-map in the normal isearch), and then
change the check to

     ((not (eq overriding-terminal-local-map
               isearch-saved-overriding-local-map))

But depending on how folding-isearch-mode-map is defined this may not be
sufficient, and changes to folding.el will be needed.


        Stefan

Message #20 received at 16035 <at> debbugs.gnu.org (full text, mbox):

From: Anders Lindgren <andlind <at> gmail.com>
To: Stefan Monnier <monnier <at> iro.umontreal.ca>
Cc: Juri Linkov <juri <at> jurta.org>, 16035 <at> debbugs.gnu.org
Subject: Re: bug#16035: 24.3.50;
 Custom isearch broken on trunk (e.g. change-log-mode)
Date: Wed, 4 Dec 2013 12:48:52 +0100

[Message part 1 (text/plain, inline)]

Hi Stefan!

I tried you solution and it works perfectly! Just make sure to save the
keymap after the call to "(run-hooks 'isearch-mode-hook)", as this is where
packages like "folding" installs its keymap.

Juri and Stefan:

While looking around the isearch code, I came up with a theory why
multi-buffer search in change-log-mode no longer works. isearch adds its
hook to the LOCAL pre-command-hook. As change-log-mode search change buffer
and the hook is not installed in the new buffer, the user can't exit
isearch.

Sincerely,
    Anders Lindgren


On Wed, Dec 4, 2013 at 5:31 AM, Stefan Monnier <monnier <at> iro.umontreal.ca>wrote:

> > folding.el performs such initialization in
> `folding-isearch-hook-function':
> >
> >           (funcall (symbol-function 'set)
> >                    'overriding-terminal-local-map
> folding-isearch-mode-map)
>
> Maybe folding.el is out of luck and will need to be updated.
>
> An alternative is to make isearch record the
> overriding-terminal-local-map that was set "at the beginning" (which
> should be folding-isearch-mode-map for the folding-isearch and
> isearch-mode-map in the normal isearch), and then
> change the check to
>
>      ((not (eq overriding-terminal-local-map
>                isearch-saved-overriding-local-map))
>
> But depending on how folding-isearch-mode-map is defined this may not be
> sufficient, and changes to folding.el will be needed.
>
>
>         Stefan
>

[Message part 2 (text/html, inline)]

Message #23 received at 16035 <at> debbugs.gnu.org (full text, mbox):

From: Stefan Monnier <monnier <at> iro.umontreal.ca>
To: Anders Lindgren <andlind <at> gmail.com>
Cc: Juri Linkov <juri <at> jurta.org>, 16035 <at> debbugs.gnu.org
Subject: Re: bug#16035: 24.3.50;
 Custom isearch broken on trunk (e.g. change-log-mode)
Date: Wed, 04 Dec 2013 12:57:38 -0500

> I tried you solution and it works perfectly! Just make sure to save the
> keymap after the call to "(run-hooks 'isearch-mode-hook)", as this is where
> packages like "folding" installs its keymap.

Great to hear.  Juri, can you do that for me?

> While looking around the isearch code, I came up with a theory why
> multi-buffer search in change-log-mode no longer works. isearch adds its
> hook to the LOCAL pre-command-hook. As change-log-mode search change buffer
> and the hook is not installed in the new buffer, the user can't exit
> isearch.

Sounds right, as well.  Juri, can you take care of that while you're at it?

BTW, Anders, I see you have signed more paperwork for Emacs than pretty
much anybody else, yet none of them covers changes to isearch.el.
If I were you, I'd sign a general assignment for all of Emacs and be
done with it once and for all.  If that's OK with you, then just tell me
and I'll send you the relevant info,


        Stefan

Message #26 received at 16035 <at> debbugs.gnu.org (full text, mbox):

From: Anders Lindgren <andlind <at> gmail.com>
To: Stefan Monnier <monnier <at> iro.umontreal.ca>
Cc: 16035 <at> debbugs.gnu.org
Subject: Re: bug#16035: 24.3.50;
 Custom isearch broken on trunk (e.g. change-log-mode)
Date: Wed, 4 Dec 2013 22:49:34 +0100

[Message part 1 (text/plain, inline)]

Hi Stefan!

For me, personally, I would be glad to sign a general assignment.
Unfortunately, my employer does not want to sign a general sign-off
agreement -- they prefer to do this case by case. (I respect this -- after
all, the company I work for develop IDE:s for the embedded industry so
there is a risk of conflict of interests, even though I mainly work on the
compiler and not the editor.)

In this case, for isearch.el, I do not see the need for an assignment, as I
only have reported bugs I have found, not contributed actual code. However,
if you think it is necessary, I will sign an assignment -- hopefully we can
define a scope that is acceptable to my employer, like "basic Emacs
functionality".

    -- Anders



On Wed, Dec 4, 2013 at 6:57 PM, Stefan Monnier <monnier <at> iro.umontreal.ca>wrote:

> > I tried you solution and it works perfectly! Just make sure to save the
> > keymap after the call to "(run-hooks 'isearch-mode-hook)", as this is
> where
> > packages like "folding" installs its keymap.
>
> Great to hear.  Juri, can you do that for me?
>
> > While looking around the isearch code, I came up with a theory why
> > multi-buffer search in change-log-mode no longer works. isearch adds its
> > hook to the LOCAL pre-command-hook. As change-log-mode search change
> buffer
> > and the hook is not installed in the new buffer, the user can't exit
> > isearch.
>
> Sounds right, as well.  Juri, can you take care of that while you're at it?
>
> BTW, Anders, I see you have signed more paperwork for Emacs than pretty
> much anybody else, yet none of them covers changes to isearch.el.
> If I were you, I'd sign a general assignment for all of Emacs and be
> done with it once and for all.  If that's OK with you, then just tell me
> and I'll send you the relevant info,
>
>
>         Stefan
>

[Message part 2 (text/html, inline)]

Message #29 received at 16035 <at> debbugs.gnu.org (full text, mbox):

From: Juri Linkov <juri <at> jurta.org>
To: Stefan Monnier <monnier <at> iro.umontreal.ca>
Cc: 16035 <at> debbugs.gnu.org, Anders Lindgren <andlind <at> gmail.com>
Subject: Re: bug#16035: 24.3.50;
 Custom isearch broken on trunk (e.g. change-log-mode)
Date: Thu, 05 Dec 2013 03:20:03 +0200

>> I tried you solution and it works perfectly! Just make sure to save the
>> keymap after the call to "(run-hooks 'isearch-mode-hook)", as this is where
>> packages like "folding" installs its keymap.
>
> Great to hear.  Juri, can you do that for me?

I tested this patch, and it seems to fix the second case reported
by Anders.  If the patch is correct then I could install it.

>> While looking around the isearch code, I came up with a theory why
>> multi-buffer search in change-log-mode no longer works. isearch adds its
>> hook to the LOCAL pre-command-hook. As change-log-mode search change buffer
>> and the hook is not installed in the new buffer, the user can't exit
>> isearch.
>
> Sounds right, as well.  Juri, can you take care of that while you're at it?

I already installed the fix for this case a day ago in revno:115368
Anders, can you try this fix? Only the first case that you reported
has been fixed and installed, and the second case fixed by this patch
will be installed soon too.

=== modified file 'lisp/isearch.el'
--- lisp/isearch.el	2013-12-04 00:12:02 +0000
+++ lisp/isearch.el	2013-12-05 01:18:56 +0000
@@ -637,6 +637,8 @@ (defvar isearch-input-method-function ni
 ;; isearch is invoked.
 (defvar isearch-input-method-local-p nil)
 
+(defvar isearch-saved-overriding-local-map nil)
+
 ;; Minor-mode-alist changes - kind of redundant with the
 ;; echo area, but if isearching in multiple windows, it can be useful.
 
@@ -904,6 +906,7 @@ (defun isearch-mode (forward &optional r
 
   (setq overriding-terminal-local-map isearch-mode-map)
   (run-hooks 'isearch-mode-hook)
+  (setq isearch-saved-overriding-local-map overriding-terminal-local-map)
 
   ;; Pushing the initial state used to be before running isearch-mode-hook,
   ;; but a hook might set `isearch-push-state-function' used in
@@ -2235,7 +2238,7 @@ (defun isearch-pre-command-hook ()
     (cond
      ;; Don't exit Isearch if we're in the middle of some
      ;; set-temporary-overlay-map thingy like universal-argument--mode.
-     ((not (eq overriding-terminal-local-map isearch-mode-map)))
+     ((not (eq overriding-terminal-local-map isearch-saved-overriding-local-map)))
      ;; Don't exit Isearch for isearch key bindings.
      ((commandp (lookup-key isearch-mode-map key nil)))
      ;; Optionally edit the search string instead of exiting.

Message #32 received at 16035 <at> debbugs.gnu.org (full text, mbox):

From: Stefan Monnier <monnier <at> iro.umontreal.ca>
To: Juri Linkov <juri <at> jurta.org>
Cc: 16035 <at> debbugs.gnu.org, Anders Lindgren <andlind <at> gmail.com>
Subject: Re: bug#16035: 24.3.50;
 Custom isearch broken on trunk (e.g. change-log-mode)
Date: Wed, 04 Dec 2013 21:45:37 -0500

> +(defvar isearch-saved-overriding-local-map nil)

Oops, now that I see it, I think this one deserves a double-dash.

>    (setq overriding-terminal-local-map isearch-mode-map)
>    (run-hooks 'isearch-mode-hook)
> +  (setq isearch-saved-overriding-local-map overriding-terminal-local-map)

Please include a comment referring either to folding-isearch or to this bug.


        Stefan

Message #35 received at 16035 <at> debbugs.gnu.org (full text, mbox):

From: Anders Lindgren <andlind <at> gmail.com>
To: Juri Linkov <juri <at> jurta.org>
Cc: 16035 <at> debbugs.gnu.org, Stefan Monnier <monnier <at> iro.umontreal.ca>
Subject: Re: bug#16035: 24.3.50;
 Custom isearch broken on trunk (e.g. change-log-mode)
Date: Thu, 5 Dec 2013 16:33:17 +0100

[Message part 1 (text/plain, inline)]

Juri,

Both problems seems to be fixed!

(I tested the latest isearch.el, add added your patch, on Emacs bzr version
115300 (newer versions don't run on OS X due to some font-related issue).)

Thanks a lot, both of you!

    -- Anders


On Thu, Dec 5, 2013 at 2:20 AM, Juri Linkov <juri <at> jurta.org> wrote:

> >> I tried you solution and it works perfectly! Just make sure to save the
> >> keymap after the call to "(run-hooks 'isearch-mode-hook)", as this is
> where
> >> packages like "folding" installs its keymap.
> >
> > Great to hear.  Juri, can you do that for me?
>
> I tested this patch, and it seems to fix the second case reported
> by Anders.  If the patch is correct then I could install it.
>
> >> While looking around the isearch code, I came up with a theory why
> >> multi-buffer search in change-log-mode no longer works. isearch adds its
> >> hook to the LOCAL pre-command-hook. As change-log-mode search change
> buffer
> >> and the hook is not installed in the new buffer, the user can't exit
> >> isearch.
> >
> > Sounds right, as well.  Juri, can you take care of that while you're at
> it?
>
> I already installed the fix for this case a day ago in revno:115368
> Anders, can you try this fix? Only the first case that you reported
> has been fixed and installed, and the second case fixed by this patch
> will be installed soon too.
>
> === modified file 'lisp/isearch.el'
> --- lisp/isearch.el     2013-12-04 00:12:02 +0000
> +++ lisp/isearch.el     2013-12-05 01:18:56 +0000
> @@ -637,6 +637,8 @@ (defvar isearch-input-method-function ni
>  ;; isearch is invoked.
>  (defvar isearch-input-method-local-p nil)
>
> +(defvar isearch-saved-overriding-local-map nil)
> +
>  ;; Minor-mode-alist changes - kind of redundant with the
>  ;; echo area, but if isearching in multiple windows, it can be useful.
>
> @@ -904,6 +906,7 @@ (defun isearch-mode (forward &optional r
>
>    (setq overriding-terminal-local-map isearch-mode-map)
>    (run-hooks 'isearch-mode-hook)
> +  (setq isearch-saved-overriding-local-map overriding-terminal-local-map)
>
>    ;; Pushing the initial state used to be before running
> isearch-mode-hook,
>    ;; but a hook might set `isearch-push-state-function' used in
> @@ -2235,7 +2238,7 @@ (defun isearch-pre-command-hook ()
>      (cond
>       ;; Don't exit Isearch if we're in the middle of some
>       ;; set-temporary-overlay-map thingy like universal-argument--mode.
> -     ((not (eq overriding-terminal-local-map isearch-mode-map)))
> +     ((not (eq overriding-terminal-local-map
> isearch-saved-overriding-local-map)))
>       ;; Don't exit Isearch for isearch key bindings.
>       ((commandp (lookup-key isearch-mode-map key nil)))
>       ;; Optionally edit the search string instead of exiting.
>
>

[Message part 2 (text/html, inline)]

Message #38 received at 16035 <at> debbugs.gnu.org (full text, mbox):

From: Juri Linkov <juri <at> jurta.org>
To: Stefan Monnier <monnier <at> iro.umontreal.ca>
Cc: 16035 <at> debbugs.gnu.org, Anders Lindgren <andlind <at> gmail.com>
Subject: Re: bug#16035: 24.3.50;
 Custom isearch broken on trunk (e.g. change-log-mode)
Date: Fri, 06 Dec 2013 02:57:22 +0200

>> +(defvar isearch-saved-overriding-local-map nil)
>
> Oops, now that I see it, I think this one deserves a double-dash.
>
>>    (setq overriding-terminal-local-map isearch-mode-map)
>>    (run-hooks 'isearch-mode-hook)
>> +  (setq isearch-saved-overriding-local-map overriding-terminal-local-map)
>
> Please include a comment referring either to folding-isearch or to this bug.

The patch is installed now including these corrections.

Now I discovered another case that fails to move point
after exiting Isearch.  Running `multi-isearch-buffers'
and typing e.g. `C-a' exits Isearch but doesn't move point
to the beginning of the line.

The problem is that `multi-isearch-buffers' let-binds
`multi-isearch-buffer-list' to the list of buffers,
and invokes `isearch-forward' without NO-RECURSIVE-EDIT,
so dynamic binding of `multi-isearch-buffer-list' is
available for Isearch to get the next buffer to search.

But after exiting in `isearch-pre-command-hook',
`isearch-done' calls `exit-recursive-edit'
that prevents this-command from executing
(in this case `move-beginning-of-line' is not executed).

So `isearch-forward' in `multi-isearch-buffers'
should be invoked with non-nil NO-RECURSIVE-EDIT argument, and
then `multi-isearch-buffer-list' needs to be a global variable.

Exactly the same problem exists in `dired-isearch-filenames'
that let-binds `dired-isearch-filenames' and calls `isearch-forward'
without NO-RECURSIVE-EDIT.  And in `comint-history-isearch-backward'
that let-binds `comint-history-isearch'.  These variables
should be global.  Maybe this is not the right solution,
but I have no better ideas. 

Message #43 received at 16035-done <at> debbugs.gnu.org (full text, mbox):

From: Juri Linkov <juri <at> jurta.org>
To: Stefan Monnier <monnier <at> iro.umontreal.ca>
Cc: 16035-done <at> debbugs.gnu.org, Anders Lindgren <andlind <at> gmail.com>
Subject: Re: bug#16035: 24.3.50;
 Custom isearch broken on trunk (e.g. change-log-mode)
Date: Wed, 11 Dec 2013 02:12:07 +0200

> Now I discovered another case that fails to move point
> after exiting Isearch.  Running `multi-isearch-buffers'
> and typing e.g. `C-a' exits Isearch but doesn't move point
> to the beginning of the line.
>
> Exactly the same problem exists in `dired-isearch-filenames'
> that let-binds `dired-isearch-filenames' and calls `isearch-forward'
> without NO-RECURSIVE-EDIT.  And in `comint-history-isearch-backward'
> that let-binds `comint-history-isearch'.

I fixed this case as well.  Multi-buffer and multi-file search
can set the buffer list globally.  And comint.el and dired-aux.el
don't need the global value - they use the let-bound value
only in a setup hook.

Message #46 received at 16035-done <at> debbugs.gnu.org (full text, mbox):

From: Anders Lindgren <andlind <at> gmail.com>
To: Juri Linkov <juri <at> jurta.org>
Cc: 16035-done <at> debbugs.gnu.org, Stefan Monnier <monnier <at> iro.umontreal.ca>
Subject: Re: bug#16035: 24.3.50;
 Custom isearch broken on trunk (e.g. change-log-mode)
Date: Mon, 16 Dec 2013 09:13:42 +0100

[Message part 1 (text/plain, inline)]

Hi Juri and Stefan!

I just found another (minor) detail problem regarding the new isearch
system.

I sometimes use ESCAPE as the meta key. However, to exit an isearch search,
pressing the real meta key (Cmd, on my mac) and < work as intended.
However, the sequence ESCAPE < does not. Emacs simply responds that
"<escape> < is undefined".

Do you want me to file a new bug report, or does this mail suffice?

    -- Anders


On Wed, Dec 11, 2013 at 1:12 AM, Juri Linkov <juri <at> jurta.org> wrote:

> > Now I discovered another case that fails to move point
> > after exiting Isearch.  Running `multi-isearch-buffers'
> > and typing e.g. `C-a' exits Isearch but doesn't move point
> > to the beginning of the line.
> >
> > Exactly the same problem exists in `dired-isearch-filenames'
> > that let-binds `dired-isearch-filenames' and calls `isearch-forward'
> > without NO-RECURSIVE-EDIT.  And in `comint-history-isearch-backward'
> > that let-binds `comint-history-isearch'.
>
> I fixed this case as well.  Multi-buffer and multi-file search
> can set the buffer list globally.  And comint.el and dired-aux.el
> don't need the global value - they use the let-bound value
> only in a setup hook.
>

[Message part 2 (text/html, inline)]

Message #49 received at 16035 <at> debbugs.gnu.org (full text, mbox):

From: Juri Linkov <juri <at> jurta.org>
To: Anders Lindgren <andlind <at> gmail.com>
Cc: 16035 <at> debbugs.gnu.org, Stefan Monnier <monnier <at> iro.umontreal.ca>
Subject: Re: bug#16035: 24.3.50;
 Custom isearch broken on trunk (e.g. change-log-mode)
Date: Mon, 16 Dec 2013 22:32:40 +0200

> I just found another (minor) detail problem regarding the new isearch
> system.
>
> I sometimes use ESCAPE as the meta key. However, to exit an isearch search,
> pressing the real meta key (Cmd, on my mac) and < work as intended.
> However, the sequence ESCAPE < does not. Emacs simply responds that
> "<escape> < is undefined".

Hopefully, the following patch fixes this in a correct way.
At least, it works in all tests that I tried:

C-s ESC <       - exits isearch and goes to the top
C-s ESC c       - doesn't exit isearch and toggles case-sensitivity
C-s ESC ESC ESC - cancels isearch

=== modified file 'lisp/isearch.el'
--- lisp/isearch.el	2013-12-06 00:55:20 +0000
+++ lisp/isearch.el	2013-12-16 20:28:12 +0000
@@ -435,8 +435,7 @@ (defvar isearch-mode-map
     ;; would be simpler to disable the global keymap, and/or have a
     ;; default local key binding for any key not otherwise bound.
     (let ((meta-map (make-sparse-keymap)))
-      (define-key map (char-to-string meta-prefix-char) meta-map)
-      (define-key map [escape] meta-map))
+      (define-key map (char-to-string meta-prefix-char) meta-map))
 
     ;; Several non-printing chars change the searching behavior.
     (define-key map "\C-s" 'isearch-repeat-forward)
@@ -453,7 +452,6 @@ (defvar isearch-mode-map
     (or (= ?\e meta-prefix-char)
 	(error "Inconsistency in isearch.el"))
     (define-key map "\e\e\e" 'isearch-cancel)
-    (define-key map  [escape escape escape] 'isearch-cancel)
 
     (define-key map "\C-q" 'isearch-quote-char)
 

Message #52 received at 16035 <at> debbugs.gnu.org (full text, mbox):

From: Anders Lindgren <andlind <at> gmail.com>
To: Juri Linkov <juri <at> jurta.org>
Cc: 16035 <at> debbugs.gnu.org, Stefan Monnier <monnier <at> iro.umontreal.ca>
Subject: Re: bug#16035: 24.3.50;
 Custom isearch broken on trunk (e.g. change-log-mode)
Date: Tue, 17 Dec 2013 07:17:19 +0100

[Message part 1 (text/plain, inline)]

Hi!

I just tried the patch. It solved the problem I reported. However, when I
did some more testing I realized that there is still a difference between
ESC and the real meta key. ESC c toggles case-sensitivity. However, Meta-c
doesn't (it exits the search).

    -- Anders


On Mon, Dec 16, 2013 at 9:32 PM, Juri Linkov <juri <at> jurta.org> wrote:

> > I just found another (minor) detail problem regarding the new isearch
> > system.
> >
> > I sometimes use ESCAPE as the meta key. However, to exit an isearch
> search,
> > pressing the real meta key (Cmd, on my mac) and < work as intended.
> > However, the sequence ESCAPE < does not. Emacs simply responds that
> > "<escape> < is undefined".
>
> Hopefully, the following patch fixes this in a correct way.
> At least, it works in all tests that I tried:
>
> C-s ESC <       - exits isearch and goes to the top
> C-s ESC c       - doesn't exit isearch and toggles case-sensitivity
> C-s ESC ESC ESC - cancels isearch
>
> === modified file 'lisp/isearch.el'
> --- lisp/isearch.el     2013-12-06 00:55:20 +0000
> +++ lisp/isearch.el     2013-12-16 20:28:12 +0000
> @@ -435,8 +435,7 @@ (defvar isearch-mode-map
>      ;; would be simpler to disable the global keymap, and/or have a
>      ;; default local key binding for any key not otherwise bound.
>      (let ((meta-map (make-sparse-keymap)))
> -      (define-key map (char-to-string meta-prefix-char) meta-map)
> -      (define-key map [escape] meta-map))
> +      (define-key map (char-to-string meta-prefix-char) meta-map))
>
>      ;; Several non-printing chars change the searching behavior.
>      (define-key map "\C-s" 'isearch-repeat-forward)
> @@ -453,7 +452,6 @@ (defvar isearch-mode-map
>      (or (= ?\e meta-prefix-char)
>         (error "Inconsistency in isearch.el"))
>      (define-key map "\e\e\e" 'isearch-cancel)
> -    (define-key map  [escape escape escape] 'isearch-cancel)
>
>      (define-key map "\C-q" 'isearch-quote-char)
>
>

[Message part 2 (text/html, inline)]

Message #55 received at 16035 <at> debbugs.gnu.org (full text, mbox):

From: Juri Linkov <juri <at> jurta.org>
To: Anders Lindgren <andlind <at> gmail.com>
Cc: 16035 <at> debbugs.gnu.org, Stefan Monnier <monnier <at> iro.umontreal.ca>
Subject: Re: bug#16035: 24.3.50;
 Custom isearch broken on trunk (e.g. change-log-mode)
Date: Tue, 17 Dec 2013 21:38:12 +0200

> ESC c toggles case-sensitivity. However, Meta-c doesn't (it exits the search).

This is very strange.  I tried `C-s ESC c' and `C-s M-c' on X and in a tty,
and they both toggle case-sensitivity.

Message #58 received at 16035 <at> debbugs.gnu.org (full text, mbox):

From: Anders Lindgren <andlind <at> gmail.com>
To: Juri Linkov <juri <at> jurta.org>
Cc: 16035 <at> debbugs.gnu.org, Stefan Monnier <monnier <at> iro.umontreal.ca>
Subject: Re: bug#16035: 24.3.50;
 Custom isearch broken on trunk (e.g. change-log-mode)
Date: Wed, 18 Dec 2013 09:02:01 +0100

[Message part 1 (text/plain, inline)]

Hi!

Sorry, my mistake -- Normally I have Meta bound to the Cmd key, however,
the default binding is Alt, so I mixed when up when testing with -Q. In
other words, it appears to be working correctly!

    -- Anders


On Tue, Dec 17, 2013 at 8:38 PM, Juri Linkov <juri <at> jurta.org> wrote:

> > ESC c toggles case-sensitivity. However, Meta-c doesn't (it exits the
> search).
>
> This is very strange.  I tried `C-s ESC c' and `C-s M-c' on X and in a tty,
> and they both toggle case-sensitivity.
>

[Message part 2 (text/html, inline)]

Message #61 received at 16035 <at> debbugs.gnu.org (full text, mbox):

From: Juri Linkov <juri <at> jurta.org>
To: Anders Lindgren <andlind <at> gmail.com>
Cc: 16035 <at> debbugs.gnu.org, Stefan Monnier <monnier <at> iro.umontreal.ca>
Subject: Re: bug#16035: 24.3.50;
 Custom isearch broken on trunk (e.g. change-log-mode)
Date: Thu, 19 Dec 2013 01:22:58 +0200

> In other words, it appears to be working correctly!

Glad to hear this is working correctly.

Meanwhile, I found another problem: typing <backspace>
in Isearch while in the Gnus Summary buffer exits Isearch
and runs `gnus-summary-prev-page' because <backspace>
is bound explicitly in Gnus.  But in other buffers,
<backspace> in Isearch runs `isearch-delete-char'.
So it requires an explicit binding in `isearch-mode-map':

=== modified file 'lisp/isearch.el'
--- lisp/isearch.el	2013-12-16 20:32:15 +0000
+++ lisp/isearch.el	2013-12-18 23:12:31 +0000
@@ -446,6 +476,7 @@ (defvar isearch-mode-map
     (define-key map "\M-\C-s" 'isearch-repeat-forward)
     (define-key map "\M-\C-r" 'isearch-repeat-backward)
     (define-key map "\177" 'isearch-delete-char)
+    (define-key map [backspace] 'isearch-delete-char)
     (define-key map "\C-g" 'isearch-abort)
 

BTW, this also reminded that preferable key bindings
could be advertised with `advertised-binding',
so I'm going to install also this change too:

=== modified file 'lisp/isearch.el'
--- lisp/isearch.el	2013-12-16 20:32:15 +0000
+++ lisp/isearch.el	2013-12-18 23:12:31 +0000
@@ -500,6 +536,15 @@ (defvar isearch-mode-map
     (define-key map "\M-r" 'isearch-toggle-regexp)
     (define-key map "\M-e" 'isearch-edit-string)
 
+    (put 'isearch-toggle-case-fold :advertised-binding "\M-sc")
+    (put 'isearch-toggle-regexp    :advertised-binding "\M-sr")
+    (put 'isearch-edit-string      :advertised-binding "\M-se")
+
+    (define-key map "\M-se" 'isearch-edit-string)
     (define-key map "\M-sc" 'isearch-toggle-case-fold)
     (define-key map "\M-si" 'isearch-toggle-invisible)
     (define-key map "\M-sr" 'isearch-toggle-regexp)

Message #64 received at 16035 <at> debbugs.gnu.org (full text, mbox):

From: Stefan Monnier <monnier <at> IRO.UMontreal.CA>
To: Juri Linkov <juri <at> jurta.org>
Cc: 16035 <at> debbugs.gnu.org, Anders Lindgren <andlind <at> gmail.com>
Subject: Re: bug#16035: 24.3.50;
 Custom isearch broken on trunk (e.g. change-log-mode)
Date: Thu, 19 Dec 2013 08:35:56 -0500

> Meanwhile, I found another problem: typing <backspace>
> in Isearch while in the Gnus Summary buffer exits Isearch
> and runs `gnus-summary-prev-page' because <backspace>
> is bound explicitly in Gnus.

Looks like a bug in Gnus, since DEL is not bound.  I.e. Gnus's binding
should be to DEL and not to `backspace'.


        Stefan

Message #67 received at 16035 <at> debbugs.gnu.org (full text, mbox):

From: Juri Linkov <juri <at> jurta.org>
To: Stefan Monnier <monnier <at> IRO.UMontreal.CA>
Cc: 16035 <at> debbugs.gnu.org, Anders Lindgren <andlind <at> gmail.com>
Subject: Re: bug#16035: 24.3.50;
 Custom isearch broken on trunk (e.g. change-log-mode)
Date: Thu, 19 Dec 2013 22:39:12 +0200

>> Meanwhile, I found another problem: typing <backspace>
>> in Isearch while in the Gnus Summary buffer exits Isearch
>> and runs `gnus-summary-prev-page' because <backspace>
>> is bound explicitly in Gnus.
>
> Looks like a bug in Gnus, since DEL is not bound.  I.e. Gnus's binding
> should be to DEL and not to `backspace'.

Then I'll install this patch.  It was necessary also to remove
[backspace] from `gnus-suppress-keymap' because without the last hunk
typing `backspace' in an Article buffer called `gnus-summary-prev-page',
but not `gnus-article-goto-prev-page' as it should.

=== modified file 'lisp/gnus/gnus-art.el'
--- lisp/gnus/gnus-art.el	2013-12-14 21:43:01 +0000
+++ lisp/gnus/gnus-art.el	2013-12-19 20:33:12 +0000
@@ -4396,7 +4396,6 @@ (gnus-define-keys gnus-article-mode-map
   [?\S-\ ] gnus-article-goto-prev-page
   "\177" gnus-article-goto-prev-page
   [delete] gnus-article-goto-prev-page
-  [backspace] gnus-article-goto-prev-page
   "\C-c^" gnus-article-refer-article
   "h" gnus-article-show-summary
   "s" gnus-article-show-summary

=== modified file 'lisp/gnus/gnus-group.el'
--- lisp/gnus/gnus-group.el	2013-12-14 21:43:01 +0000
+++ lisp/gnus/gnus-group.el	2013-12-19 20:33:12 +0000
@@ -571,7 +571,6 @@ (gnus-define-keys gnus-group-mode-map
   "p" gnus-group-prev-unread-group
   "\177" gnus-group-prev-unread-group
   [delete] gnus-group-prev-unread-group
-  [backspace] gnus-group-prev-unread-group
   "N" gnus-group-next-group
   "P" gnus-group-prev-group
   "\M-n" gnus-group-next-unread-group-same-level

=== modified file 'lisp/gnus/gnus-sum.el'
--- lisp/gnus/gnus-sum.el	2013-09-17 17:22:32 +0000
+++ lisp/gnus/gnus-sum.el	2013-12-19 20:33:12 +0000
@@ -1850,7 +1850,6 @@ (gnus-define-keys gnus-summary-mode-map
   [?\S-\ ] gnus-summary-prev-page
   "\177" gnus-summary-prev-page
   [delete] gnus-summary-prev-page
-  [backspace] gnus-summary-prev-page
   "\r" gnus-summary-scroll-up
   "\M-\r" gnus-summary-scroll-down
   "n" gnus-summary-next-unread-article
@@ -2222,7 +2221,6 @@ (gnus-define-keys (gnus-summary-backend-
   "\M-\C-e" gnus-summary-expire-articles-now
   "\177" gnus-summary-delete-article
   [delete] gnus-summary-delete-article
-  [backspace] gnus-summary-delete-article
   "m" gnus-summary-move-article
   "r" gnus-summary-respool-article
   "w" gnus-summary-edit-article

=== modified file 'lisp/gnus/gnus.el'
--- lisp/gnus/gnus.el	2013-08-13 07:18:50 +0000
+++ lisp/gnus/gnus.el	2013-12-19 20:33:52 +0000
@@ -3035,7 +3035,7 @@ (defcustom gnus-summary-line-format "%U%
 
 (defun gnus-suppress-keymap (keymap)
   (suppress-keymap keymap)
-  (let ((keys `([backspace] [delete] "\177" "\M-u"))) ;gnus-mouse-2
+  (let ((keys `([delete] "\177" "\M-u"))) ;gnus-mouse-2
     (while keys
       (define-key keymap (pop keys) 'undefined))))
 

Previous Next

GNU bug tracking system
Copyright (C) 1999 Darren O. Benham, 1997,2003 nCipher Corporation Ltd, 1994-97 Ian Jackson.