GNU bug report logs - #72229
(setq overriding-terminal-local-map nil) in isearch-done

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 72229 in the body.
You can then email your comments to 72229 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: Michael Heerdegen <michael_heerdegen <at> web.de>
To: bug-gnu-emacs <at> gnu.org
Subject: (setq overriding-terminal-local-map nil) in isearch-done
Date: Sun, 21 Jul 2024 16:49:21 +0200

Hello,

exiting isearch always explicitly sets `overriding-terminal-local-map'
to nil.  This will interfere with any other users of
`overriding-terminal-local-map', including any active transient maps.

Wouldn't a less radical means suffice?  If isearch really must _set_,
i.e., completely override that variable, why not restore the original
value?


TIA,

Michael.

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

From: Michael Heerdegen <michael_heerdegen <at> web.de>
To: Michael Heerdegen via "Bug reports for GNU Emacs, the Swiss army knife
 of text editors" <bug-gnu-emacs <at> gnu.org>
Cc: 72229 <at> debbugs.gnu.org
Subject: Re: bug#72229: (setq overriding-terminal-local-map nil) in
 isearch-done
Date: Mon, 22 Jul 2024 14:49:34 +0200

Michael Heerdegen via "Bug reports for GNU Emacs, the Swiss army knife
of text editors" <bug-gnu-emacs <at> gnu.org> writes:

> exiting isearch always explicitly sets `overriding-terminal-local-map'
> to nil.  This will interfere with any other users of
> `overriding-terminal-local-map', including any active transient maps.
>
> Wouldn't a less radical means suffice?  If isearch really must _set_,
> i.e., completely override that variable, why not restore the original
> value?

Juri, what's your opinion on this?


A little more forward looking:

In such situations I often get this thought: if the variable was
replaced with a function accepting zero arguments, then we could use
`add-function' and `remove-function' to control the return value of the
"binding".

I know the advice mechanism has the reputation of only being suitable
for end user customization/hacks, but in cases like this one we could
make the modification of the value more explicit and controllable.
Dealing with interferences would be forced to be taken into account more
directly , and we would get some useful mechanisms like priorities or
looking at the context out of the box.

We could also invent some even better mechanism, maybe.  But in this
case binding variables doesn't look like an optimal approach at least.


Michael.

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

From: Juri Linkov <juri <at> linkov.net>
To: Michael Heerdegen <michael_heerdegen <at> web.de>
Cc: 72229 <at> debbugs.gnu.org
Subject: Re: bug#72229: (setq overriding-terminal-local-map nil) in
 isearch-done
Date: Tue, 23 Jul 2024 09:32:21 +0300

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

> Michael Heerdegen via "Bug reports for GNU Emacs, the Swiss army knife
> of text editors" <bug-gnu-emacs <at> gnu.org> writes:
>
>> exiting isearch always explicitly sets `overriding-terminal-local-map'
>> to nil.  This will interfere with any other users of
>> `overriding-terminal-local-map', including any active transient maps.
>>
>> Wouldn't a less radical means suffice?  If isearch really must _set_,
>> i.e., completely override that variable, why not restore the original
>> value?
>
> Juri, what's your opinion on this?

Indeed, you are right, `isearch-done' should restore the original value.
The existing variable `isearch--saved-overriding-local-map' can't be used,
so a similar variable should be added like in this patch:

[isearch--saved-local-map.patch (text/x-diff, inline)]

diff --git a/lisp/isearch.el b/lisp/isearch.el
index dc9edf267f2..697dcdbb3d8 100644
--- a/lisp/isearch.el
+++ b/lisp/isearch.el
@@ -972,6 +972,7 @@ isearch-hidden
 ;; The value of input-method-function when isearch is invoked.
 (defvar isearch-input-method-function nil)
 
+(defvar isearch--saved-local-map nil)
 (defvar isearch--saved-overriding-local-map nil)
 
 ;; Minor-mode-alist changes - kind of redundant with the
@@ -1321,6 +1322,7 @@ isearch-mode
   (setq	isearch-mode " Isearch")  ;; forward? regexp?
   (force-mode-line-update)
 
