bug#56682: locked narrowing (was: bug#56682: Fix the long lines font locking related slowdowns)

[Stefan Monnier]

Eli Zaretskii [2022-08-16 20:26:23] wrote:
>> From: Stefan Monnier <monnier@iro.umontreal.ca>
>> Cc: dgutov@yandex.ru,  56682@debbugs.gnu.org,  gregory@heytings.org
>> Date: Tue, 16 Aug 2022 13:15:04 -0400
>> Eli Zaretskii [2022-08-16 18:51:41] wrote:
>> >> I missed it then: you're saying that I can make nlinum-mode override the
>> >> locked narrowing to find the real current line number without having to
>> >> recompile Emacs?
>> > No, I'm saying that you can test improvements in nlinum-mode that
>> > would make it work better with long lines without recompiling.
>> How?
> I'm sure you can figure it out if you just think for a few seconds.

I have no idea where to start because I can't see any way to circumvent
the locked narrowing.

> But if you want me to do it for you, then please tell which changes do
> you want to try and in what situations, because the technique depends
> on that.

I'd start with the patch below.

> (Not that I understand where are you going with this.  Especially
> since a feature to unlock the narrowing is just one simple commit
> away.)

I really appreciate Gregory's (and your) work to improve the handling of
long lines.  This was way overdue.  *BUT* the code for the locked
narrowing just .... sucks rocks.
[ Sorry, Gregory.  But see below for more concrete/constructive criticism.  ]

So, given that you (Eli) accepted it into `master` and have not voiced
concerns over it even after Dmitry's and my complaints, I feel that you
do support it enough that I'm not in a position to just install
"one simple commit".

The most glaring problems I see with it (beside the impossibility to
override the locking, obviously):

- `widen` will silently fail.  Maybe this is the right thing to do,
  maybe not, but it's definitely not clear (e.g. it leads to inf-loops
  in CC-mode, which doesn't seem much better than the excruciating
  slowdowns that locked narrowing is intended to fix).  It *should* be
  (or have been) discussed.
- Not only you can't `widen` but you can't narrow either
  (`narrow-to-region` just does nothing when the restriction is locked,
  regardless if the call tries to grow or shrink the restriction).
  AFAIK this is a plain bug, but given the fundamental disagreement
  about what is "right" in this discussion, I can't be sure.
- 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...
- ...the plural in the name "restrictions-locked" suggests that the global
  effect is on purpose, tho I can't see any justification for it.
- 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.
- 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.
- In that same function the two branches of the `if (lock)` have a lot
  of obvious redundancy.

So, yes, I could go and write a patch which I think fixes those
problems, but I don't understand how this code made it to `master` in
the first place, so I feel like I'm missing something and can't be sure
what is considered a bug and what is considered a feature.


        Stefan


diff --git a/nlinum.el b/nlinum.el
index 4f0e02fef1..3feaaca5c3 100644
--- a/nlinum.el
+++ b/nlinum.el
@@ -312,7 +312,7 @@ Only works right if point is at BOL."
   (if nlinum-widen
       (save-excursion
         (save-restriction
-          (widen)
+          (REALLY-widen)
           (forward-line 0)              ;In case (point-min) was not at BOL.
           (let ((nlinum-widen nil))
             (nlinum--line-number-at-pos))))