|
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.
[Prev in Thread] | Current Thread | [Next in Thread] |