+  (setq isearch--saved-local-map overriding-terminal-local-map)
   (setq overriding-terminal-local-map isearch-mode-map)
   (run-hooks 'isearch-mode-hook)
   ;; Remember the initial map possibly modified
@@ -1439,10 +1444,12 @@ isearch-update
 
 (defun isearch-done (&optional nopush edit)
   "Exit Isearch mode.
+Called by all commands that terminate isearch-mode.
 For successful search, pass no args.
 For a failing search, NOPUSH is t.
 For going to the minibuffer to edit the search string,
-NOPUSH is t and EDIT is t."
+NOPUSH is t and EDIT is t.
+If NOPUSH is non-nil, we don't push the string on the search ring."
 
   (when isearch-resume-in-command-history
     (add-to-history 'command-history
@@ -1460,9 +1467,7 @@ isearch-done
       (setq isearch--current-buffer nil)
       (setq cursor-sensor-inhibit (delq 'isearch cursor-sensor-inhibit))))
 
-  ;; Called by all commands that terminate isearch-mode.
-  ;; If NOPUSH is non-nil, we don't push the string on the search ring.
-  (setq overriding-terminal-local-map nil)
+  (setq overriding-terminal-local-map isearch--saved-local-map)
   ;; (setq pre-command-hook isearch-old-pre-command-hook) ; for lemacs
   (setq minibuffer-message-timeout isearch-original-minibuffer-message-timeout)
   (isearch-dehighlight)
@@ -2676,7 +2681,7 @@ isearch-mouse-2
 is bound to outside of Isearch."
   (interactive "e")
   (let ((w (posn-window (event-start click)))
-        (binding (let ((overriding-terminal-local-map nil)
+        (binding (let ((overriding-terminal-local-map isearch--saved-local-map)
                        ;; Key search depends on mode (bug#47755)
                        (isearch-mode nil))
                    (key-binding (this-command-keys-vector) t))))

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

> A little more forward looking:
>
> In such situations I often get this thought: if the variable was
> replaced with a function accepting zero arguments, then we could use
> `add-function' and `remove-function' to control the return value of the
> "binding".
>
> I know the advice mechanism has the reputation of only being suitable
> for end user customization/hacks, but in cases like this one we could
> make the modification of the value more explicit and controllable.
> Dealing with interferences would be forced to be taken into account more
> directly , and we would get some useful mechanisms like priorities or
> looking at the context out of the box.
>
> We could also invent some even better mechanism, maybe.  But in this
> case binding variables doesn't look like an optimal approach at least.

This mechanism looks like a variable watcher enabled by `add-variable-watcher'.
So you could add a watcher that conditionally controls variable modifications.

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

From: Eli Zaretskii <eliz <at> gnu.org>
To: Juri Linkov <juri <at> linkov.net>
Cc: michael_heerdegen <at> web.de, 72229 <at> debbugs.gnu.org
Subject: Re: bug#72229: (setq overriding-terminal-local-map nil) in
 isearch-done
Date: Tue, 23 Jul 2024 14:29:25 +0300

> Cc: 72229 <at> debbugs.gnu.org
> From: Juri Linkov <juri <at> linkov.net>
> Date: Tue, 23 Jul 2024 09:32:21 +0300
> 
> > Michael Heerdegen via "Bug reports for GNU Emacs, the Swiss army knife
> > of text editors" <bug-gnu-emacs <at> gnu.org> writes:
> >
> >> exiting isearch always explicitly sets `overriding-terminal-local-map'
> >> to nil.  This will interfere with any other users of
> >> `overriding-terminal-local-map', including any active transient maps.
> >>
> >> Wouldn't a less radical means suffice?  If isearch really must _set_,
> >> i.e., completely override that variable, why not restore the original
> >> value?
> >
> > Juri, what's your opinion on this?
> 
> Indeed, you are right, `isearch-done' should restore the original value.
> The existing variable `isearch--saved-overriding-local-map' can't be used,
> so a similar variable should be added like in this patch:

Thanks.  If this is deemed the right solution, please install on
master, not on emacs-30 (as we have lived with this issue since 1995).

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

From: Michael Heerdegen <michael_heerdegen <at> web.de>
To: Juri Linkov <juri <at> linkov.net>
Cc: 72229 <at> debbugs.gnu.org
Subject: Re: bug#72229: (setq overriding-terminal-local-map nil) in
 isearch-done
Date: Tue, 23 Jul 2024 18:05:31 +0200

Juri Linkov <juri <at> linkov.net> writes:

> Indeed, you are right, `isearch-done' should restore the original value.
> The existing variable `isearch--saved-overriding-local-map' can't be used,
> so a similar variable should be added like in this patch:

LGTM for master - thank you.


> This mechanism looks like a variable watcher enabled by
> `add-variable-watcher'.
> So you could add a watcher that conditionally controls variable
> modifications.

I don't think variable watchers are very helpful here.  They don't solve
the underlying problem: potentially infinite variables of the same name
can exist, shadowing each other, with values partly sharing structures.


Using variable watchers I can see whether a variable value gets shadowed
or unassigned using a set operation - but I can't know whether the
previous value still exists, as binding of some other variable, and if
it will be stored back into the variable.  Nor do I have access to old
bindings and their values until the program assigns it back to the
variable.

I saw that in Bug#70938.  Manipulations of a variable can interfere in
annoying ways.

Functions are different.  You have only one dynamic binding (unless in
the rare case of using `cl-letf', which is extremely rare).  And you
always have access to it to undo any prior modification.


Michael.

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

From: Juri Linkov <juri <at> linkov.net>
To: Michael Heerdegen <michael_heerdegen <at> web.de>
Cc: 72229 <at> debbugs.gnu.org
Subject: Re: bug#72229: (setq overriding-terminal-local-map nil) in
 isearch-done
Date: Tue, 23 Jul 2024 20:46:33 +0300

>> This mechanism looks like a variable watcher enabled by
>> `add-variable-watcher'.
>> So you could add a watcher that conditionally controls variable
>> modifications.
>
> I don't think variable watchers are very helpful here.  They don't solve
> the underlying problem: potentially infinite variables of the same name
> can exist, shadowing each other, with values partly sharing structures.
>
> Using variable watchers I can see whether a variable value gets shadowed
> or unassigned using a set operation - but I can't know whether the
> previous value still exists, as binding of some other variable, and if
> it will be stored back into the variable.  Nor do I have access to old
> bindings and their values until the program assigns it back to the
> variable.
>
> I saw that in Bug#70938.  Manipulations of a variable can interfere in
> annoying ways.
>
> Functions are different.  You have only one dynamic binding (unless in
> the rare case of using `cl-letf', which is extremely rare).  And you
> always have access to it to undo any prior modification.

I remember Stefan M suggested to use function variables as much as possible
such as (defvar isearch-filter-predicate #'isearch-filter-visible), then
it's easy to add more filters with add-function.

But I'm not sure is it possible to do the same with a variable
getter function?  I mean that instead of (funcall isearch-filter-predicate)
such a getter function could be called (get-value isearch-filter-predicate)
to access the variable value.

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

From: Juri Linkov <juri <at> linkov.net>
To: Eli Zaretskii <eliz <at> gnu.org>
Cc: michael_heerdegen <at> web.de, 72229 <at> debbugs.gnu.org
Subject: Re: bug#72229: (setq overriding-terminal-local-map nil) in
 isearch-done
Date: Tue, 23 Jul 2024 20:54:19 +0300

close 72229 31.0.50
thanks

>> Indeed, you are right, `isearch-done' should restore the original value.
>> The existing variable `isearch--saved-overriding-local-map' can't be used,
>> so a similar variable should be added like in this patch:
>
> Thanks.  If this is deemed the right solution, please install on
> master, not on emacs-30 (as we have lived with this issue since 1995).

Now installed on master.  I need to dust off an old patch
that obsoletes overriding-terminal-local-map in isearch
by enabling buffer-local isearch-mode.  I guess it will go
to master as well.

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

From: Michael Heerdegen <michael_heerdegen <at> web.de>
To: Juri Linkov <juri <at> linkov.net>
Cc: 72229 <at> debbugs.gnu.org
Subject: Re: bug#72229: (setq overriding-terminal-local-map nil) in
 isearch-done
Date: Wed, 24 Jul 2024 18:42:31 +0200

Juri Linkov <juri <at> linkov.net> writes:

> But I'm not sure is it possible to do the same with a variable
> getter function?

I think it would be unavoidable to change all code places where the
variable is referenced, including those in C, to call the function value
stored in the variable instead of referencing the variable binding.


Michael.

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

From: Drew Adams <drew.adams <at> oracle.com>
To: Michael Heerdegen <michael_heerdegen <at> web.de>, Juri Linkov <juri <at> linkov.net>
Cc: "72229 <at> debbugs.gnu.org" <72229 <at> debbugs.gnu.org>
Subject: RE: [External] : bug#72229: (setq overriding-terminal-local-map nil)
 in isearch-done
Date: Wed, 24 Jul 2024 17:27:06 +0000

> > But I'm not sure is it possible to do the same with a variable
> > getter function?
> 
> I think it would be unavoidable to change all code places where the
> variable is referenced, including those in C, to call the function value
> stored in the variable instead of referencing the variable binding.

I'm not knowledgeable in this area, and I'm not
sure what you're discussing (I think it's the
possibility of substituting a function for a var),
but would defining a symbol macro help?

We at least have `cl-symbol-macrolet', even if we
don't (yet) have `define-symbol-macro'.

https://lisp-docs.github.io/cl-language-reference/chap-3/d-i-dictionary/define-symbol-macro_macro

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

From: Michael Heerdegen <michael_heerdegen <at> web.de>
To: Drew Adams <drew.adams <at> oracle.com>
Cc: "72229 <at> debbugs.gnu.org" <72229 <at> debbugs.gnu.org>,
 Juri Linkov <juri <at> linkov.net>
Subject: Re: [External] : bug#72229: (setq overriding-terminal-local-map
 nil) in isearch-done
Date: Wed, 24 Jul 2024 22:12:10 +0200

Drew Adams <drew.adams <at> oracle.com> writes:

> I'm not knowledgeable in this area, and I'm not
> sure what you're discussing (I think it's the
> possibility of substituting a function for a var),
> but would defining a symbol macro help?
>
> We at least have `cl-symbol-macrolet', even if we
> don't (yet) have `define-symbol-macro'.
>
> https://lisp-docs.github.io/cl-language-reference/chap-3/d-i-dictionary/define-symbol-macro_macro

We would need global symbol macros.  But even these would not be able to
affect variable references from C I think.

Symbol macros are also a bit fragile in Elisp, and some completely
dislike them (Richard for example).

So, yes, this is suggesting itself but would make the problem to solve
even harder, unfortunately.



Michael.

Previous Next

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