From mboxrd@z Thu Jan 1 00:00:00 1970 Path: news.gmane.io!.POSTED.blaine.gmane.org!not-for-mail From: Stefan Monnier via "Bug reports for GNU Emacs, the Swiss army knife of text editors" Newsgroups: gmane.emacs.bugs Subject: bug#68272: [PATCH] Fix -1 leaking from C to lisp in 'read-event' etc. Date: Tue, 06 Feb 2024 16:04:48 -0500 Message-ID: References: <46480759b6d89b5a4864e8ee1b986817366a56e5.camel@timruffing.de> <83wmsmucuo.fsf@gnu.org> <1b3fa12138838a3fe5643a9e76a65d32a677e34d.camel@timruffing.de> Reply-To: Stefan Monnier Mime-Version: 1.0 Content-Type: text/plain Injection-Info: ciao.gmane.io; posting-host="blaine.gmane.org:116.202.254.214"; logging-data="37176"; mail-complaints-to="usenet@ciao.gmane.io" User-Agent: Gnus/5.13 (Gnus v5.13) Cc: Eli Zaretskii , 68272@debbugs.gnu.org To: Tim Ruffing Original-X-From: bug-gnu-emacs-bounces+geb-bug-gnu-emacs=m.gmane-mx.org@gnu.org Tue Feb 06 22:06:17 2024 Return-path: Envelope-to: geb-bug-gnu-emacs@m.gmane-mx.org Original-Received: from lists.gnu.org ([209.51.188.17]) by ciao.gmane.io with esmtps (TLS1.2:ECDHE_RSA_AES_256_GCM_SHA384:256) (Exim 4.92) (envelope-from ) id 1rXSe0-0009VM-PV for geb-bug-gnu-emacs@m.gmane-mx.org; Tue, 06 Feb 2024 22:06:17 +0100 Original-Received: from localhost ([::1] helo=lists1p.gnu.org) by lists.gnu.org with esmtp (Exim 4.90_1) (envelope-from ) id 1rXSdh-0005fo-QA; Tue, 06 Feb 2024 16:05:57 -0500 Original-Received: from eggs.gnu.org ([2001:470:142:3::10]) by lists.gnu.org with esmtps (TLS1.2:ECDHE_RSA_AES_256_GCM_SHA384:256) (Exim 4.90_1) (envelope-from ) id 1rXSdZ-0005fR-AT for bug-gnu-emacs@gnu.org; Tue, 06 Feb 2024 16:05:49 -0500 Original-Received: from debbugs.gnu.org ([2001:470:142:5::43]) by eggs.gnu.org with esmtps (TLS1.2:ECDHE_RSA_AES_128_GCM_SHA256:128) (Exim 4.90_1) (envelope-from ) id 1rXSdZ-0004rW-2p for bug-gnu-emacs@gnu.org; Tue, 06 Feb 2024 16:05:49 -0500 Original-Received: from Debian-debbugs by debbugs.gnu.org with local (Exim 4.84_2) (envelope-from ) id 1rXSdm-0003yQ-Ai for bug-gnu-emacs@gnu.org; Tue, 06 Feb 2024 16:06:02 -0500 X-Loop: help-debbugs@gnu.org Resent-From: Stefan Monnier Original-Sender: "Debbugs-submit" Resent-CC: bug-gnu-emacs@gnu.org Resent-Date: Tue, 06 Feb 2024 21:06:02 +0000 Resent-Message-ID: Resent-Sender: help-debbugs@gnu.org X-GNU-PR-Message: followup 68272 X-GNU-PR-Package: emacs X-GNU-PR-Keywords: patch Original-Received: via spool by 68272-submit@debbugs.gnu.org id=B68272.170725351315211 (code B ref 68272); Tue, 06 Feb 2024 21:06:02 +0000 Original-Received: (at 68272) by debbugs.gnu.org; 6 Feb 2024 21:05:13 +0000 Original-Received: from localhost ([127.0.0.1]:55228 helo=debbugs.gnu.org) by debbugs.gnu.org with esmtp (Exim 4.84_2) (envelope-from ) id 1rXScy-0003xG-IA for submit@debbugs.gnu.org; Tue, 06 Feb 2024 16:05:12 -0500 Original-Received: from mailscanner.iro.umontreal.ca ([132.204.25.50]:59197) by debbugs.gnu.org with esmtp (Exim 4.84_2) (envelope-from ) id 1rXScw-0003x0-K4 for 68272@debbugs.gnu.org; Tue, 06 Feb 2024 16:05:11 -0500 Original-Received: from pmg3.iro.umontreal.ca (localhost [127.0.0.1]) by pmg3.iro.umontreal.ca (Proxmox) with ESMTP id 8246E440985; Tue, 6 Feb 2024 16:04:51 -0500 (EST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=iro.umontreal.ca; s=mail; t=1707253489; bh=/q+HwZheln83fykOPNvYexE6U4RMtO0c11xTe446nSI=; h=From:To:Cc:Subject:In-Reply-To:References:Date:From; b=me1KzoB1v0zG7iUuMCkPQz+uOEMXzG5x/lU7LVAyhIRBCBW4MYR6kQOAprojL0bLb UzWV4JGXLAQSDn36v5kgGDGEzog1ujDSgwiScVJoEMP6QBapLcIem+Aj78gkEZjjNf dePkTW1jVcj9Xh1/A1/YsbQmRQzOKALh32g4IbTQuScxh5b43v3+AhtVP2wPQh7CfL nnjLnnMOT3JZcGS/aM+agFJOfAB1MQjyE2novHVJqlB1MGN1kBzrrX+91FDozCD3B2 pP2h/QQzPrla9qA4Odpq4CHglSfeXFQwR/j9z31n4dvtRH0jNm7ieP7Fr9Bsy8ibfD 2fHgOaXxRcEdA== Original-Received: from mail01.iro.umontreal.ca (unknown [172.31.2.1]) by pmg3.iro.umontreal.ca (Proxmox) with ESMTP id 74ED244240B; Tue, 6 Feb 2024 16:04:49 -0500 (EST) Original-Received: from pastel (69-165-153-17.dsl.teksavvy.com [69.165.153.17]) by mail01.iro.umontreal.ca (Postfix) with ESMTPSA id 5007412021A; Tue, 6 Feb 2024 16:04:49 -0500 (EST) In-Reply-To: (Tim Ruffing's message of "Fri, 02 Feb 2024 19:04:45 +0100") X-BeenThere: debbugs-submit@debbugs.gnu.org X-Mailman-Version: 2.1.18 Precedence: list X-BeenThere: bug-gnu-emacs@gnu.org List-Id: "Bug reports for GNU Emacs, the Swiss army knife of text editors" List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: bug-gnu-emacs-bounces+geb-bug-gnu-emacs=m.gmane-mx.org@gnu.org Original-Sender: bug-gnu-emacs-bounces+geb-bug-gnu-emacs=m.gmane-mx.org@gnu.org Xref: news.gmane.io gmane.emacs.bugs:279523 Archived-At: > 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