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#56682: locked narrowing Date: Tue, 16 Aug 2022 16:57:05 -0400 Message-ID: References: <66bbbb95983414e79637@heytings.org> <83wnb9hadb.fsf@gnu.org> <395454dd-7238-c5d0-e924-2f65a186baa7@yandex.ru> <83r11hh4pm.fsf@gnu.org> <3a1232a17b09ce88af40@heytings.org> <83edxghqg2.fsf@gnu.org> <325f95fd2bcc0b666b0b@heytings.org> <83edxgfi75.fsf@gnu.org> <5e3c3081-f098-8140-c767-b895c32bf30b@yandex.ru> <835yisffil.fsf@gnu.org> <831qtgff78.fsf@gnu.org> <83zgg4dw4y.fsf@gnu.org> <83r11gdrr4.fsf@gnu.org> <325f95fd2b8688c3aa74@heytings.org> 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="22178"; mail-complaints-to="usenet@ciao.gmane.io" User-Agent: Gnus/5.13 (Gnus v5.13) Emacs/29.0.50 (gnu/linux) Cc: 56682@debbugs.gnu.org, Eli Zaretskii , dgutov@yandex.ru To: Gregory Heytings Original-X-From: bug-gnu-emacs-bounces+geb-bug-gnu-emacs=m.gmane-mx.org@gnu.org Tue Aug 16 23:19:50 2022 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 1oO3yW-0005aS-Oo for geb-bug-gnu-emacs@m.gmane-mx.org; Tue, 16 Aug 2022 23:19:49 +0200 Original-Received: from localhost ([::1]:51162 helo=lists1p.gnu.org) by lists.gnu.org with esmtp (Exim 4.90_1) (envelope-from ) id 1oO3yV-0008SA-9y for geb-bug-gnu-emacs@m.gmane-mx.org; Tue, 16 Aug 2022 17:19:47 -0400 Original-Received: from eggs.gnu.org ([2001:470:142:3::10]:44816) by lists.gnu.org with esmtps (TLS1.2:ECDHE_RSA_AES_256_GCM_SHA384:256) (Exim 4.90_1) (envelope-from ) id 1oO3dT-0000yY-3z for bug-gnu-emacs@gnu.org; Tue, 16 Aug 2022 16:58:03 -0400 Original-Received: from debbugs.gnu.org ([209.51.188.43]:58751) by eggs.gnu.org with esmtps (TLS1.2:ECDHE_RSA_AES_128_GCM_SHA256:128) (Exim 4.90_1) (envelope-from ) id 1oO3dS-0000ub-Pi for bug-gnu-emacs@gnu.org; Tue, 16 Aug 2022 16:58:02 -0400 Original-Received: from Debian-debbugs by debbugs.gnu.org with local (Exim 4.84_2) (envelope-from ) id 1oO3dS-0001GL-I1 for bug-gnu-emacs@gnu.org; Tue, 16 Aug 2022 16:58:02 -0400 X-Loop: help-debbugs@gnu.org Resent-From: Stefan Monnier Original-Sender: "Debbugs-submit" Resent-CC: bug-gnu-emacs@gnu.org Resent-Date: Tue, 16 Aug 2022 20:58:02 +0000 Resent-Message-ID: Resent-Sender: help-debbugs@gnu.org X-GNU-PR-Message: followup 56682 X-GNU-PR-Package: emacs Original-Received: via spool by 56682-submit@debbugs.gnu.org id=B56682.16606834414804 (code B ref 56682); Tue, 16 Aug 2022 20:58:02 +0000 Original-Received: (at 56682) by debbugs.gnu.org; 16 Aug 2022 20:57:21 +0000 Original-Received: from localhost ([127.0.0.1]:48500 helo=debbugs.gnu.org) by debbugs.gnu.org with esmtp (Exim 4.84_2) (envelope-from ) id 1oO3cm-0001FP-F9 for submit@debbugs.gnu.org; Tue, 16 Aug 2022 16:57:20 -0400 Original-Received: from mailscanner.iro.umontreal.ca ([132.204.25.50]:58390) by debbugs.gnu.org with esmtp (Exim 4.84_2) (envelope-from ) id 1oO3ci-0001F8-Er for 56682@debbugs.gnu.org; Tue, 16 Aug 2022 16:57:18 -0400 Original-Received: from pmg2.iro.umontreal.ca (localhost.localdomain [127.0.0.1]) by pmg2.iro.umontreal.ca (Proxmox) with ESMTP id CB1E98079C; Tue, 16 Aug 2022 16:57:10 -0400 (EDT) Original-Received: from mail01.iro.umontreal.ca (unknown [172.31.2.1]) by pmg2.iro.umontreal.ca (Proxmox) with ESMTP id E6AD1801B5; Tue, 16 Aug 2022 16:57:08 -0400 (EDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=iro.umontreal.ca; s=mail; t=1660683428; bh=oJRq6eWkAEAQ3Z3IRHu9bsO2h7v3oRy1bFw3z2iNGz8=; h=From:To:Cc:Subject:In-Reply-To:References:Date:From; b=GRHfYvzy5rZ3mVlCccfmAqMO2OHzU1ngHqwl1X+FjjWOJi2WCEyEquss/+l/nv5eG GFH9VjEa2EMD7H25rDkjB2NSDYsDd12UE7BEq/dD2FgVgywMZH9ntIlfZ6JZ8lkxK8 pgGR174uoScJXbG4RhPL8MGNShUQykdXpuxJYsKS7uretzRAaDy7CxLx+jNn41spOI TJwOktWjPHTimCfnWmSJLj89efDSwzToAiEXL3ALhbSu8RWQTUowmHkPObemKAdMlH FoMbBKtPDeFpulPT9D+lIdNcQVogpRDSJobFF6RwBBerH3LDjhZAijILozpfWPRzBx ygN6VEhAdo1Xw== Original-Received: from alfajor (unknown [45.44.229.252]) by mail01.iro.umontreal.ca (Postfix) with ESMTPSA id AD54612023A; Tue, 16 Aug 2022 16:57:08 -0400 (EDT) In-Reply-To: <325f95fd2b8688c3aa74@heytings.org> (Gregory Heytings's message of "Tue, 16 Aug 2022 19:39:21 +0000") 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" Xref: news.gmane.io gmane.emacs.bugs:240008 Archived-At: Gregory Heytings [2022-08-16 19:39:21] wrote: >> [ Sorry, Gregory. But see below for more concrete/constructive >> criticism. ] > Perhaps I misuderstood or overlooked something in what you said > earlier, but to me your objection to that part of the code until now > was mainly/purely "philosophical". My *main* objection is in the not-overridable part of the design, yes (i.e. philosophical). The rest is just a minor question of coding. >> - The locking is global: any `widen` or `narrow-to-region` will fail when >> a locked narrowing is in effect, even when it's applied to an unrelated >> buffer. Again, it looks like a plain bug, but... > It's not global, given that it is controlled by symbol that is bound within > a function called inside a specific buffer. Which buffer is current when the let-binding is installed has no influence on the behavior because the variable is not buffer-local (i.e. it's global, which is why the effect is global). The dynamic scope means that the effect is limited *in time* (and only applies to the current thread, admittedly), but it's still global in the sense that it affects all the code that is run during this time, no matter where it is executed (or were it was written). But, yes, dynamically scoped variables are often described as being "locally bound", even though the effect of that binding is global in the sense that it applies everywhere while active, so there's plenty of opportunities for confusion with this terminology :-( > But indeed, if inside that > function we use narrow-to-region/widen in another buffer, that would fail. > It seems easy to correct that bug, though: if > (current_buffer->long_line_optimizations_p && ! NILP (Vrestrictions_locked)) If `current_buffer` is not the one that's currently being narrowed by redisplay, then `->long_line_optimizations_p` doesn't say that `current_buffer` has been narrowed by "us", so such a fix might work but I think a better fix is to `Fmake_local_variable` before calling `specbind` so that it really states clearly that this lock applies only to the narrowing installed in this particular buffer. >> - The doc for `restrictions-locked` says: >> >> This happens when `narrow-to-region', which see, is called from Lisp >> with an optional argument LOCK non-nil. >> >> but the `narrow-to-region` function doesn't have any such LOCK argument. > > That should have been removed, indeed. In a previous iteration of the code > a LOCK argument was added to that function, and later removed. Adding such a LOCK argument might be a good idea, actually, since ELisp packages may also want to impose restrictions on widening (see my recent message to `emacs-devel` about ELisp support for locked narrowing). >> - The `narrow_to_region_internal` function, when called with `lock=true` >> oddly includes the functionality of save-restriction to restore the >> restriction and point but not to restore the `restrictions-locked` state. > > This I do not understand. The comment above that function explains that it > should be followed by "specbind (Qrestrictions_locked, Qt)". There is no > need to restore the restrictions-locked state, or am > I misunderstanding something? Why leave the `specbind (Qrestrictions_locked, Qt)` out of the function when you've already included the 3 calls to `record_unwind_protect`? The 4 share the same requirements (because `specbind` pushes on the `specpdl`, just like `record_unwind_protect`). They naturally belong together. Also, a function that sometimes pushes to `specpdl` (and hence requires the caller to do the `SPECPDL_INDEX + unbind_to` dance) and sometimes not, depending on some value of its argument is a bit unnatural (it complicates the reasoning when we try and convince ourselves that the stack discipline of the specpdl is obeyed). So I'd recommend you split the function into two: a first one that only does the narrowing itself and a second one which calls the first plus unconditionally does the `specbind+record_unwind_protect`. >> - In that same function the two branches of the `if (lock)` have a lot of >> obvious redundancy. > Not "a lot", but indeed there's some redundancy there that could have > been avoided. AFAICT it's more than half of the code of each branch. Stefan