GNU bug report logs - #56682
Fix the long lines font locking related slowdowns

Previous Next

Full log

View this message in rfc822 format

From: Gregory Heytings <gregory <at> heytings.org>
To: Stefan Monnier <monnier <at> iro.umontreal.ca>
Cc: 56682 <at> debbugs.gnu.org, Eli Zaretskii <eliz <at> gnu.org>, dgutov <at> yandex.ru
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.

Previous Next

GNU bug tracking system
Copyright (C) 1999 Darren O. Benham, 1997,2003 nCipher Corporation Ltd, 1994-97 Ian Jackson.