GNU bug report logs - #73584
29.3; read-key

Previous Next

Package: emacs;

Reported by: Devon Sean McCullough <Emacs-hacker2023 <at> jovi.net>

Date: Tue, 1 Oct 2024 18:22:02 UTC

Severity: normal

Found in version 29.3

Done: Eli Zaretskii <eliz <at> gnu.org>

Bug is archived. No further changes may be made.

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

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

View this report as an mbox folder, status mbox, maintainer mbox


Report forwarded to bug-gnu-emacs <at> gnu.org:
bug#73584; Package emacs. (Tue, 01 Oct 2024 18:22:02 GMT) Full text and rfc822 format available.

Acknowledgement sent to Devon Sean McCullough <Emacs-hacker2023 <at> jovi.net>:
New bug report received and forwarded. Copy sent to bug-gnu-emacs <at> gnu.org. (Tue, 01 Oct 2024 18:22:02 GMT) Full text and rfc822 format available.

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

From: Devon Sean McCullough <Emacs-hacker2023 <at> jovi.net>
To: bug-gnu-emacs <at> gnu.org
Subject: 29.3; read-key
Date: Tue, 01 Oct 2024 13:21:11 -0500
(read-key 0) jails Emacs in a null keymap.

To escape:  Menu Bar > Help > Emacs Psychotherapist

        Peace
            --Devon

P.S.  In a tty Emacs you're out of luck
unless you're running the emacs server.

In GNU Emacs 29.3 (build 1, x86_64-apple-darwin18.7.0, NS appkit-1671.60
 Version 10.14.6 (Build 18G9323)) of 2024-03-24 built on
 builder10-14.lan
Windowing system distributor 'Apple', version 10.3.1671
System Description:  Mac OS X 10.14.6

Configured using:
 'configure --with-ns '--enable-locallisppath=/Library/Application
 Support/Emacs/${version}/site-lisp:/Library/Application
 Support/Emacs/site-lisp' --with-modules 'CFLAGS=-DFD_SETSIZE=10000
 -DDARWIN_UNLIMITED_SELECT' --with-x-toolkit=no'

Configured features:
ACL GLIB GMP GNUTLS JPEG JSON LIBXML2 MODULES NOTIFY KQUEUE NS PDUMPER
PNG RSVG SQLITE3 THREADS TIFF TOOLKIT_SCROLL_BARS TREE_SITTER ZLIB

Important settings:
  value of $LC_CTYPE: UTF-8
  value of $LANG: en_US.UTF-8
  locale-coding-system: utf-8-unix

Major mode: Lisp Interaction

Minor modes in effect:
  tooltip-mode: t
  global-eldoc-mode: t
  eldoc-mode: t
  show-paren-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
  line-number-mode: t
  indent-tabs-mode: t
  transient-mark-mode: t
  auto-composition-mode: t
  auto-encryption-mode: t
  auto-compression-mode: t

Load-path shadows:
None found.

Features:
(shadow sort mail-extr emacsbug message mailcap yank-media puny dired
dired-loaddefs rfc822 mml mml-sec password-cache epa derived epg rfc6068
epg-config gnus-util text-property-search time-date subr-x mm-decode
mm-bodies mm-encode mail-parse rfc2231 mailabbrev gmm-utils mailheader
cl-loaddefs cl-lib sendmail rfc2047 rfc2045 ietf-drums mm-util
mail-prsvr mail-utils rmc iso-transl tooltip cconv eldoc paren electric
uniquify ediff-hook vc-hooks lisp-float-type elisp-mode mwheel
term/ns-win ns-win ucs-normalize mule-util term/common-win tool-bar dnd
fontset image regexp-opt fringe tabulated-list replace newcomment
text-mode lisp-mode prog-mode register page tab-bar menu-bar rfn-eshadow
isearch easymenu timer select scroll-bar mouse jit-lock font-lock syntax
font-core term/tty-colors frame minibuffer nadvice seq simple cl-generic
indonesian philippine cham georgian utf-8-lang misc-lang vietnamese
tibetan thai tai-viet lao korean japanese eucjp-ms cp51932 hebrew greek
romanian slovak czech european ethiopic indian cyrillic chinese
composite emoji-zwj charscript charprop case-table epa-hook
jka-cmpr-hook help abbrev obarray oclosure cl-preloaded button loaddefs
theme-loaddefs faces cus-face macroexp files window text-properties
overlay sha1 md5 base64 format env code-pages mule custom widget keymap
hashtable-print-readable backquote threads kqueue cocoa ns multi-tty
make-network-process emacs)

