* bug#16035: 24.3.50; Custom isearch broken on trunk (e.g. change-log-mode)
@ 2013-12-03 9:38 Anders Lindgren
2013-12-04 0:14 ` Juri Linkov
2013-12-04 0:18 ` Juri Linkov
0 siblings, 2 replies; 21+ messages in thread
From: Anders Lindgren @ 2013-12-03 9:38 UTC (permalink / raw)
To: 16035
[-- Attachment #1: Type: text/plain, Size: 4308 bytes --]
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@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@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@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)
[-- Attachment #2: Type: text/html, Size: 6218 bytes --]
^ permalink raw reply [flat|nested] 21+ messages in thread
* bug#16035: 24.3.50; Custom isearch broken on trunk (e.g. change-log-mode)
2013-12-03 9:38 bug#16035: 24.3.50; Custom isearch broken on trunk (e.g. change-log-mode) Anders Lindgren
@ 2013-12-04 0:14 ` Juri Linkov
2013-12-04 0:18 ` Juri Linkov
1 sibling, 0 replies; 21+ messages in thread
From: Juri Linkov @ 2013-12-04 0:14 UTC (permalink / raw)
To: Anders Lindgren; +Cc: 16035
> 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.
^ permalink raw reply [flat|nested] 21+ messages in thread
* bug#16035: 24.3.50; Custom isearch broken on trunk (e.g. change-log-mode)
2013-12-03 9:38 bug#16035: 24.3.50; Custom isearch broken on trunk (e.g. change-log-mode) Anders Lindgren
2013-12-04 0:14 ` Juri Linkov
@ 2013-12-04 0:18 ` Juri Linkov
2013-12-04 0:37 ` Juri Linkov
2013-12-04 4:31 ` Stefan Monnier
1 sibling, 2 replies; 21+ messages in thread
From: Juri Linkov @ 2013-12-04 0:18 UTC (permalink / raw)
To: Anders Lindgren; +Cc: 16035
> 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.
^ permalink raw reply [flat|nested] 21+ messages in thread
* bug#16035: 24.3.50; Custom isearch broken on trunk (e.g. change-log-mode)
2013-12-04 0:18 ` Juri Linkov
@ 2013-12-04 0:37 ` Juri Linkov
2013-12-04 4:31 ` Stefan Monnier
1 sibling, 0 replies; 21+ messages in thread
From: Juri Linkov @ 2013-12-04 0:37 UTC (permalink / raw)
To: Anders Lindgren; +Cc: 16035
> ((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.
^ permalink raw reply [flat|nested] 21+ messages in thread
* bug#16035: 24.3.50; Custom isearch broken on trunk (e.g. change-log-mode)
2013-12-04 0:18 ` Juri Linkov
2013-12-04 0:37 ` Juri Linkov
@ 2013-12-04 4:31 ` Stefan Monnier
2013-12-04 11:48 ` Anders Lindgren
1 sibling, 1 reply; 21+ messages in thread
From: Stefan Monnier @ 2013-12-04 4:31 UTC (permalink / raw)
To: Juri Linkov; +Cc: 16035, Anders Lindgren
> 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
^ permalink raw reply [flat|nested] 21+ messages in thread
* bug#16035: 24.3.50; Custom isearch broken on trunk (e.g. change-log-mode)
2013-12-04 4:31 ` Stefan Monnier
@ 2013-12-04 11:48 ` Anders Lindgren
2013-12-04 17:57 ` Stefan Monnier
0 siblings, 1 reply; 21+ messages in thread
From: Anders Lindgren @ 2013-12-04 11:48 UTC (permalink / raw)
To: Stefan Monnier; +Cc: 16035
[-- Attachment #1: Type: text/plain, Size: 1430 bytes --]
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@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
>
[-- Attachment #2: Type: text/html, Size: 2031 bytes --]
^ permalink raw reply [flat|nested] 21+ messages in thread
* bug#16035: 24.3.50; Custom isearch broken on trunk (e.g. change-log-mode)
2013-12-04 11:48 ` Anders Lindgren
@ 2013-12-04 17:57 ` Stefan Monnier
2013-12-04 21:49 ` Anders Lindgren
2013-12-05 1:20 ` Juri Linkov
0 siblings, 2 replies; 21+ messages in thread
From: Stefan Monnier @ 2013-12-04 17:57 UTC (permalink / raw)
To: Anders Lindgren; +Cc: 16035
> 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
^ permalink raw reply [flat|nested] 21+ messages in thread
* bug#16035: 24.3.50; Custom isearch broken on trunk (e.g. change-log-mode)
2013-12-04 17:57 ` Stefan Monnier
@ 2013-12-04 21:49 ` Anders Lindgren
2013-12-05 1:20 ` Juri Linkov
1 sibling, 0 replies; 21+ messages in thread
From: Anders Lindgren @ 2013-12-04 21:49 UTC (permalink / raw)
To: Stefan Monnier; +Cc: 16035
[-- Attachment #1: Type: text/plain, Size: 1816 bytes --]
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@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
>
[-- Attachment #2: Type: text/html, Size: 2399 bytes --]
^ permalink raw reply [flat|nested] 21+ messages in thread
* bug#16035: 24.3.50; Custom isearch broken on trunk (e.g. change-log-mode)
2013-12-04 17:57 ` Stefan Monnier
2013-12-04 21:49 ` Anders Lindgren
@ 2013-12-05 1:20 ` Juri Linkov
2013-12-05 2:45 ` Stefan Monnier
2013-12-05 15:33 ` Anders Lindgren
1 sibling, 2 replies; 21+ messages in thread
From: Juri Linkov @ 2013-12-05 1:20 UTC (permalink / raw)
To: Stefan Monnier; +Cc: 16035, Anders Lindgren
>> 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.
^ permalink raw reply [flat|nested] 21+ messages in thread
* bug#16035: 24.3.50; Custom isearch broken on trunk (e.g. change-log-mode)
2013-12-05 1:20 ` Juri Linkov
@ 2013-12-05 2:45 ` Stefan Monnier
2013-12-06 0:57 ` Juri Linkov
2013-12-05 15:33 ` Anders Lindgren
1 sibling, 1 reply; 21+ messages in thread
From: Stefan Monnier @ 2013-12-05 2:45 UTC (permalink / raw)
To: Juri Linkov; +Cc: 16035, Anders Lindgren
> +(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
^ permalink raw reply [flat|nested] 21+ messages in thread
* bug#16035: 24.3.50; Custom isearch broken on trunk (e.g. change-log-mode)
2013-12-05 1:20 ` Juri Linkov
2013-12-05 2:45 ` Stefan Monnier
@ 2013-12-05 15:33 ` Anders Lindgren
1 sibling, 0 replies; 21+ messages in thread
From: Anders Lindgren @ 2013-12-05 15:33 UTC (permalink / raw)
To: Juri Linkov; +Cc: 16035
[-- Attachment #1: Type: text/plain, Size: 2762 bytes --]
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@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.
>
>
[-- Attachment #2: Type: text/html, Size: 3483 bytes --]
^ permalink raw reply [flat|nested] 21+ messages in thread
* bug#16035: 24.3.50; Custom isearch broken on trunk (e.g. change-log-mode)
2013-12-05 2:45 ` Stefan Monnier
@ 2013-12-06 0:57 ` Juri Linkov
2013-12-11 0:12 ` Juri Linkov
0 siblings, 1 reply; 21+ messages in thread
From: Juri Linkov @ 2013-12-06 0:57 UTC (permalink / raw)
To: Stefan Monnier; +Cc: 16035, Anders Lindgren
>> +(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.
^ permalink raw reply [flat|nested] 21+ messages in thread
* bug#16035: 24.3.50; Custom isearch broken on trunk (e.g. change-log-mode)
2013-12-06 0:57 ` Juri Linkov
@ 2013-12-11 0:12 ` Juri Linkov
2013-12-16 8:13 ` Anders Lindgren
0 siblings, 1 reply; 21+ messages in thread
From: Juri Linkov @ 2013-12-11 0:12 UTC (permalink / raw)
To: Stefan Monnier; +Cc: 16035-done, Anders Lindgren
> 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.
^ permalink raw reply [flat|nested] 21+ messages in thread
* bug#16035: 24.3.50; Custom isearch broken on trunk (e.g. change-log-mode)
2013-12-11 0:12 ` Juri Linkov
@ 2013-12-16 8:13 ` Anders Lindgren
2013-12-16 20:32 ` Juri Linkov
0 siblings, 1 reply; 21+ messages in thread
From: Anders Lindgren @ 2013-12-16 8:13 UTC (permalink / raw)
To: Juri Linkov; +Cc: 16035-done
[-- Attachment #1: Type: text/plain, Size: 1193 bytes --]
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@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.
>
[-- Attachment #2: Type: text/html, Size: 1758 bytes --]
^ permalink raw reply [flat|nested] 21+ messages in thread
* bug#16035: 24.3.50; Custom isearch broken on trunk (e.g. change-log-mode)
2013-12-16 8:13 ` Anders Lindgren
@ 2013-12-16 20:32 ` Juri Linkov
2013-12-17 6:17 ` Anders Lindgren
0 siblings, 1 reply; 21+ messages in thread
From: Juri Linkov @ 2013-12-16 20:32 UTC (permalink / raw)
To: Anders Lindgren; +Cc: 16035
> 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)
^ permalink raw reply [flat|nested] 21+ messages in thread
* bug#16035: 24.3.50; Custom isearch broken on trunk (e.g. change-log-mode)
2013-12-16 20:32 ` Juri Linkov
@ 2013-12-17 6:17 ` Anders Lindgren
2013-12-17 19:38 ` Juri Linkov
0 siblings, 1 reply; 21+ messages in thread
From: Anders Lindgren @ 2013-12-17 6:17 UTC (permalink / raw)
To: Juri Linkov; +Cc: 16035
[-- Attachment #1: Type: text/plain, Size: 1955 bytes --]
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@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)
>
>
[-- Attachment #2: Type: text/html, Size: 2552 bytes --]
^ permalink raw reply [flat|nested] 21+ messages in thread
* bug#16035: 24.3.50; Custom isearch broken on trunk (e.g. change-log-mode)
2013-12-17 6:17 ` Anders Lindgren
@ 2013-12-17 19:38 ` Juri Linkov
2013-12-18 8:02 ` Anders Lindgren
0 siblings, 1 reply; 21+ messages in thread
From: Juri Linkov @ 2013-12-17 19:38 UTC (permalink / raw)
To: Anders Lindgren; +Cc: 16035
> 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.
^ permalink raw reply [flat|nested] 21+ messages in thread
* bug#16035: 24.3.50; Custom isearch broken on trunk (e.g. change-log-mode)
2013-12-17 19:38 ` Juri Linkov
@ 2013-12-18 8:02 ` Anders Lindgren
2013-12-18 23:22 ` Juri Linkov
0 siblings, 1 reply; 21+ messages in thread
From: Anders Lindgren @ 2013-12-18 8:02 UTC (permalink / raw)
To: Juri Linkov; +Cc: 16035
[-- Attachment #1: Type: text/plain, Size: 494 bytes --]
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@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.
>
[-- Attachment #2: Type: text/html, Size: 884 bytes --]
^ permalink raw reply [flat|nested] 21+ messages in thread
* bug#16035: 24.3.50; Custom isearch broken on trunk (e.g. change-log-mode)
2013-12-18 8:02 ` Anders Lindgren
@ 2013-12-18 23:22 ` Juri Linkov
2013-12-19 13:35 ` Stefan Monnier
0 siblings, 1 reply; 21+ messages in thread
From: Juri Linkov @ 2013-12-18 23:22 UTC (permalink / raw)
To: Anders Lindgren; +Cc: 16035
> 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)
^ permalink raw reply [flat|nested] 21+ messages in thread
* bug#16035: 24.3.50; Custom isearch broken on trunk (e.g. change-log-mode)
2013-12-18 23:22 ` Juri Linkov
@ 2013-12-19 13:35 ` Stefan Monnier
2013-12-19 20:39 ` Juri Linkov
0 siblings, 1 reply; 21+ messages in thread
From: Stefan Monnier @ 2013-12-19 13:35 UTC (permalink / raw)
To: Juri Linkov; +Cc: 16035, Anders Lindgren
> 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
^ permalink raw reply [flat|nested] 21+ messages in thread
* bug#16035: 24.3.50; Custom isearch broken on trunk (e.g. change-log-mode)
2013-12-19 13:35 ` Stefan Monnier
@ 2013-12-19 20:39 ` Juri Linkov
0 siblings, 0 replies; 21+ messages in thread
From: Juri Linkov @ 2013-12-19 20:39 UTC (permalink / raw)
To: Stefan Monnier; +Cc: 16035, Anders Lindgren
>> 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))))
^ permalink raw reply [flat|nested] 21+ messages in thread
end of thread, other threads:[~2013-12-19 20:39 UTC | newest]
Thread overview: 21+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2013-12-03 9:38 bug#16035: 24.3.50; Custom isearch broken on trunk (e.g. change-log-mode) Anders Lindgren
2013-12-04 0:14 ` Juri Linkov
2013-12-04 0:18 ` Juri Linkov
2013-12-04 0:37 ` Juri Linkov
2013-12-04 4:31 ` Stefan Monnier
2013-12-04 11:48 ` Anders Lindgren
2013-12-04 17:57 ` Stefan Monnier
2013-12-04 21:49 ` Anders Lindgren
2013-12-05 1:20 ` Juri Linkov
2013-12-05 2:45 ` Stefan Monnier
2013-12-06 0:57 ` Juri Linkov
2013-12-11 0:12 ` Juri Linkov
2013-12-16 8:13 ` Anders Lindgren
2013-12-16 20:32 ` Juri Linkov
2013-12-17 6:17 ` Anders Lindgren
2013-12-17 19:38 ` Juri Linkov
2013-12-18 8:02 ` Anders Lindgren
2013-12-18 23:22 ` Juri Linkov
2013-12-19 13:35 ` Stefan Monnier
2013-12-19 20:39 ` Juri Linkov
2013-12-05 15:33 ` Anders Lindgren
Code repositories for project(s) associated with this public inbox
https://git.savannah.gnu.org/cgit/emacs.git
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).