bug-gnu-emacs
[Top][All Lists]
Advanced

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

bug#56682: locked narrowing (was: bug#56682: Fix the long lines font loc


From: Gregory Heytings
Subject: bug#56682: locked narrowing (was: bug#56682: Fix the long lines font locking related slowdowns)
Date: Tue, 16 Aug 2022 19:39:21 +0000



[ Sorry, Gregory. But see below for more concrete/constructive criticism. ]


There's no need to be sorry. This is WIP, any (constructive) criticism is welcome. And I do not understand why you did not raise some of the points below earlier. 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".


- 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.


This (allowing narrow-to-region and widen to work within the bounds of the locked narrowing) is currently being implemented, after an off-list discussion with Alan.


- 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. 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)) ...


- 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.


- 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?


- 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.

I'll fix those problems soon.





reply via email to

[Prev in Thread] Current Thread [Next in Thread]