Memory information:
((conses 16 37423 7895)
 (symbols 48 5048 0)
 (strings 32 12600 1784)
 (string-bytes 1 359254)
 (vectors 16 10402)
 (vector-slots 8 156348 13684)
 (floats 8 37 17)
 (intervals 56 218 0)
 (buffers 984 10))




Information forwarded to bug-gnu-emacs <at> gnu.org:
bug#73584; Package emacs. (Wed, 02 Oct 2024 06:02:01 GMT) Full text and rfc822 format available.

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

From: Eli Zaretskii <eliz <at> gnu.org>
To: Devon Sean McCullough <Emacs-hacker2023 <at> jovi.net>
Cc: 73584 <at> debbugs.gnu.org
Subject: Re: bug#73584: 29.3; read-key
Date: Wed, 02 Oct 2024 08:58:59 +0300
> Date: Tue, 01 Oct 2024 13:21:11 -0500
> From: Devon Sean McCullough <Emacs-hacker2023 <at> jovi.net>
> 
> (read-key 0) jails Emacs in a null keymap.

You should be able to escape the jail with ESC or C-[.

I don't see a bug here.




Information forwarded to bug-gnu-emacs <at> gnu.org:
bug#73584; Package emacs. (Wed, 02 Oct 2024 10:04:02 GMT) Full text and rfc822 format available.

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

From: Stefan Kangas <stefankangas <at> gmail.com>
To: Eli Zaretskii <eliz <at> gnu.org>,
 Devon Sean McCullough <Emacs-hacker2023 <at> jovi.net>
Cc: 73584 <at> debbugs.gnu.org
Subject: Re: bug#73584: 29.3; read-key
Date: Wed, 2 Oct 2024 10:02:44 +0000
Eli Zaretskii <eliz <at> gnu.org> writes:

>> Date: Tue, 01 Oct 2024 13:21:11 -0500
>> From: Devon Sean McCullough <Emacs-hacker2023 <at> jovi.net>
>>
>> (read-key 0) jails Emacs in a null keymap.
>
> You should be able to escape the jail with ESC or C-[.
>
> I don't see a bug here.

It would arguably be a bit nicer to signal the error early to avoid
having to mash C-[:

diff --git a/lisp/subr.el b/lisp/subr.el
index 2eaed682406..b3872513005 100644
--- a/lisp/subr.el
+++ b/lisp/subr.el
@@ -3307,6 +3307,8 @@ read-key
 what you want as `read-key' temporarily removes all bindings
 while calling `read-key-sequence'.  If nil or unspecified, the
 only unbound fallback disabled is downcasing of the last event."
+  (or (stringp prompt)
+      (signal 'wrong-type-argument (list 'stringp prompt)))
   ;; This overriding-terminal-local-map binding also happens to
   ;; disable quail's input methods, so although read-key-sequence
   ;; always inherits the input method, in practice read-key does not

BTW, do we have something like `cl-check-type' outside of cl-lib?




Information forwarded to bug-gnu-emacs <at> gnu.org:
bug#73584; Package emacs. (Wed, 02 Oct 2024 10:52:01 GMT) Full text and rfc822 format available.

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

From: Eli Zaretskii <eliz <at> gnu.org>
To: Stefan Kangas <stefankangas <at> gmail.com>
Cc: 73584 <at> debbugs.gnu.org, Emacs-hacker2023 <at> jovi.net
Subject: Re: bug#73584: 29.3; read-key
Date: Wed, 02 Oct 2024 13:50:55 +0300
> From: Stefan Kangas <stefankangas <at> gmail.com>
> Date: Wed, 2 Oct 2024 10:02:44 +0000
> Cc: 73584 <at> debbugs.gnu.org
> 
> Eli Zaretskii <eliz <at> gnu.org> writes:
> 
> >> Date: Tue, 01 Oct 2024 13:21:11 -0500
> >> From: Devon Sean McCullough <Emacs-hacker2023 <at> jovi.net>
> >>
> >> (read-key 0) jails Emacs in a null keymap.
> >
> > You should be able to escape the jail with ESC or C-[.
> >
> > I don't see a bug here.
> 
> It would arguably be a bit nicer to signal the error early to avoid
> having to mash C-[:

Maybe.  But what exactly is wrong with signaling the error from
read-key-sequence-vector, which read-key calls?

> --- a/lisp/subr.el
> +++ b/lisp/subr.el
> @@ -3307,6 +3307,8 @@ read-key
>  what you want as `read-key' temporarily removes all bindings
>  while calling `read-key-sequence'.  If nil or unspecified, the
>  only unbound fallback disabled is downcasing of the last event."
> +  (or (stringp prompt)
> +      (signal 'wrong-type-argument (list 'stringp prompt)))

This will signal an error if PROMPT is nil or omitted, which we
definitely must support.

>    ;; This overriding-terminal-local-map binding also happens to
>    ;; disable quail's input methods, so although read-key-sequence
>    ;; always inherits the input method, in practice read-key does not
> 
> BTW, do we have something like `cl-check-type' outside of cl-lib?

We shouldn't use any cl stuff in subr.el, surely?




Information forwarded to bug-gnu-emacs <at> gnu.org:
bug#73584; Package emacs. (Wed, 02 Oct 2024 21:49:01 GMT) Full text and rfc822 format available.

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

From: Stefan Kangas <stefankangas <at> gmail.com>
To: Eli Zaretskii <eliz <at> gnu.org>
Cc: 73584 <at> debbugs.gnu.org, Emacs-hacker2023 <at> jovi.net
Subject: Re: bug#73584: 29.3; read-key
Date: Wed, 2 Oct 2024 21:47:19 +0000
Eli Zaretskii <eliz <at> gnu.org> writes:

>> From: Stefan Kangas <stefankangas <at> gmail.com>
>> Date: Wed, 2 Oct 2024 10:02:44 +0000
>> Cc: 73584 <at> debbugs.gnu.org
>>
>> It would arguably be a bit nicer to signal the error early to avoid
>> having to mash C-[:
>
> Maybe.  But what exactly is wrong with signaling the error from
> read-key-sequence-vector, which read-key calls?

I just observe that I have to mash C-[ to get back control of Emacs, and
that it would probably have been better if I didn't have to.  I imagine
situations when this happens because of a bug in some library, and users
won't know what to do in that situation.  It clearly confused OP.

>> --- a/lisp/subr.el
>> +++ b/lisp/subr.el
>> @@ -3307,6 +3307,8 @@ read-key
>>  what you want as `read-key' temporarily removes all bindings
>>  while calling `read-key-sequence'.  If nil or unspecified, the
>>  only unbound fallback disabled is downcasing of the last event."
>> +  (or (stringp prompt)
>> +      (signal 'wrong-type-argument (list 'stringp prompt)))
>
> This will signal an error if PROMPT is nil or omitted, which we
> definitely must support.

Thanks, yes.  That would have to be amended.

>>    ;; This overriding-terminal-local-map binding also happens to
>>    ;; disable quail's input methods, so although read-key-sequence
>>    ;; always inherits the input method, in practice read-key does not
>>
>> BTW, do we have something like `cl-check-type' outside of cl-lib?
>
> We shouldn't use any cl stuff in subr.el, surely?

I meant to ask if we have something similar to `cl-check-type' that does
not come from cl-lib.el.  I'm asking that precisely because we can't use
cl-lib in subr.el.

It would have been convenient if we had something like:

    (defmacro check-type (form predicate)
       `(unless (,predicate form)
           (signal 'wrong-type-argument (list ,predicate form))))

We have this pattern in quite a few places in our code, so maybe we
should consider adding it if we don't have it already.




Information forwarded to bug-gnu-emacs <at> gnu.org:
bug#73584; Package emacs. (Thu, 03 Oct 2024 06:05:02 GMT) Full text and rfc822 format available.

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

From: Eli Zaretskii <eliz <at> gnu.org>
To: Stefan Kangas <stefankangas <at> gmail.com>,
 Stefan Monnier <monnier <at> iro.umontreal.ca>
Cc: 73584 <at> debbugs.gnu.org, Emacs-hacker2023 <at> jovi.net
Subject: Re: bug#73584: 29.3; read-key
Date: Thu, 03 Oct 2024 09:04:27 +0300
> From: Stefan Kangas <stefankangas <at> gmail.com>
> Date: Wed, 2 Oct 2024 21:47:19 +0000
> Cc: Emacs-hacker2023 <at> jovi.net, 73584 <at> debbugs.gnu.org
> 
> Eli Zaretskii <eliz <at> gnu.org> writes:
> 
> >> From: Stefan Kangas <stefankangas <at> gmail.com>
> >> Date: Wed, 2 Oct 2024 10:02:44 +0000
> >> Cc: 73584 <at> debbugs.gnu.org
> >>
> >> It would arguably be a bit nicer to signal the error early to avoid
> >> having to mash C-[:
> >
> > Maybe.  But what exactly is wrong with signaling the error from
> > read-key-sequence-vector, which read-key calls?
> 
> I just observe that I have to mash C-[ to get back control of Emacs, and
> that it would probably have been better if I didn't have to.  I imagine
> situations when this happens because of a bug in some library, and users
> won't know what to do in that situation.  It clearly confused OP.

The OP shot himself in the foot by invoking by hand a low-level
function with a wrong argument.  Interactive functions and high-level
APIs must check their arguments right away, but low-level functions
like this one do not.

What I could understand is if we'd arranged for the temporary keymap
to stop being in effect even inside the debugger.  Because the
scenario of this bug reveals some more general issue: when we signal
an error from a function that sets up some limited keymap, the
debugger is invoked with that keymap still in effect, or so it seems.

Stefan Monnier, do we have a good way of doing that in cases like
this?  Perhaps the debugger should define some minimal key bindings
when it starts or something?

> I meant to ask if we have something similar to `cl-check-type' that does
> not come from cl-lib.el.  I'm asking that precisely because we can't use
> cl-lib in subr.el.

Sorry, my misunderstanding.

> It would have been convenient if we had something like:
> 
>     (defmacro check-type (form predicate)
>        `(unless (,predicate form)
>            (signal 'wrong-type-argument (list ,predicate form))))
> 
> We have this pattern in quite a few places in our code, so maybe we
> should consider adding it if we don't have it already.

If we want something like this, I think we should support a list of
allowed types, not just one type.




Information forwarded to bug-gnu-emacs <at> gnu.org:
bug#73584; Package emacs. (Thu, 03 Oct 2024 15:44:02 GMT) Full text and rfc822 format available.

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

From: Stefan Monnier <monnier <at> iro.umontreal.ca>
To: Eli Zaretskii <eliz <at> gnu.org>
Cc: 73584 <at> debbugs.gnu.org, Stefan Kangas <stefankangas <at> gmail.com>,
 Emacs-hacker2023 <at> jovi.net
Subject: Re: bug#73584: 29.3; read-key
Date: Thu, 03 Oct 2024 11:43:25 -0400
> The OP shot himself in the foot by invoking by hand a low-level
> function with a wrong argument.  Interactive functions and high-level
> APIs must check their arguments right away, but low-level functions
> like this one do not.

I think the "bug" is indeed not in the fact that we check too late for
a string.  I guess we could say that any code which temporarily rebinds
the global map should be super-extra careful never to signal an error,
and prevent itself from being Edebugged etc... but it would be
preferable for the debuggers to "handle it".

> Stefan Monnier, do we have a good way of doing that in cases like
> this?  Perhaps the debugger should define some minimal key bindings
> when it starts or something?

The debuggers already do a fair bit of "setup" to try and isolate the
state of the debugger from the state of the debugged code.

If we got rid of `current-global-map` and `use-global-map` (and instead
just tied the ELisp `global-map` to C's `current_global_map`, so we
could (re) define those two functions in ELisp), then `read-key` could
let-bind `global-map` and the debuggers could also let-bind it (back) to
the value in `default-toplevel-value`.

Short of doing that we could have a new var `normal-global-map` which
would be initialized to the value of `global-map` and then make the
debuggers install that map with `set-global-map` while they're active.

More generally, the debuggers should (behave as if they're) run
in their own thread so they're not affected by the dynamic-bindings of
the code they're debugging.

>> I meant to ask if we have something similar to `cl-check-type' that does
>> not come from cl-lib.el.  I'm asking that precisely because we can't use
>> cl-lib in subr.el.
> Sorry, my misunderstanding.

A think a better course of action for this specific sub-problem is to
arrange for `read-keys` to be able to use `cl-check-type` (instead of
re-inventing it).

In general, I think it would be beneficial to split `subr.el` into two
files: one that is still loaded just as early and another that's loaded
a bit later so it can use `cl-lib` macros.
In the mean time, we can often get a similar result by moving the code
to `simple.el`.

There can be other ways to solve this bootstrap problem.  Such as
arranging for `subr.el` to load `cl-lib` in a slightly more careful way
than `eval-when-compile`: it would load it when the file is being
compiled (just like `eval-when-compile`) or it would schedule `cl-lib`
for "near future" loading (as soon as enough other files have been
loaded that `cl-lib` can be loaded).  As long as `cl-lib` is loaded
before `read-keys` is called, that would be sufficient.


        Stefan





Information forwarded to bug-gnu-emacs <at> gnu.org:
bug#73584; Package emacs. (Sun, 13 Oct 2024 11:43:02 GMT) Full text and rfc822 format available.

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

From: Eli Zaretskii <eliz <at> gnu.org>
To: Stefan Monnier <monnier <at> iro.umontreal.ca>
Cc: 73584 <at> debbugs.gnu.org, stefankangas <at> gmail.com, Emacs-hacker2023 <at> jovi.net
Subject: Re: bug#73584: 29.3; read-key
Date: Sun, 13 Oct 2024 14:41:34 +0300
> From: Stefan Monnier <monnier <at> iro.umontreal.ca>
> Cc: Stefan Kangas <stefankangas <at> gmail.com>,  Emacs-hacker2023 <at> jovi.net,
>   73584 <at> debbugs.gnu.org
> Date: Thu, 03 Oct 2024 11:43:25 -0400
> 
> > The OP shot himself in the foot by invoking by hand a low-level
> > function with a wrong argument.  Interactive functions and high-level
> > APIs must check their arguments right away, but low-level functions
> > like this one do not.
> 
> I think the "bug" is indeed not in the fact that we check too late for
> a string.  I guess we could say that any code which temporarily rebinds
> the global map should be super-extra careful never to signal an error,
> and prevent itself from being Edebugged etc... but it would be
> preferable for the debuggers to "handle it".

Is the patch below acceptable as the solution to this?

diff --git a/lisp/emacs-lisp/backtrace.el b/lisp/emacs-lisp/backtrace.el
index 120972d..e353970 100644
--- a/lisp/emacs-lisp/backtrace.el
+++ b/lisp/emacs-lisp/backtrace.el
@@ -202,6 +202,7 @@ backtrace-mode-map
   "+"   #'backtrace-multi-line
   "-"   #'backtrace-single-line
   "."   #'backtrace-expand-ellipses
+  "C-]"    #'abort-recursive-edit
   "<follow-link>" 'mouse-face
   "<mouse-2>"     #'mouse-select-window




Reply sent to Eli Zaretskii <eliz <at> gnu.org>:
You have taken responsibility. (Sun, 27 Oct 2024 10:31:01 GMT) Full text and rfc822 format available.

Notification sent to Devon Sean McCullough <Emacs-hacker2023 <at> jovi.net>:
bug acknowledged by developer. (Sun, 27 Oct 2024 10:31:02 GMT) Full text and rfc822 format available.

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

From: Eli Zaretskii <eliz <at> gnu.org>
To: Emacs-hacker2023 <at> jovi.net
Cc: 73584-done <at> debbugs.gnu.org, monnier <at> iro.umontreal.ca,
 stefankangas <at> gmail.com
Subject: Re: bug#73584: 29.3; read-key
Date: Sun, 27 Oct 2024 12:30:02 +0200
> Cc: 73584 <at> debbugs.gnu.org, stefankangas <at> gmail.com, Emacs-hacker2023 <at> jovi.net
> Date: Sun, 13 Oct 2024 14:41:34 +0300
> From: Eli Zaretskii <eliz <at> gnu.org>
> 
> > From: Stefan Monnier <monnier <at> iro.umontreal.ca>
> > Cc: Stefan Kangas <stefankangas <at> gmail.com>,  Emacs-hacker2023 <at> jovi.net,
> >   73584 <at> debbugs.gnu.org
> > Date: Thu, 03 Oct 2024 11:43:25 -0400
> > 
> > > The OP shot himself in the foot by invoking by hand a low-level
> > > function with a wrong argument.  Interactive functions and high-level
> > > APIs must check their arguments right away, but low-level functions
> > > like this one do not.
> > 
> > I think the "bug" is indeed not in the fact that we check too late for
> > a string.  I guess we could say that any code which temporarily rebinds
> > the global map should be super-extra careful never to signal an error,
> > and prevent itself from being Edebugged etc... but it would be
> > preferable for the debuggers to "handle it".
> 
> Is the patch below acceptable as the solution to this?
> 
> diff --git a/lisp/emacs-lisp/backtrace.el b/lisp/emacs-lisp/backtrace.el
> index 120972d..e353970 100644
> --- a/lisp/emacs-lisp/backtrace.el
> +++ b/lisp/emacs-lisp/backtrace.el
> @@ -202,6 +202,7 @@ backtrace-mode-map
>    "+"   #'backtrace-multi-line
>    "-"   #'backtrace-single-line
>    "."   #'backtrace-expand-ellipses
> +  "C-]"    #'abort-recursive-edit
>    "<follow-link>" 'mouse-face
>    "<mouse-2>"     #'mouse-select-window

No further comments within 2 weeks, so I've now installed this on the
master branch, and I'm therefore closing this bug.




Information forwarded to bug-gnu-emacs <at> gnu.org:
bug#73584; Package emacs. (Mon, 28 Oct 2024 22:55:02 GMT) Full text and rfc822 format available.

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

From: Stefan Kangas <stefankangas <at> gmail.com>
To: Eli Zaretskii <eliz <at> gnu.org>, Emacs-hacker2023 <at> jovi.net
Cc: 73584-done <at> debbugs.gnu.org, monnier <at> iro.umontreal.ca
Subject: Re: bug#73584: 29.3; read-key
Date: Mon, 28 Oct 2024 15:53:45 -0700
Eli Zaretskii <eliz <at> gnu.org> writes:

> No further comments within 2 weeks, so I've now installed this on the
> master branch, and I'm therefore closing this bug.

Thanks.




bug archived. Request was from Debbugs Internal Request <help-debbugs <at> gnu.org> to internal_control <at> debbugs.gnu.org. (Tue, 26 Nov 2024 12:24:09 GMT) Full text and rfc822 format available.

This bug report was last modified 205 days ago.

Previous Next


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