Package: emacs;
Reported by: Daniel Colascione <dancol <at> dancol.org>
Date: Mon, 9 Jun 2025 20:50:02 UTC
Severity: normal
View this message in rfc822 format
From: Daniel Colascione <dancol <at> dancol.org> To: Pip Cet <pipcet <at> protonmail.com> Cc: 78737 <at> debbugs.gnu.org, Eli Zaretskii <eliz <at> gnu.org>, Stefan Monnier <monnier <at> iro.umontreal.ca> Subject: bug#78737: sit-for behavior changes when byte-compiled Date: Wed, 11 Jun 2025 05:40:29 -0700
Pip Cet <pipcet <at> protonmail.com> writes: > "Daniel Colascione" <dancol <at> dancol.org> writes: > >> On June 10, 2025 10:40:39 AM PDT, Pip Cet <pipcet <at> protonmail.com> wrote: >>>"Stefan Monnier" <monnier <at> iro.umontreal.ca> writes: >>> >>>>> BTW: the problem isn't just with transient. It also manifests with >>>>> read-extended-command! It's a nasty race that, AFAICT, has been with us >>>>> since the 90s. I think defining read_char to translate quits to quit_char >>>>> solves the problem. >>>> >>>> I like your way of thinking. I'm not completely sure it will solve >>>> world hunger, and it may come with regressions, but it's worth a try. >>> >>>I must be missing something: read_char translates quits to quit_char if >>>called with inhibit-quit bound to t, and never returns with quit-flag >>>set to t in that case. >> >> You shouldn't have to call it with inhibit-quit for it do that. It should just happen all the time. > > But we don't usually want read-event to eat quits and report them by > returning quit_char. The old behavior gave me a choice. You still have a choice. You can signal quit if you get a quit event. > (while t (read-event)) > > on your branch appears to hang the Emacs session unquittably (even > SIGUSR2 doesn't seem to work). It should permit quits, because that > code says nothing about wanting quits to be reported. By calling read-event, you're asking Emacs read an event. C-g is an event. Lisp isn't saying read-event-unless-it's-C-g-in-which-case-exit-non-locally. Lisp says read-event. Emacs is doing exactly what Lisp is asking it to do. If Lisp wants to quit in response to a char_char event, it can just (signal 'quit nil). Besides, (let ((inhibit-quit t)) (while t (read-event))) will be similarly unquittable on master today. (while t ((read-key-sequence "))) is unquittable too. On current master, (read-event) then C-g signals quit but (read-key-sequence "") C-g returns a quit_char key. Why? Why should we expect (while t (read-event)) to be quittable but not (while t (read-key-sequence "")), which can't be quit today on master? Why would the _high level_ event reading function report quit as a character but the low-level one exit nonlocally? Consider, say, (let ((unread-command-events '(\?C-g))) (read-event)). That'll return \?C-g, not quit. Therefore, anyone who calls (read-event) has to account for both ways of C-g being reported. What's the benefit of this complexity? So what if the behavior of read-event changes? Does having read-event report a quit as an event actually break anything considering that _sometimes_ it already can report a C-g as an event and not a quit signal. We need to be consistent about how we treat C-g when reading events. It makes no sense to quit while reading an event because in order to quit, you must first read an event! The current ambiguity is confusing and leads to real-world actual bugs. A lisp-visible behavior change isn't dispositive by itself: any resolution of the current ambiguity will result in new behavior, so we might as well make it good new behavior. If C-g means quit, we should resolve the inconsistency above by having read-event quit if it finds a C-g events in a queue. We also need to change cmd_error to detect quits, push a synthetic quit_char to unread_command_events, return Qt, and have the command loop look up the event on the next loop. That'll fix the transient bug. If C-g means quit_char, which is the better behavior, we need to document that functions that read events report quits as quit_char. Thanks for pointing out the SIGUSR2 thing. That's just a bug we'll fix on the branch. Likewise, we should try to get the emergency quit mechanism working somehow too. None of this means that (read-event) then physically hitting C-g means (read-event) should quit rather than return. > Reporting them as quit_char doesn't always make sense, since there are > other ways to generate a quit, such as SIGUSR2, We can make SIGUSR2 generate a subtype of quit and have read_char translate only the basic event. Likewise for emergency quits. > and quit_char can change. Can it in practice? Lots of stuff hardcodes C-g and I'm not confident that a non-default quit_char would produce usable behavior. I'd rather just remove set-quit-char than worry about quit_char changing. >> I don't think it should do that, it doesn't >>>match the quit-flag documentation, but it is what happens right now. Do >>>we really need a new function which is precisely equivalent to >>> >>>(let ((inhibit-quit t)) (read-event ...)) ? >>> >>>As for throw-on-input, I don't know what Daniel is proposing to do to >>>handle it. Is every caller of read-event supposed to check >>>throw-on-input and simulate it? How is that better than looking at >>>quit-flag, or simply keeping inhibit-quit bound for the critical >>>section? >> >> See the fix in the fix i mentioned a message ago. Now the read event >> functions detect that they're in a context in which quitting is >> inevitable and try to save the event before control even gets to >> Lisp. Should be transparent change. > > It's pushing the event without the (cons t event) wrapper now? Won't > that change behavior of sit-for (and adding the wrapper would change > behavior of other users)? > > Maybe we should just fix the original problem by making read_char call > maybe_quit instead of removing an event from the queue, if Vquit_flag is > set. I don't think it's a good idea to have parallel event processing paths, one for events delivered as Lisp objects and one for events delivered as signals. > No reason to change anything about the API, read_char already calls Lisp > so quitting should not be a problem (famous last words). > > Of course, the ability to peek at/wait for events would still be a good > thing, as would the ability to nest while-no-input invocations. > >>>As for peeking at events, the easiest way seems to me to be to let-bind >>>unread-command-events to nil around a call to read-event. That'll make >>>it ignore them, read the next event, which you can then append to >>>unread-command-events or not depending on whether you want the command >>>loop to handle it. >> >> Isn't unread command events kind of lossy and not something we want to use all the time? > > How is it "kind of lossy"? Anyway, you're using it on your branch, so > if it's unsafe it'll be unsafe for you, too. > > If your concern is that unread-command-events might be updated > asynchronously as quit-flag is, I don't think we do that. > >>>I agree it would make sense to separate inhibit-quit meaning "inhibit >>>acting on the quit flag" and inhibit-quit meaning "inhibit setting the >>>quit flag", but that seems a minor change. >> >> We have a lot of code that makes subtle assumptions about the meaning >> of the quit flag. I wouldn't change it lightly. The more local changes >> on my branch seem sufficient. > > Er, you just did change it, in a way that breaks existing behavior. But > I agree, no changes such as that one are necessary. The above is talking about changing the low-level meaning of Vinhibit_quit in C. Lots of code assumes that binding Vinhibit_quit will result in Vquit_flag being set and I'm a lot more worried about breaking things as a result of changing _that_ behavior than I am about making read_char consistent about how it reports C-g. > Anyway, here are the minor changes to keyboard.c to avoid the original > problem (the third change is somewhat independent and avoids quitting in > kbd_buffer_get_event): This change papers over the problem of ambiguous C-g representation without trying to fix it or address the even worse problem of the quits going to the event loop and not being looked up as commands.
GNU bug tracking system
Copyright (C) 1999 Darren O. Benham,
1997,2003 nCipher Corporation Ltd,
1994-97 Ian Jackson.