From mboxrd@z Thu Jan 1 00:00:00 1970 Path: news.gmane.io!.POSTED.blaine.gmane.org!not-for-mail From: Stefan Monnier Newsgroups: gmane.emacs.devel Subject: Re: master 47b377f64b: Prevent non-local exits from ns-in-echo-area Date: Sat, 12 Nov 2022 20:52:40 -0500 Message-ID: References: <166830147947.13137.4061439325094811374@vcs2.savannah.gnu.org> <20221113010439.BF21DC00AB5@vcs2.savannah.gnu.org> Mime-Version: 1.0 Content-Type: text/plain Injection-Info: ciao.gmane.io; posting-host="blaine.gmane.org:116.202.254.214"; logging-data="18822"; mail-complaints-to="usenet@ciao.gmane.io" User-Agent: Gnus/5.13 (Gnus v5.13) Cc: Po Lu To: emacs-devel@gnu.org Original-X-From: emacs-devel-bounces+ged-emacs-devel=m.gmane-mx.org@gnu.org Sun Nov 13 02:53:33 2022 Return-path: Envelope-to: ged-emacs-devel@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 1ou2Bg-0004hi-RW for ged-emacs-devel@m.gmane-mx.org; Sun, 13 Nov 2022 02:53:32 +0100 Original-Received: from localhost ([::1] helo=lists1p.gnu.org) by lists.gnu.org with esmtp (Exim 4.90_1) (envelope-from ) id 1ou2Az-0003dI-1U; Sat, 12 Nov 2022 20:52:49 -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 1ou2Ay-0003bO-5T for emacs-devel@gnu.org; Sat, 12 Nov 2022 20:52:48 -0500 Original-Received: from mailscanner.iro.umontreal.ca ([132.204.25.50]) by eggs.gnu.org with esmtps (TLS1.2:ECDHE_RSA_AES_256_GCM_SHA384:256) (Exim 4.90_1) (envelope-from ) id 1ou2Aw-0000B1-Co for emacs-devel@gnu.org; Sat, 12 Nov 2022 20:52:47 -0500 Original-Received: from pmg2.iro.umontreal.ca (localhost.localdomain [127.0.0.1]) by pmg2.iro.umontreal.ca (Proxmox) with ESMTP id 53CD5807E9; Sat, 12 Nov 2022 20:52:44 -0500 (EST) Original-Received: from mail01.iro.umontreal.ca (unknown [172.31.2.1]) by pmg2.iro.umontreal.ca (Proxmox) with ESMTP id 3F9FB80539; Sat, 12 Nov 2022 20:52:42 -0500 (EST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=iro.umontreal.ca; s=mail; t=1668304362; bh=hIplNo2TBbThONOdaxOIQN3U3uCQU6ewvg2skkekh4w=; h=From:To:Cc:Subject:In-Reply-To:References:Date:From; b=btzDj9HstfPyVd0VsWwjjpsLrdJ6DO5TMdfWn7v/a3jafTK8ok8wuJUv4WDNa3jqz T1I8VDYgcJeTILHTZYmKT4T0+jfhrEpKBEEe2d++LOkIXtkSVzyl9e4f8CnMmB+mUV 9Cm/iywtUTuQ5CK2ZglaSnLaCVvVq8Ng375e5JSvlRX0eKJm7S0JF6o40EiF52VgaT p7pHUNd9L13L1+WqXJ3No6K0JhzybHEh9zrdzN8lY5t0aXu/q3KFW/fEImB3UX7zYa PWM0iyQxJ/TqL8FsERtTR6yI600F0CZFBlbblUH2qQ09EbqGDeaJhMi9y61Xei3Dca 4S8yE+6KUObCA== Original-Received: from pastel (unknown [104.247.241.157]) by mail01.iro.umontreal.ca (Postfix) with ESMTPSA id 16B92120247; Sat, 12 Nov 2022 20:52:42 -0500 (EST) In-Reply-To: <20221113010439.BF21DC00AB5@vcs2.savannah.gnu.org> (Po Lu via Mailing list for Emacs changes's message of "Sat, 12 Nov 2022 20:04:39 -0500 (EST)") Received-SPF: pass client-ip=132.204.25.50; envelope-from=monnier@iro.umontreal.ca; helo=mailscanner.iro.umontreal.ca X-Spam_score_int: -42 X-Spam_score: -4.3 X-Spam_bar: ---- X-Spam_report: (-4.3 / 5.0 requ) BAYES_00=-1.9, DKIM_SIGNED=0.1, DKIM_VALID=-0.1, DKIM_VALID_AU=-0.1, RCVD_IN_DNSWL_MED=-2.3, SPF_HELO_NONE=0.001, SPF_PASS=-0.001 autolearn=ham autolearn_force=no X-Spam_action: no action X-BeenThere: emacs-devel@gnu.org X-Mailman-Version: 2.1.29 Precedence: list List-Id: "Emacs development discussions." List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: emacs-devel-bounces+ged-emacs-devel=m.gmane-mx.org@gnu.org Original-Sender: emacs-devel-bounces+ged-emacs-devel=m.gmane-mx.org@gnu.org Xref: news.gmane.io gmane.emacs.devel:299690 Archived-At: Hmm... this still looks like an ugly hack, and still without any comments explaining why we're doing it. E.g. why do we need `safe_call` in addition to `internal_catch_all`? If we're using `safe_call`, why not use that code's handling of `inhibit_quit` and write our own around it instead? AFAICT we don't *really* know why we need `internal_catch_all` or `inhibit-quit` and even less if/why we need both. And we don't know what we *should* do if this code signals a weird error or `throw`s, really (currently, we just silently drop the error/throw, but maybe we should arrange for it to be "recreated" elsewhere). I'm not in a position to dig any further, and it looks like noone here is either, but *please* add a comment explaining as much as you can about what we know now, so as to help the future guy who wants to try and figure it out (or so as to avoid re-introducing the problem when someone stumbles upon this weird code full of redundant protections and figures it can be simplified). Stefan Po Lu via Mailing list for Emacs changes [2022-11-12 20:04:39] wrote: > branch: master > commit 47b377f64bef8c3da519b3aa9c5c90b7199ba524 > Author: Po Lu > Commit: Po Lu > > Prevent non-local exits from ns-in-echo-area > > * src/nsterm.m (ns_in_echo_area_1): > (ns_in_echo_area_2): > (ns_in_echo_area): New functions. > ([EmacsView firstRectForCharacterRange:]): Call them instead. > (syms_of_nsterm): New defsym. > --- > src/nsterm.m | 33 ++++++++++++++++++++++++++++++++- > 1 file changed, 32 insertions(+), 1 deletion(-) > > diff --git a/src/nsterm.m b/src/nsterm.m > index 17f40dc7e3..507f2a9e7d 100644 > --- a/src/nsterm.m > +++ b/src/nsterm.m > @@ -7056,6 +7056,36 @@ ns_create_font_panel_buttons (id target, SEL select, SEL cancel_action) > processingCompose = NO; > } > > +static Lisp_Object > +ns_in_echo_area_1 (void *ptr) > +{ > + Lisp_Object in_echo_area; > + specpdl_ref count; > + > + count = SPECPDL_INDEX (); > + specbind (Qinhibit_quit, Qt); > + in_echo_area = safe_call (1, Qns_in_echo_area); > + > + return unbind_to (count, in_echo_area); > +} > + > +static Lisp_Object > +ns_in_echo_area_2 (enum nonlocal_exit exit, Lisp_Object error) > +{ > + return Qnil; > +} > + > +static bool > +ns_in_echo_area (void) > +{ > + Lisp_Object in_echo_area; > + > + in_echo_area > + = internal_catch_all (ns_in_echo_area_1, NULL, > + ns_in_echo_area_2); > + > + return !NILP (in_echo_area); > +} > > /* Used to position char selection windows, etc. */ > - (NSRect)firstRectForCharacterRange: (NSRange)theRange > @@ -7069,7 +7099,7 @@ ns_create_font_panel_buttons (id target, SEL select, SEL cancel_action) > if (NS_KEYLOG) > NSLog (@"firstRectForCharRange request"); > > - if (WINDOWP (echo_area_window) && ! NILP (call0 (intern ("ns-in-echo-area")))) > + if (WINDOWP (echo_area_window) && ns_in_echo_area ()) > win = XWINDOW (echo_area_window); > else > win = XWINDOW (FRAME_SELECTED_WINDOW (emacsframe)); > @@ -11012,6 +11042,7 @@ respectively. */); > DEFSYM (Qcondensed, "condensed"); > DEFSYM (Qreverse_italic, "reverse-italic"); > DEFSYM (Qexpanded, "expanded"); > + DEFSYM (Qns_in_echo_area, "ns-in-echo-area"); > > #ifdef NS_IMPL_COCOA > Fprovide (Qcocoa, Qnil);