From: Stefan Monnier via "Bug reports for GNU Emacs, the Swiss army knife of text editors" <bug-gnu-emacs@gnu.org>
To: Tim Ruffing <crypto@timruffing.de>
Cc: Eli Zaretskii <eliz@gnu.org>, 68272@debbugs.gnu.org
Subject: bug#68272: [PATCH] Fix -1 leaking from C to lisp in 'read-event' etc.
Date: Tue, 06 Feb 2024 16:04:48 -0500 [thread overview]
Message-ID: <jwvzfwduxg2.fsf-monnier+emacs@gnu.org> (raw)
In-Reply-To: <ee99965d25ed8f4ed8f2235fbc707930525c2952.camel@timruffing.de> (Tim Ruffing's message of "Fri, 02 Feb 2024 19:04:45 +0100")
> Just to make sure we're on same page: You mean it should (maybe) push
> the events into `unread-command-events` assuming LIFO semantics?
Yes.
>> Seeing how `calc.el` used the -1 to signal an error (i.e. forcing
>> kmacros to contain "complete sequences"), maybe a half-way behavior
>> between the current one and your new one would be for `read_char` to
>> return -1 (or rather a more explicit `end-of-kbd-macro`) event
>> *once* and then on the next call to go on and read from the keyboard.
>
> Hm, indeed. But to be honest, I'm not convinced that lisp code should
> be able to distinguish this at all when trying to get an event. (If you
> *really* want to detect this, you could still check `executing-kbd-
> macro` after reading an event and see if it has changed from non-nil to
> nil.) Moreover, it would make the C code more complicated, because
> we'll probably have to add another state variable that captures if
> we've returned -1 (or a symbol) already or not.
FWIW, I tend to agree.
> Another compromise, which I find nicer, is to introduce a new variant
> of `read-char` (or add a flag) that suppresses the -1 entirely.
I'd rather not go there, if we can avoid it.
> 1. Work on an improved version of my patch, which brings it closer
> to the ideal semantics, but doesn't touch too much of the code?
That's what I'd vote for.
> 2. Write a new patch that implements the "full" non-nesting
> semantics suggest above?
In the long run it might be worth trying it out, but I suspect this
/will/ bump into surprising corner cases, so I'd keep it as a subsequent
step which could be made optional (I suspect it can be implemented fully
in ELisp).
>> IIUC, with your patch, we have the following scenario:
>> - Say we're inside a kmacro with N events left to execute.
>> - Dbus reads those N events and stashes them them onto `unread-
>> command-events`.
>> - Dbus finally can read the actual dbus event and does its thing.
>> - Good!
>> - But now `at_end_of_macro_p` will return true, even though we've yet
>> to
>> execute the last N events. We'll presumably still execute them
>> (since they're in `unread-command-events`) but that won't be
>> considered as coming from a kmacro any more.
> This matches my understanding. My thinking is that this is not a big
> deal in this specific case. The dbus code currently relies on the idea
> of reading events and putting them back to `unread-command-events`.
> While the patch affects the behavior, it doesn't change anything about
> the fact that this is an ugly hack anyway.
You mean they had it coming? I can agree to some extent, but currently
there aren't very many alternative approaches :-(
>> Yes, I think we should document in a comment somewhere how the end of
>> kmacro is handled from a "global" perspective: what `read_char` does
>> when it's the case, where/when it's supposed to be detected and how
>> this "signal" is propagated from there to the corresponding call to
>> `execute-kbd-macro`, how that relates to setting
>> `executing-kbd-macro` back to nil, ...
>
> Agreed.
Could you try writing such a description? It doesn't have to be
complete: just write what you happen to know already.
Stefan
next prev parent reply other threads:[~2024-02-06 21:04 UTC|newest]
Thread overview: 24+ messages / expand[flat|nested] mbox.gz Atom feed top
2024-01-05 21:19 bug#68272: [PATCH] Fix -1 leaking from C to lisp in 'read-event' etc Tim Ruffing
2024-01-06 7:42 ` Eli Zaretskii
2024-01-06 14:32 ` Tim Ruffing
2024-01-13 9:39 ` Eli Zaretskii
2024-01-13 17:40 ` Stefan Monnier via Bug reports for GNU Emacs, the Swiss army knife of text editors
2024-02-02 18:04 ` Tim Ruffing
2024-02-06 21:04 ` Stefan Monnier via Bug reports for GNU Emacs, the Swiss army knife of text editors [this message]
2024-03-01 12:14 ` Tim Ruffing
2024-03-04 18:42 ` Tim Ruffing
2024-03-05 13:10 ` Eli Zaretskii
2024-03-05 16:45 ` Stefan Monnier via Bug reports for GNU Emacs, the Swiss army knife of text editors
2024-03-05 16:55 ` Eli Zaretskii
2024-03-05 17:57 ` Stefan Monnier via Bug reports for GNU Emacs, the Swiss army knife of text editors
2024-03-05 18:53 ` Eli Zaretskii
2024-03-05 19:29 ` Stefan Monnier via Bug reports for GNU Emacs, the Swiss army knife of text editors
2024-03-05 19:55 ` Eli Zaretskii
2024-03-05 20:18 ` Stefan Monnier via Bug reports for GNU Emacs, the Swiss army knife of text editors
2024-03-06 11:46 ` Eli Zaretskii
2024-03-09 12:33 ` Tim Ruffing
2024-03-09 18:08 ` Stefan Monnier via Bug reports for GNU Emacs, the Swiss army knife of text editors
2024-03-09 18:37 ` Eli Zaretskii
2024-03-10 8:24 ` Michael Albinus via Bug reports for GNU Emacs, the Swiss army knife of text editors
2024-03-10 14:48 ` Stefan Monnier via Bug reports for GNU Emacs, the Swiss army knife of text editors
2024-01-06 9:15 ` Andreas Schwab
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
List information: https://www.gnu.org/software/emacs/
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=jwvzfwduxg2.fsf-monnier+emacs@gnu.org \
--to=bug-gnu-emacs@gnu.org \
--cc=68272@debbugs.gnu.org \
--cc=crypto@timruffing.de \
--cc=eliz@gnu.org \
--cc=monnier@iro.umontreal.ca \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
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).