all messages for Emacs-related lists mirrored at yhetil.org
 help / color / mirror / code / Atom feed
* bug#72229: (setq overriding-terminal-local-map nil) in isearch-done
@ 2024-07-21 14:49 Michael Heerdegen via Bug reports for GNU Emacs, the Swiss army knife of text editors
  2024-07-22 12:49 ` Michael Heerdegen via Bug reports for GNU Emacs, the Swiss army knife of text editors
  0 siblings, 1 reply; 10+ messages in thread
From: Michael Heerdegen via Bug reports for GNU Emacs, the Swiss army knife of text editors @ 2024-07-21 14:49 UTC (permalink / raw)
  To: 72229


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.





^ permalink raw reply	[flat|nested] 10+ messages in thread

* bug#72229: (setq overriding-terminal-local-map nil) in isearch-done
  2024-07-21 14:49 bug#72229: (setq overriding-terminal-local-map nil) in isearch-done Michael Heerdegen via Bug reports for GNU Emacs, the Swiss army knife of text editors
@ 2024-07-22 12:49 ` Michael Heerdegen via Bug reports for GNU Emacs, the Swiss army knife of text editors
  2024-07-23  6:32   ` Juri Linkov
  0 siblings, 1 reply; 10+ messages in thread
From: Michael Heerdegen via Bug reports for GNU Emacs, the Swiss army knife of text editors @ 2024-07-22 12:49 UTC (permalink / raw)
  To: 72229; +Cc: Juri Linkov

Michael Heerdegen via "Bug reports for GNU Emacs, the Swiss army knife
of text editors" <bug-gnu-emacs@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.





^ permalink raw reply	[flat|nested] 10+ messages in thread

* bug#72229: (setq overriding-terminal-local-map nil) in isearch-done
  2024-07-22 12:49 ` Michael Heerdegen via Bug reports for GNU Emacs, the Swiss army knife of text editors
@ 2024-07-23  6:32   ` Juri Linkov
  2024-07-23 11:29     ` Eli Zaretskii
  2024-07-23 16:05     ` Michael Heerdegen via Bug reports for GNU Emacs, the Swiss army knife of text editors
  0 siblings, 2 replies; 10+ messages in thread
From: Juri Linkov @ 2024-07-23  6:32 UTC (permalink / raw)
  To: Michael Heerdegen; +Cc: 72229

[-- Attachment #1: Type: text/plain, Size: 735 bytes --]

> Michael Heerdegen via "Bug reports for GNU Emacs, the Swiss army knife
> of text editors" <bug-gnu-emacs@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:


[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: isearch--saved-local-map.patch --]
[-- Type: text/x-diff, Size: 2279 bytes --]

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))))

[-- Attachment #3: Type: text/plain, Size: 970 bytes --]


> 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.

^ permalink raw reply related	[flat|nested] 10+ messages in thread

* bug#72229: (setq overriding-terminal-local-map nil) in isearch-done
  2024-07-23  6:32   ` Juri Linkov
@ 2024-07-23 11:29     ` Eli Zaretskii
  2024-07-23 17:54       ` Juri Linkov
  2024-07-23 16:05     ` Michael Heerdegen via Bug reports for GNU Emacs, the Swiss army knife of text editors
  1 sibling, 1 reply; 10+ messages in thread
From: Eli Zaretskii @ 2024-07-23 11:29 UTC (permalink / raw)
  To: Juri Linkov; +Cc: michael_heerdegen, 72229

> Cc: 72229@debbugs.gnu.org
> From: Juri Linkov <juri@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@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).





^ permalink raw reply	[flat|nested] 10+ messages in thread

* bug#72229: (setq overriding-terminal-local-map nil) in isearch-done
  2024-07-23  6:32   ` Juri Linkov
  2024-07-23 11:29     ` Eli Zaretskii
@ 2024-07-23 16:05     ` Michael Heerdegen via Bug reports for GNU Emacs, the Swiss army knife of text editors
  2024-07-23 17:46       ` Juri Linkov
  1 sibling, 1 reply; 10+ messages in thread
From: Michael Heerdegen via Bug reports for GNU Emacs, the Swiss army knife of text editors @ 2024-07-23 16:05 UTC (permalink / raw)
  To: Juri Linkov; +Cc: 72229

Juri Linkov <juri@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.





^ permalink raw reply	[flat|nested] 10+ messages in thread

* bug#72229: (setq overriding-terminal-local-map nil) in isearch-done
  2024-07-23 16:05     ` Michael Heerdegen via Bug reports for GNU Emacs, the Swiss army knife of text editors
@ 2024-07-23 17:46       ` Juri Linkov
  2024-07-24 16:42         ` Michael Heerdegen via Bug reports for GNU Emacs, the Swiss army knife of text editors
  0 siblings, 1 reply; 10+ messages in thread
From: Juri Linkov @ 2024-07-23 17:46 UTC (permalink / raw)
  To: Michael Heerdegen; +Cc: 72229

>> 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.





^ permalink raw reply	[flat|nested] 10+ messages in thread

* bug#72229: (setq overriding-terminal-local-map nil) in isearch-done
  2024-07-23 11:29     ` Eli Zaretskii
@ 2024-07-23 17:54       ` Juri Linkov
  0 siblings, 0 replies; 10+ messages in thread
From: Juri Linkov @ 2024-07-23 17:54 UTC (permalink / raw)
  To: Eli Zaretskii; +Cc: michael_heerdegen, 72229

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.





^ permalink raw reply	[flat|nested] 10+ messages in thread

* bug#72229: (setq overriding-terminal-local-map nil) in isearch-done
  2024-07-23 17:46       ` Juri Linkov
@ 2024-07-24 16:42         ` Michael Heerdegen via Bug reports for GNU Emacs, the Swiss army knife of text editors
  2024-07-24 17:27           ` Drew Adams via Bug reports for GNU Emacs, the Swiss army knife of text editors
  0 siblings, 1 reply; 10+ messages in thread
From: Michael Heerdegen via Bug reports for GNU Emacs, the Swiss army knife of text editors @ 2024-07-24 16:42 UTC (permalink / raw)
  To: Juri Linkov; +Cc: 72229

Juri Linkov <juri@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.





^ permalink raw reply	[flat|nested] 10+ messages in thread

* bug#72229: (setq overriding-terminal-local-map nil) in isearch-done
  2024-07-24 16:42         ` Michael Heerdegen via Bug reports for GNU Emacs, the Swiss army knife of text editors
@ 2024-07-24 17:27           ` Drew Adams via Bug reports for GNU Emacs, the Swiss army knife of text editors
  2024-07-24 20:12             ` Michael Heerdegen via Bug reports for GNU Emacs, the Swiss army knife of text editors
  0 siblings, 1 reply; 10+ messages in thread
From: Drew Adams via Bug reports for GNU Emacs, the Swiss army knife of text editors @ 2024-07-24 17:27 UTC (permalink / raw)
  To: Michael Heerdegen, Juri Linkov; +Cc: 72229@debbugs.gnu.org

> > 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





^ permalink raw reply	[flat|nested] 10+ messages in thread

* bug#72229: (setq overriding-terminal-local-map nil) in isearch-done
  2024-07-24 17:27           ` Drew Adams via Bug reports for GNU Emacs, the Swiss army knife of text editors
@ 2024-07-24 20:12             ` Michael Heerdegen via Bug reports for GNU Emacs, the Swiss army knife of text editors
  0 siblings, 0 replies; 10+ messages in thread
From: Michael Heerdegen via Bug reports for GNU Emacs, the Swiss army knife of text editors @ 2024-07-24 20:12 UTC (permalink / raw)
  To: Drew Adams; +Cc: 72229@debbugs.gnu.org, Juri Linkov

Drew Adams <drew.adams@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.





^ permalink raw reply	[flat|nested] 10+ messages in thread

end of thread, other threads:[~2024-07-24 20:12 UTC | newest]

Thread overview: 10+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2024-07-21 14:49 bug#72229: (setq overriding-terminal-local-map nil) in isearch-done Michael Heerdegen via Bug reports for GNU Emacs, the Swiss army knife of text editors
2024-07-22 12:49 ` Michael Heerdegen via Bug reports for GNU Emacs, the Swiss army knife of text editors
2024-07-23  6:32   ` Juri Linkov
2024-07-23 11:29     ` Eli Zaretskii
2024-07-23 17:54       ` Juri Linkov
2024-07-23 16:05     ` Michael Heerdegen via Bug reports for GNU Emacs, the Swiss army knife of text editors
2024-07-23 17:46       ` Juri Linkov
2024-07-24 16:42         ` Michael Heerdegen via Bug reports for GNU Emacs, the Swiss army knife of text editors
2024-07-24 17:27           ` Drew Adams via Bug reports for GNU Emacs, the Swiss army knife of text editors
2024-07-24 20:12             ` Michael Heerdegen via Bug reports for GNU Emacs, the Swiss army knife of text editors

Code repositories for project(s) associated with this external index

	https://git.savannah.gnu.org/cgit/emacs.git
	https://git.savannah.gnu.org/cgit/emacs/org-mode.git

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.