Re: feature/improved-locked-narrowing 9dee6df39c: Reworked locked narrowing.

Re: feature/improved-locked-narrowing 9dee6df39c: Reworked locked narrowing.

[Stefan Monnier]

>     Reworked locked narrowing.
>     
>     * src/editfns.c: (narrowing_locks): New alist to hold the narrowing
>     locks and their buffers.
>     (narrowing_lock_get_bound, narrowing_lock_peek_tag)
>     (narrowing_lock_push, narrowing_lock_pop): New functions to access
>     and update 'narrowing_locks'.
>     (reset_outermost_narrowings, unwind_reset_outermost_narrowing):
>     Functions moved from src/xdisp.c, and rewritten with the above
>     functions.
>     (Fwiden): Use the above functions. Update docstring.
>     (Fnarrow_to_region, Fnarrowing_lock, Fnarrowing_unlock): Use the above
>     functions.
>     (syms_of_editfns): Remove the 'narrowing-locks' variable.
>     
>     * src/lisp.h: Make 'reset_outermost_narrowings' externally visible.
>     
>     * src/xdisp.c (reset_outermost_narrowings)
>     unwind_reset_outermost_narrowing): Functions moved to src/editfns.c.
>     
>     * lisp/subr.el (with-locked-narrowing): Improved macro, with a helper
>     function.

[ Sorry for coming late.  ]
This commit (which is now on emacs-29 branch) still has some problems:

- I don't see any mention of those new features in etc/NEWS.
- I can't think of a case where a piece of code could/should make use of
  `narrowing-unlock`.
- In `nlinum--line-number-at-pos`, I want to circumvent the narrowing
  lock due to long lines (since its performance is only affected by
  buffer size but not by line length), but I can't see how to do that.
  The only primitive I see which would let me get out of the locked
  narrowing is `narrowing-unlock` but:
  - I don't know which tag(s) to use.
  - I don't want to remove the lock, I only want to circumvent it
    temporarily, and I can't see how to re-install a lock after
    removing it (`narrowing-lock` doesn't fit the bill because I don't
    know the bounds of the lock and because that would not preserve the
    stacking order of locks).


-- Stefan

bug#56682: feature/improved-locked-narrowing 9dee6df39c: Reworked locked narrowing.

[Gregory Heytings]

[Moved to the bug tracker.]

Thanks for your comments, Stefan!

>
> - I don't see any mention of those new features in etc/NEWS.
>

Yes, that's something I still need to do.

>
> - I can't think of a case where a piece of code could/should make use of 
> `narrowing-unlock`.
>

I don't understand what you mean by that.  'narrowing-lock' locks a 
narrowing, and is used e.g. in the 'with-narrowing' macro. 
'narrowing-unlock' can be used to unlock a narrowing.

>
> - In `nlinum--line-number-at-pos`, I want to circumvent the narrowing 
> lock due to long lines (since its performance is only affected by buffer 
> size but not by line length), but I can't see how to do that. The only 
> primitive I see which would let me get out of the locked narrowing is 
> `narrowing-unlock` but:
>
> - I don't know which tag(s) to use.
>

That depends on the place where 'nlinum--line-number-at-pos' is called, or 
IOW where/by which function the narrowing was locked, or IOW again which 
tag was used to unlock the narrowing.

>
> - I don't want to remove the lock, I only want to circumvent it 
> temporarily, and I can't see how to re-install a lock after removing it 
> (`narrowing-lock` doesn't fit the bill because I don't know the bounds 
> of the lock and because that would not preserve the stacking order of 
> locks).
>

That's part of the improvements you suggested, and I implemented.  Simply 
do a:

(save-restriction
   (narrowing-unlock 'the-appropriate-tag)
   (widen)
   ... your code ...)

Upon return from the save-restriction, the narrowing locks are restored.

bug#56682: feature/improved-locked-narrowing 9dee6df39c: Reworked locked narrowing.

[Gregory Heytings]

>
> That depends on the place where 'nlinum--line-number-at-pos' is called, 
> or IOW where/by which function the narrowing was locked, or IOW again 
> which tag was used to unlock the narrowing.
>

Sorry for the typo: I meant "which tag was used to _lock_ the narrowing".

bug#56682: feature/improved-locked-narrowing 9dee6df39c: Reworked locked narrowing.

[Stefan Monnier via Bug reports for GNU Emacs, the Swiss army knife of text editors]

>> - I don't see any mention of those new features in etc/NEWS.
> Yes, that's something I still need to do.

I think it's quite urgent (it's all too easy to forget: better a subpar
entry than no entry at all).

>> - In `nlinum--line-number-at-pos`, I want to circumvent the narrowing lock
>> due to long lines (since its performance is only affected by buffer size
>> but not by line length), but I can't see how to do that. The only
>> primitive I see which would let me get out of the locked narrowing is
>> `narrowing-unlock` but:
>>
>> - I don't know which tag(s) to use.
>
> That depends on the place where 'nlinum--line-number-at-pos' is called, or
> IOW where/by which function the narrowing was locked, or IOW again which tag
> was used to unlock the narrowing.

Hmm... I understand that part but that doesn't tell me how I (the coder)
or the function can find out which tag to use :-(

>> - I don't want to remove the lock, I only want to circumvent it
>> temporarily, and I can't see how to re-install a lock after removing it
>> (`narrowing-lock` doesn't fit the bill because I don't know the bounds of
>> the lock and because that would not preserve the stacking order of locks).
>
> That's part of the improvements you suggested, and I implemented.
> Simply do a:
>
> (save-restriction
>   (narrowing-unlock 'the-appropriate-tag)
>   (widen)
>   ... your code ...)
>
> Upon return from the save-restriction, the narrowing locks are restored.

Ah, great, I had missed that, thanks.
So the remaining issue is to find the (set of) tags to unlock.

Looking more at the code, I have another question: why is
`narrowing_locks` a global alist indexed by buffers, instead of being
a buffer-local variable?


        Stefan

bug#56682: feature/improved-locked-narrowing 9dee6df39c: Reworked locked narrowing.

[Gregory Heytings]

>
> I think it's quite urgent (it's all too easy to forget: better a subpar 
> entry than no entry at all).
>

I'll do that very soon.

>
> Hmm... I understand that part but that doesn't tell me how I (the coder) 
> or the function can find out which tag to use :-(
>

You have to know where your function is called and by which caller a lock 
was placed.  Currently there are three such tags (and I don't expect more 
tags in core): fontification-functions, pre-command-hook and 
post-command-hook.

>
> Looking more at the code, I have another question: why is 
> `narrowing_locks` a global alist indexed by buffers, instead of being a 
> buffer-local variable?
>

For efficiency reasons.  If it were a buffer-local variable, 
reset_outermost_narrowings, which is called by redisplay_internal, would 
have to consider all buffers, which would become unnecessarily slow with 
many (say 1000) buffers.  With a global alist, only the (few) buffers in 
which narrowing locks are actually in effect are considered.

bug#56682: feature/improved-locked-narrowing 9dee6df39c: Reworked locked narrowing.

[Stefan Monnier via Bug reports for GNU Emacs, the Swiss army knife of text editors]

> You have to know where your function is called and by which caller
> a lock was placed.

That doesn't seem sufficient, because you need to additionally find the
name of the tag they used.  AFAICT currently you can only find that out
by looking at the code.  I think the names should be documented
somewhere else than just in the code itself.

> Currently there are three such tags (and I don't expect more
> tags in core): fontification-functions, pre-command-hook and
> post-command-hook.

These names seem wrong: I don't think the tag should say when/where the
lock happened to be placed, but what caused it to be placed.  IIUC all
three cases up there share the same reason: overly long lines.
So I think they should all use the same tag which could be something
like `long-lines` or maybe they should use names such as
`long-line-threshold` and `large-hscroll-threshold` to record precisely
the cause for the lock.

>> Looking more at the code, I have another question: why is
>> `narrowing_locks` a global alist indexed by buffers, instead of being
>> a buffer-local variable?
>
> For efficiency reasons.  If it were a buffer-local variable,
> reset_outermost_narrowings, which is called by redisplay_internal, would
> have to consider all buffers, which would become unnecessarily slow with
> many (say 1000) buffers.  With a global alist, only the (few) buffers in
> which narrowing locks are actually in effect are considered.

Then I suggest you put a comment to that effect in the code.  I also
can't understand why we need `xdisp.c` to call
`reset_outermost_narrowings`.  I see you tried to explain it in the
doc-comment of the function but I failed to understand the explanation.
Maybe you could extend the comment by pointing to a very concrete
example (or maybe a discussion on the mailing list)?

Another problem I see with it is that it seems to presume a very
particular use of locked narrowing (such as the one installed by the
long-lines code), whereas other uses of locked narrowing might not want
to be reset during redisplay.

Similarly the doc-comment of `narrowing_lock_get_bound` talks about
"bounds ... that are visible on display", but that function doesn't know
what bounds are visible on display, actually.  The interaction with
what's visible on display completely depends on when/where it's called
and when/where locks are installed.  Could you try and clarify the
doc-comment to say what the function actually does, and then separately
explain how it *may* relate with "what's visible on display" and under
which assumption this relation may hold?
IIUC what the function does when OUTERMOST is true is return the
narrowing that was in effect when the first lock was installed, right?


        Stefan

bug#56682: feature/improved-locked-narrowing 9dee6df39c: Reworked locked narrowing.

[Eli Zaretskii]

> Cc: 56682@debbugs.gnu.org
> Date: Fri, 30 Dec 2022 13:51:19 -0500
> From:  Stefan Monnier via "Bug reports for GNU Emacs,
>  the Swiss army knife of text editors" <bug-gnu-emacs@gnu.org>
> 
> > You have to know where your function is called and by which caller
> > a lock was placed.
> 
> That doesn't seem sufficient, because you need to additionally find the
> name of the tag they used.  AFAICT currently you can only find that out
> by looking at the code.  I think the names should be documented
> somewhere else than just in the code itself.
> 
> > Currently there are three such tags (and I don't expect more
> > tags in core): fontification-functions, pre-command-hook and
> > post-command-hook.
> 
> These names seem wrong: I don't think the tag should say when/where the
> lock happened to be placed, but what caused it to be placed.  IIUC all
> three cases up there share the same reason: overly long lines.
> So I think they should all use the same tag which could be something
> like `long-lines` or maybe they should use names such as
> `long-line-threshold` and `large-hscroll-threshold` to record precisely
> the cause for the lock.
> 
> >> Looking more at the code, I have another question: why is
> >> `narrowing_locks` a global alist indexed by buffers, instead of being
> >> a buffer-local variable?
> >
> > For efficiency reasons.  If it were a buffer-local variable,
> > reset_outermost_narrowings, which is called by redisplay_internal, would
> > have to consider all buffers, which would become unnecessarily slow with
> > many (say 1000) buffers.  With a global alist, only the (few) buffers in
> > which narrowing locks are actually in effect are considered.
> 
> Then I suggest you put a comment to that effect in the code.  I also
> can't understand why we need `xdisp.c` to call
> `reset_outermost_narrowings`.  I see you tried to explain it in the
> doc-comment of the function but I failed to understand the explanation.
> Maybe you could extend the comment by pointing to a very concrete
> example (or maybe a discussion on the mailing list)?
> 
> Another problem I see with it is that it seems to presume a very
> particular use of locked narrowing (such as the one installed by the
> long-lines code), whereas other uses of locked narrowing might not want
> to be reset during redisplay.
> 
> Similarly the doc-comment of `narrowing_lock_get_bound` talks about
> "bounds ... that are visible on display", but that function doesn't know
> what bounds are visible on display, actually.  The interaction with
> what's visible on display completely depends on when/where it's called
> and when/where locks are installed.  Could you try and clarify the
> doc-comment to say what the function actually does, and then separately
> explain how it *may* relate with "what's visible on display" and under
> which assumption this relation may hold?
> IIUC what the function does when OUTERMOST is true is return the
> narrowing that was in effect when the first lock was installed, right?

Gregory, any progress with documenting this?

bug#56682: feature/improved-locked-narrowing 9dee6df39c: Reworked locked narrowing.

[Gregory Heytings]

>
> Gregory, any progress with documenting this?
>

It's on my plate for this week.

bug#56682: feature/improved-locked-narrowing 9dee6df39c: Reworked locked narrowing.

[Eli Zaretskii]

> Date: Sat, 14 Jan 2023 21:38:57 +0000
> From: Gregory Heytings <gregory@heytings.org>
> cc: Stefan Monnier <monnier@iro.umontreal.ca>, 56682@debbugs.gnu.org
> 
> > Gregory, any progress with documenting this?
> 
> It's on my plate for this week.

Any progress with this issue?  I'd like to have this resolved before
we make the first pretest of Emacs 29, please.

Thanks.

bug#56682: feature/improved-locked-narrowing 9dee6df39c: Reworked locked narrowing.

[Gregory Heytings]

>
> Any progress with this issue?  I'd like to have this resolved before we 
> make the first pretest of Emacs 29, please.
>
> Thanks.
>

I'm working on this, hopefully it will be finished tomorrow.

bug#56682: feature/improved-locked-narrowing 9dee6df39c: Reworked locked narrowing.

[Eli Zaretskii]

> Date: Sat, 28 Jan 2023 15:11:21 +0000
> From: Gregory Heytings <gregory@heytings.org>
> cc: monnier@iro.umontreal.ca, 56682@debbugs.gnu.org
> 
> > Any progress with this issue?  I'd like to have this resolved before we 
> > make the first pretest of Emacs 29, please.
> >
> > Thanks.
> 
> I'm working on this, hopefully it will be finished tomorrow.

Thanks.

bug#56682: feature/improved-locked-narrowing 9dee6df39c: Reworked locked narrowing.

[Gregory Heytings]

After spending another two days on this, I concluded that TRT is to remove 
that feature from Emacs 29.

Are you okay with that?

Stefan, any comments?

bug#56682: feature/improved-locked-narrowing 9dee6df39c: Reworked locked narrowing.

[Eli Zaretskii]

> Date: Mon, 30 Jan 2023 09:00:23 +0000
> From: Gregory Heytings <gregory@heytings.org>
> cc: 56682@debbugs.gnu.org, monnier@iro.umontreal.ca
> 
> 
> After spending another two days on this, I concluded that TRT is to remove 
> that feature from Emacs 29.
> 
> Are you okay with that?

What exactly will that remove? only what's on the feature branch, or
also some stuff on the emacs-29 branch?

And what are your reasons for removing this?  It is hard to tell
whether or not I agree without knowing to what I should agree ;-)

bug#56682: feature/improved-locked-narrowing 9dee6df39c: Reworked locked narrowing.

[Stefan Monnier via Bug reports for GNU Emacs, the Swiss army knife of text editors]

> After spending another two days on this, I concluded that TRT is to remove
> that feature from Emacs 29.

Remove what feature?  You mean removing the ability to override
the lock?  I wouldn't be happy with that, no.


        Stefan

bug#56682: feature/improved-locked-narrowing 9dee6df39c: Reworked locked narrowing.

[Gregory Heytings]

>> After spending another two days on this, I concluded that TRT is to 
>> remove that feature from Emacs 29.
>>
>> Are you okay with that?
>
> What exactly will that remove? only what's on the feature branch, or 
> also some stuff on the emacs-29 branch?
>

The "locked narrowing" feature, IOW, the portions of editfns.c in which it 
is implemented, and its use around pre-command-hook, post-command-hook and 
fontification-functions.

>
> And what are your reasons for removing this?  It is hard to tell whether 
> or not I agree without knowing to what I should agree ;-)
>

The reason is that I'm now convinced that it is not a good solution to the 
problem of ill-behaving modes in the presence of long lines.

bug#56682: feature/improved-locked-narrowing 9dee6df39c: Reworked locked narrowing.

[Gregory Heytings]

>> After spending another two days on this, I concluded that TRT is to 
>> remove that feature from Emacs 29.
>
> Remove what feature?  You mean removing the ability to override the 
> lock?  I wouldn't be happy with that, no.
>

No, I mean the "locked narrowing" feature.

bug#56682: feature/improved-locked-narrowing 9dee6df39c: Reworked locked narrowing.

[Stefan Monnier via Bug reports for GNU Emacs, the Swiss army knife of text editors]

>>> After spending another two days on this, I concluded that TRT is to
>>> remove that feature from Emacs 29.
>> Remove what feature?  You mean removing the ability to override the lock?
>> I wouldn't be happy with that, no.
> No, I mean the "locked narrowing" feature.

Ah, I'd be OK with that.


        Stefan

bug#56682: feature/improved-locked-narrowing 9dee6df39c: Reworked locked narrowing.

[Eli Zaretskii]

> Date: Mon, 30 Jan 2023 15:03:54 +0000
> From: Gregory Heytings <gregory@heytings.org>
> cc: 56682@debbugs.gnu.org, monnier@iro.umontreal.ca
> 
> > What exactly will that remove? only what's on the feature branch, or 
> > also some stuff on the emacs-29 branch?
> 
> The "locked narrowing" feature, IOW, the portions of editfns.c in which it 
> is implemented, and its use around pre-command-hook, post-command-hook and 
> fontification-functions.
> 
> >
> > And what are your reasons for removing this?  It is hard to tell whether 
> > or not I agree without knowing to what I should agree ;-)
> >
> 
> The reason is that I'm now convinced that it is not a good solution to the 
> problem of ill-behaving modes in the presence of long lines.

So we are removing all the stuff that prevented font-lock from slowing
down redisplay when long lines are in the buffer?  IOW, something
which we have for several months, and which so far brought up only one
complaint?  Frankly, this makes no sense to me, unless we delay the
pretest for another half year or so.  It's too late for such changes.

Or am I missing something?

bug#56682: feature/improved-locked-narrowing 9dee6df39c: Reworked locked narrowing.

[Juri Linkov]

>> > And what are your reasons for removing this?  It is hard to tell whether
>> > or not I agree without knowing to what I should agree ;-)
>>
>> The reason is that I'm now convinced that it is not a good solution to the
>> problem of ill-behaving modes in the presence of long lines.
>
> So we are removing all the stuff that prevented font-lock from slowing
> down redisplay when long lines are in the buffer?  IOW, something
> which we have for several months, and which so far brought up only one
> complaint?  Frankly, this makes no sense to me, unless we delay the
> pretest for another half year or so.  It's too late for such changes.
>
> Or am I missing something?

It would be sad to lose this helpful feature, but there are too many
problems with weird behavior in buffers with long-line-optimizations.

For example, when there is at least one long line near the end of
a diff-mode buffer, then sometimes the buffer gets truncated,
'diff-hunk-file-names' returns nil on diff headings,
and there are a lot of such errors in the Messages buffer:

  Error during redisplay: (jit-lock-function 1) signaled (args-out-of-range #<buffer *vc-diff*> 1118 1370)
  ...

Maybe it's still possible to fix this?

bug#56682: feature/improved-locked-narrowing 9dee6df39c: Reworked locked narrowing.

[Eli Zaretskii]

> From: Juri Linkov <juri@linkov.net>
> Cc: Gregory Heytings <gregory@heytings.org>,  56682@debbugs.gnu.org,
>   monnier@iro.umontreal.ca
> Date: Mon, 30 Jan 2023 19:24:56 +0200
> 
> For example, when there is at least one long line near the end of
> a diff-mode buffer, then sometimes the buffer gets truncated,
> 'diff-hunk-file-names' returns nil on diff headings,
> and there are a lot of such errors in the Messages buffer:
> 
>   Error during redisplay: (jit-lock-function 1) signaled (args-out-of-range #<buffer *vc-diff*> 1118 1370)
>   ...
> 
> Maybe it's still possible to fix this?

We can and should fix this and other problems, but only if they are
reported.  Would you please file a bug report for any such problem you
find?

bug#56682: feature/improved-locked-narrowing 9dee6df39c: Reworked locked narrowing.

[Juri Linkov]

>> For example, when there is at least one long line near the end of
>> a diff-mode buffer, then sometimes the buffer gets truncated,
>> 'diff-hunk-file-names' returns nil on diff headings,
>> and there are a lot of such errors in the Messages buffer:
>>
>>   Error during redisplay: (jit-lock-function 1) signaled (args-out-of-range #<buffer *vc-diff*> 1118 1370)
>>   ...
>>
>> Maybe it's still possible to fix this?
>
> We can and should fix this and other problems, but only if they are
> reported.  Would you please file a bug report for any such problem you
> find?

I noticed this problem only in one commit last week, but didn't realize
it's caused by long lines.  I had time to debug it only today.
Now could try to create a minimal test case.

bug#56682: feature/improved-locked-narrowing 9dee6df39c: Reworked locked narrowing.

[Eli Zaretskii]

> From: Juri Linkov <juri@linkov.net>
> Cc: gregory@heytings.org,  56682@debbugs.gnu.org,  monnier@iro.umontreal.ca
> Date: Mon, 30 Jan 2023 19:56:44 +0200
> 
> I noticed this problem only in one commit last week, but didn't realize
> it's caused by long lines.  I had time to debug it only today.
> Now could try to create a minimal test case.

Thanks, please do.

bug#56682: feature/improved-locked-narrowing 9dee6df39c: Reworked locked narrowing.

[Dmitry Gutov]

On 30/01/2023 19:11, Eli Zaretskii wrote:
>> Date: Mon, 30 Jan 2023 15:03:54 +0000
>> From: Gregory Heytings<gregory@heytings.org>
>> cc:56682@debbugs.gnu.org,monnier@iro.umontreal.ca
>>
>>> What exactly will that remove? only what's on the feature branch, or
>>> also some stuff on the emacs-29 branch?
>> The "locked narrowing" feature, IOW, the portions of editfns.c in which it
>> is implemented, and its use around pre-command-hook, post-command-hook and
>> fontification-functions.
>>
>>> And what are your reasons for removing this?  It is hard to tell whether
>>> or not I agree without knowing to what I should agree 😉
>>>
>> The reason is that I'm now convinced that it is not a good solution to the
>> problem of ill-behaving modes in the presence of long lines.
> So we are removing all the stuff that prevented font-lock from slowing
> down redisplay when long lines are in the buffer?  IOW, something
> which we have for several months, and which so far brought up only one
> complaint?

More than one, FTR.

I'm not 100% sure what Gregory's plan is, but note that a significant 
part of the redisplay slowdowns wasn't caused by font-lock, and thus the 
removal of locking will not regress those performance improvements.

And for those (potential?) cases where font-lock is a real problem for 
performance, we could stop it from widening using an existing knob.

bug#56682: feature/improved-locked-narrowing 9dee6df39c: Reworked locked narrowing.

[Eli Zaretskii]

> Date: Mon, 30 Jan 2023 20:56:15 +0200
> Cc: 56682@debbugs.gnu.org, monnier@iro.umontreal.ca
> From: Dmitry Gutov <dgutov@yandex.ru>
> 
> On 30/01/2023 19:11, Eli Zaretskii wrote:
> >> Date: Mon, 30 Jan 2023 15:03:54 +0000
> >> From: Gregory Heytings<gregory@heytings.org>
> >> cc:56682@debbugs.gnu.org,monnier@iro.umontreal.ca
> >>
> >>> What exactly will that remove? only what's on the feature branch, or
> >>> also some stuff on the emacs-29 branch?
> >> The "locked narrowing" feature, IOW, the portions of editfns.c in which it
> >> is implemented, and its use around pre-command-hook, post-command-hook and
> >> fontification-functions.
> >>
> >>> And what are your reasons for removing this?  It is hard to tell whether
> >>> or not I agree without knowing to what I should agree 😉
> >>>
> >> The reason is that I'm now convinced that it is not a good solution to the
> >> problem of ill-behaving modes in the presence of long lines.
> > So we are removing all the stuff that prevented font-lock from slowing
> > down redisplay when long lines are in the buffer?  IOW, something
> > which we have for several months, and which so far brought up only one
> > complaint?
> 
> More than one, FTR.
> 
> I'm not 100% sure what Gregory's plan is, but note that a significant 
> part of the redisplay slowdowns wasn't caused by font-lock, and thus the 
> removal of locking will not regress those performance improvements.
> 
> And for those (potential?) cases where font-lock is a real problem for 
> performance, we could stop it from widening using an existing knob.

Yes, this is an old and known disagreement between us.  But this is my
decision to make, not anyone else's.

bug#56682: feature/improved-locked-narrowing 9dee6df39c: Reworked locked narrowing.

[Dmitry Gutov]

On 30/01/2023 21:02, Eli Zaretskii wrote:
>> Date: Mon, 30 Jan 2023 20:56:15 +0200
>> Cc:56682@debbugs.gnu.org,monnier@iro.umontreal.ca
>> From: Dmitry Gutov<dgutov@yandex.ru>
>>
>> On 30/01/2023 19:11, Eli Zaretskii wrote:
>>>> Date: Mon, 30 Jan 2023 15:03:54 +0000
>>>> From: Gregory Heytings<gregory@heytings.org>
>>>> cc:56682@debbugs.gnu.org,monnier@iro.umontreal.ca
>>>>
>>>>> What exactly will that remove? only what's on the feature branch, or
>>>>> also some stuff on the emacs-29 branch?
>>>> The "locked narrowing" feature, IOW, the portions of editfns.c in which it
>>>> is implemented, and its use around pre-command-hook, post-command-hook and
>>>> fontification-functions.
>>>>
>>>>> And what are your reasons for removing this?  It is hard to tell whether
>>>>> or not I agree without knowing to what I should agree 😉
>>>>>
>>>> The reason is that I'm now convinced that it is not a good solution to the
>>>> problem of ill-behaving modes in the presence of long lines.
>>> So we are removing all the stuff that prevented font-lock from slowing
>>> down redisplay when long lines are in the buffer?  IOW, something
>>> which we have for several months, and which so far brought up only one
>>> complaint?
>> More than one, FTR.
>>
>> I'm not 100% sure what Gregory's plan is, but note that a significant
>> part of the redisplay slowdowns wasn't caused by font-lock, and thus the
>> removal of locking will not regress those performance improvements.
>>
>> And for those (potential?) cases where font-lock is a real problem for
>> performance, we could stop it from widening using an existing knob.
> Yes, this is an old and known disagreement between us.  But this is my
> decision to make, not anyone else's.

With the previous change that Gregory had made (different limits for 
different features, i.e. font-lock uses much wider narrowing bounds 
which are also customizable) I don't have that much of a horse in that 
race anymore.

Just helping clear up some misconceptions, if any.

Also note that previously, I'd never have suggested that redisplay code 
changes the value of font-lock-dont-widen.

bug#56682: feature/improved-locked-narrowing 9dee6df39c: Reworked locked narrowing.

[Stefan Monnier via Bug reports for GNU Emacs, the Swiss army knife of text editors]

> Also note that previously, I'd never have suggested that redisplay code
>  changes the value of font-lock-dont-widen.

It would be ugly for redisplay code to mess with font-lock data like
`font-lock-dont-widen`.  Instead, it should let-bind its own variable to
indicate that the long-lines code has currently installed a narrowing,
and `font-lock.el` can decide whether to pay attention to that var.


        Stefan

bug#56682: feature/improved-locked-narrowing 9dee6df39c: Reworked locked narrowing.

[Dmitry Gutov]

On 30/01/2023 23:49, Stefan Monnier wrote:
>> Also note that previously, I'd never have suggested that redisplay code
>>   changes the value of font-lock-dont-widen.
> It would be ugly for redisplay code to mess with font-lock data like
> `font-lock-dont-widen`.  Instead, it should let-bind its own variable to
> indicate that the long-lines code has currently installed a narrowing,
> and `font-lock.el` can decide whether to pay attention to that var.

That does sound better, but either way, the net effect would (probably) 
be that by default font-lock obeys the added narrowing, but can escape 
it for special purposes with no trouble.

bug#56682: feature/improved-locked-narrowing 9dee6df39c: Reworked locked narrowing.

[Eli Zaretskii]

> Date: Mon, 30 Jan 2023 23:07:22 +0200
> Cc: 56682@debbugs.gnu.org, gregory@heytings.org, monnier@iro.umontreal.ca
> From: Dmitry Gutov <dgutov@yandex.ru>
> 
> With the previous change that Gregory had made (different limits for 
> different features, i.e. font-lock uses much wider narrowing bounds 
> which are also customizable) I don't have that much of a horse in that 
> race anymore.

Which change is that?  I don't think I follow.

> Also note that previously, I'd never have suggested that redisplay code 
> changes the value of font-lock-dont-widen.

Hmm... not sure how is that variable related to this particular
thread.  Please elaborate.

bug#56682: feature/improved-locked-narrowing 9dee6df39c: Reworked locked narrowing.

[Gregory Heytings]

>
> So we are removing all the stuff that prevented font-lock from slowing 
> down redisplay when long lines are in the buffer?  IOW, something which 
> we have for several months, and which so far brought up only one 
> complaint?  Frankly, this makes no sense to me, unless we delay the 
> pretest for another half year or so.  It's too late for such changes.
>
> Or am I missing something?
>

I looked with a critical eye at the code I wrote, and concluded that the 
cure is worse than the disease.

It's true that some slowdowns caused by font-locking are prevented by 
locked narrowing, but:

1. The slowdowns caused by font-locking were not the major cause of 
slowdowns in buffers with long lines.  They were the "last step" to make 
editing operations in such buffers as fast as possible, and my opinion now 
is that that last step was a step too far.  Efficiency issues in font 
locking routines are the responsibility of mode authors.  Efficiency 
issues in the redisplay routines are the responsibility of Emacs, and have 
been dealt with.

2. Locked narrowing does not prevent all slowdowns caused by font-locking.

3. Locked narrowing can create slowdowns (e.g. infinite loops) that do not 
exist when it is not present.

4. Mode authors who do care about efficiency will update their modes to 
deal with problematic buffers, and do not need to be incited by locked 
narrowing to do so.

5. Mode authors who do not care about such problematic buffers will simply 
replace calls to "(widen)" by "(narrowing-unlock 'fontification-functions) 
(widen)" in their code, and the situation will not have improved.

bug#56682: feature/improved-locked-narrowing 9dee6df39c: Reworked locked narrowing.

[Gregory Heytings]

>
> note that a significant part of the redisplay slowdowns wasn't caused by 
> font-lock, and thus the removal of locking will not regress those 
> performance improvements.
>

That is correct.

>
> And for those (potential?) cases where font-lock is a real problem for 
> performance, we could stop it from widening using an existing knob.
>

That is not correct.  There is no knob that can prevent a mode from 
widening.

bug#56682: feature/improved-locked-narrowing 9dee6df39c: Reworked locked narrowing.

[Dmitry Gutov]

On 31/01/2023 14:14, Eli Zaretskii wrote:
>> Date: Mon, 30 Jan 2023 23:07:22 +0200
>> Cc: 56682@debbugs.gnu.org, gregory@heytings.org, monnier@iro.umontreal.ca
>> From: Dmitry Gutov <dgutov@yandex.ru>
>>
>> With the previous change that Gregory had made (different limits for
>> different features, i.e. font-lock uses much wider narrowing bounds
>> which are also customizable) I don't have that much of a horse in that
>> race anymore.
> 
> Which change is that?  I don't think I follow.

The addition of long-line-locked-narrowing-region-size, I think, 
separated the narrowing radius for font-lock from narrowing for the 
improvement of other display stuff. I could be wrong there, though.

>> Also note that previously, I'd never have suggested that redisplay code
>> changes the value of font-lock-dont-widen.
> 
> Hmm... not sure how is that variable related to this particular
> thread.  Please elaborate.

If we drop the locking feature, the display engine could still apply 
narrowing around fontification functions (and hooks, and etc). And 
perhaps bind font-lock-down-widen to t. Or add a new variable, like 
Stefan suggested -- that would be tidier.

That would still not protect us from misbehaving modes, but modes that 
absolutely cannot work within a narrowing are a problem anyway.

And for the rest, we have an existing convention (discussed at length 
and rubber-stamped by the previous maintainer back then) that major 
modes shouldn't call 'widen' in font-lock-keywords. Nor in indentation 
code. That was part of the convention which made js-mode, python-mode, 
etc, work better with multil-major-mode packages, and it did.

bug#56682: feature/improved-locked-narrowing 9dee6df39c: Reworked locked narrowing.

[Dmitry Gutov]

On 31/01/2023 17:17, Gregory Heytings wrote:
> 
>>
>> note that a significant part of the redisplay slowdowns wasn't caused 
>> by font-lock, and thus the removal of locking will not regress those 
>> performance improvements.
>>
> 
> That is correct.
> 
>>
>> And for those (potential?) cases where font-lock is a real problem for 
>> performance, we could stop it from widening using an existing knob.
>>
> 
> That is not correct.  There is no knob that can prevent a mode from 
> widening.

Just the existing convention which implores modes not to do that.

CC Mode will continue to do what it does, I guess.

bug#56682: feature/improved-locked-narrowing 9dee6df39c: Reworked locked narrowing.

[Dmitry Gutov]

On 31/01/2023 17:14, Gregory Heytings wrote:
> 
>>
>> So we are removing all the stuff that prevented font-lock from slowing 
>> down redisplay when long lines are in the buffer?  IOW, something 
>> which we have for several months, and which so far brought up only one 
>> complaint?  Frankly, this makes no sense to me, unless we delay the 
>> pretest for another half year or so.  It's too late for such changes.
>>
>> Or am I missing something?
>>
> 
> I looked with a critical eye at the code I wrote, and concluded that the 
> cure is worse than the disease.
> 
> It's true that some slowdowns caused by font-locking are prevented by 
> locked narrowing, but:
> 
> 1. The slowdowns caused by font-locking were not the major cause of 
> slowdowns in buffers with long lines.  They were the "last step" to make 
> editing operations in such buffers as fast as possible, and my opinion 
> now is that that last step was a step too far.  Efficiency issues in 
> font locking routines are the responsibility of mode authors.  
> Efficiency issues in the redisplay routines are the responsibility of 
> Emacs, and have been dealt with.

FWIW, I happy you reached that POV.

> 2. Locked narrowing does not prevent all slowdowns caused by font-locking.
> 
> 3. Locked narrowing can create slowdowns (e.g. infinite loops) that do 
> not exist when it is not present.

This could be a pretty good argument. When testing in larger files, I 
did notice some performance kinks I could not easily explain, nor stably 
reproduce.

Speaking of additional slowdowns, though, at least in theory -- both 
syntax-ppss and tree-sitter drop their current parse result when 
point-min changes. So it an already fully parsed buffer (which applies 
more often to tree-sitter, though), scrolling through the buffer and 
fontifying should cause caches to drop to be recomputed again when the 
narrowing changes. Though that particular effect might not be too 
noticeable, since those features are rather fast in a 50000 region.

bug#56682: feature/improved-locked-narrowing 9dee6df39c: Reworked locked narrowing.

[Stefan Monnier via Bug reports for GNU Emacs, the Swiss army knife of text editors]

> Speaking of additional slowdowns, though, at least in theory -- both
> syntax-ppss and tree-sitter drop their current parse result when point-min
> changes.

FWIW, `syntax-ppss` keeps 2 caches: one for point-min=1 and one for the
other cases.  So narrowing doesn't quite "drop the current parse", it
just temporarily ignores it, but recovers it when we exit the narrowing.


        Stefan

bug#56682: feature/improved-locked-narrowing 9dee6df39c: Reworked locked narrowing.

[Dmitry Gutov]

On 31/01/2023 23:46, Stefan Monnier wrote:
>> Speaking of additional slowdowns, though, at least in theory -- both
>> syntax-ppss and tree-sitter drop their current parse result when point-min
>> changes.
> FWIW, `syntax-ppss` keeps 2 caches: one for point-min=1 and one for the
> other cases.  So narrowing doesn't quite "drop the current parse", it
> just temporarily ignores it, but recovers it when we exit the narrowing.

Take the scenario for scrolling through a whole file. When the file 
reaches a certain length, the current scheme is to narrow, linearly, to 
successive text chunks of size N, and fontify them.

And since for every chunk point-min will have a new value, it will have 
to re-parse the text from the beginning of the chunk. Even if the cache 
exists already for the whole (unrestricted) buffer.

And even if syntax-ppss-max-span is lower than N (which it currently is 
by default, 20000 vs 500000).

bug#56682: feature/improved-locked-narrowing 9dee6df39c: Reworked locked narrowing.

[Eli Zaretskii]

> Date: Tue, 31 Jan 2023 15:14:14 +0000
> From: Gregory Heytings <gregory@heytings.org>
> cc: 56682@debbugs.gnu.org, monnier@iro.umontreal.ca
> 
> > So we are removing all the stuff that prevented font-lock from slowing 
> > down redisplay when long lines are in the buffer?  IOW, something which 
> > we have for several months, and which so far brought up only one 
> > complaint?  Frankly, this makes no sense to me, unless we delay the 
> > pretest for another half year or so.  It's too late for such changes.
> >
> > Or am I missing something?
> 
> I looked with a critical eye at the code I wrote, and concluded that the 
> cure is worse than the disease.
> 
> It's true that some slowdowns caused by font-locking are prevented by 
> locked narrowing, but:
> 
> 1. The slowdowns caused by font-locking were not the major cause of 
> slowdowns in buffers with long lines.  They were the "last step" to make 
> editing operations in such buffers as fast as possible, and my opinion now 
> is that that last step was a step too far.  Efficiency issues in font 
> locking routines are the responsibility of mode authors.  Efficiency 
> issues in the redisplay routines are the responsibility of Emacs, and have 
> been dealt with.
> 
> 2. Locked narrowing does not prevent all slowdowns caused by font-locking.
> 
> 3. Locked narrowing can create slowdowns (e.g. infinite loops) that do not 
> exist when it is not present.
> 
> 4. Mode authors who do care about efficiency will update their modes to 
> deal with problematic buffers, and do not need to be incited by locked 
> narrowing to do so.
> 
> 5. Mode authors who do not care about such problematic buffers will simply 
> replace calls to "(widen)" by "(narrowing-unlock 'fontification-functions) 
> (widen)" in their code, and the situation will not have improved.

I must admit that it's very strange to hear this from you.  Those very
opinions were voiced by several other people over the last year, and
you always disagreed with these very arguments in the strongest terms,
citing various use cases and data points, with several relevant files
and modes.  I wonder what have happened that you now see in the code
what you didn't see then, even when others told you they saw it.

Anyway, after thinking about this, I cannot agree to removing what we
now have on emacs-29.  It's too late for that, and I'm not prepared to
delay the pretest for any significant time.  So we will go into the
pretest with what we have, and decide what to do with it in Emacs 29.2
and 30.1 according to how the chips will fall.

I guess by suggesting to remove the code you were also telling me that
you won't be fixing those documentation issues, which you promised to
work on a month ago?  If so, I guess this is one more buck that stops
with me...

bug#56682: feature/improved-locked-narrowing 9dee6df39c: Reworked locked narrowing.

[dick]

EZ> which promised to work on a month ago?

What's with the butt-hurt?  Narrowing guy just spent two whole days unto
the breach, and made a heroic concession, one which I could never make.
Of everyone here, you most of all should empathize with "code blinders,"
or every programmer's inclination to marshall arguments, however flimsy,
to preserve man-months of one's own work.

bug#56682: feature/improved-locked-narrowing 9dee6df39c: Reworked locked narrowing.

[Gregory Heytings]

>
> I must admit that it's very strange to hear this from you.
>

When I erred, I have no problems whatsoever to admit that I erred.

>
> Those very opinions were voiced by several other people over the last 
> year, and you always disagreed with these very arguments in the 
> strongest terms, citing various use cases and data points, with several 
> relevant files and modes.  I wonder what have happened that you now see 
> in the code what you didn't see then, even when others told you they saw 
> it.
>

I don't think it's useful to start a discussion about what I said and 
meant to say, but FTR I do not agree with the above.  The only thing I 
strongly disagreed with is Dmitry's claim that the existing mechanisms are 
sufficient to limit the damage of ill-behaving modes.  For the rest, I 
tried to find the best compromise to handle buffers with long lines as 
well as possible.  As we all know, determining where to place the cursor 
is hard.  I did believe that adding a forced narrowing would be helpful, 
and in some cases it is, but in other cases it isn't, and all in all I now 
consider that Emacs would be in a better shape without that mechanism. 
In particular, the fifth point is the result of the (recent) addition of a 
mechanism to unlock a locked narrowing, which makes the exercise almost 
pointless.

>
> Anyway, after thinking about this, I cannot agree to removing what we 
> now have on emacs-29.  It's too late for that, and I'm not prepared to 
> delay the pretest for any significant time.  So we will go into the 
> pretest with what we have, and decide what to do with it in Emacs 29.2 
> and 30.1 according to how the chips will fall.
>

Okay, if that's your decision.  IMO just going back to what the code did 
earlier is (very) safe, but I cannot do that without your approval.

>
> I guess by suggesting to remove the code you were also telling me that 
> you won't be fixing those documentation issues, which you promised to 
> work on a month ago?  If so, I guess this is one more buck that stops 
> with me...
>

Even if I think it's not TRT to do, I can still do that.

bug#56682: feature/improved-locked-narrowing 9dee6df39c: Reworked locked narrowing.

[Eli Zaretskii]

> Date: Wed, 01 Feb 2023 22:42:56 +0000
> From: Gregory Heytings <gregory@heytings.org>
> cc: 56682@debbugs.gnu.org, monnier@iro.umontreal.ca
> 
> > Anyway, after thinking about this, I cannot agree to removing what we 
> > now have on emacs-29.  It's too late for that, and I'm not prepared to 
> > delay the pretest for any significant time.  So we will go into the 
> > pretest with what we have, and decide what to do with it in Emacs 29.2 
> > and 30.1 according to how the chips will fall.
> 
> Okay, if that's your decision.  IMO just going back to what the code did 
> earlier is (very) safe, but I cannot do that without your approval.

It might be safe or it might not be safe.  We will not know for sure
without some period of people using that.  And I'm unwilling to delay
the pretest by any significant time, sorry.

> > I guess by suggesting to remove the code you were also telling me that 
> > you won't be fixing those documentation issues, which you promised to 
> > work on a month ago?  If so, I guess this is one more buck that stops 
> > with me...
> 
> Even if I think it's not TRT to do, I can still do that.

Then please do.  This is one of a small number of issues whose
resolution delays the pretest ATM.  Which is why I pinged you several
times over the last month about it.

TIA

bug#56682: feature/improved-locked-narrowing 9dee6df39c: Reworked locked narrowing.

[Gregory Heytings]

>> Okay, if that's your decision.  IMO just going back to what the code 
>> did earlier is (very) safe, but I cannot do that without your approval.
>
> It might be safe or it might not be safe.  We will not know for sure 
> without some period of people using that.  And I'm unwilling to delay 
> the pretest by any significant time, sorry.
>

All the code that deals with long lines is carefully placed inside 
conditionals.  What I'm proposing is strictly equivalent to the following 
patch:

diff --git a/src/keyboard.c b/src/keyboard.c
index 6f0f075e54e..888a5d075b2 100644
--- a/src/keyboard.c
+++ b/src/keyboard.c
@@ -1909,7 +1909,7 @@ safe_run_hooks_maybe_narrowed (Lisp_Object hook, 
struct window *w)

    specbind (Qinhibit_quit, Qt);

-  if (current_buffer->long_line_optimizations_p
+  if (0 /* current_buffer->long_line_optimizations_p */
        && long_line_locked_narrowing_region_size > 0)
      {
        ptrdiff_t begv = get_locked_narrowing_begv (PT);
diff --git a/src/xdisp.c b/src/xdisp.c
index d2c91e5847b..4ed9c858cd6 100644
--- a/src/xdisp.c
+++ b/src/xdisp.c
@@ -4393,7 +4393,7 @@ handle_fontified_prop (struct it *it)

        eassert (it->end_charpos == ZV);

-      if (current_buffer->long_line_optimizations_p
+      if (0 /* current_buffer->long_line_optimizations_p */
           && long_line_locked_narrowing_region_size > 0)
         {
           ptrdiff_t begv = it->locked_narrowing_begv;

By definition, this cannot be unsafe for any buffer, except those in which 
long_line_optimizations_p is set.  So the worst that could possibly happen 
is that Emacs would hang in some buffers in which 
long_line_optimizations_p is set, which is not a regression: Emacs 28 and 
earlier would have hanged under the same conditions.

>
> Then please do.  This is one of a small number of issues whose 
> resolution delays the pretest ATM.  Which is why I pinged you several 
> times over the last month about it.
>

I know.  Unfortunately (or perhaps fortunately?) my time to hack on Emacs 
is limited.

bug#56682: feature/improved-locked-narrowing 9dee6df39c: Reworked locked narrowing.

[Eli Zaretskii]

> Date: Fri, 03 Feb 2023 00:20:40 +0000
> From: Gregory Heytings <gregory@heytings.org>
> cc: 56682@debbugs.gnu.org, monnier@iro.umontreal.ca
> 
> By definition, this cannot be unsafe for any buffer, except those in which 
> long_line_optimizations_p is set.  So the worst that could possibly happen 
> is that Emacs would hang in some buffers in which 
> long_line_optimizations_p is set, which is not a regression: Emacs 28 and 
> earlier would have hanged under the same conditions.

Given the fanfare we announced the solution to long-line problems, the
above is precisely the definition of "not safe" from my POV.

> > Then please do.  This is one of a small number of issues whose 
> > resolution delays the pretest ATM.  Which is why I pinged you several 
> > times over the last month about it.
> 
> I know.  Unfortunately (or perhaps fortunately?) my time to hack on Emacs 
> is limited.

I'm sorry I have to insist, but does this mean you will be able to fix
these documentation issues in less than a week?  I don't think I can
wait any longer than that.

TIA

bug#56682: feature/improved-locked-narrowing 9dee6df39c: Reworked locked narrowing.

[Gregory Heytings]

>> By definition, this cannot be unsafe for any buffer, except those in 
>> which long_line_optimizations_p is set.  So the worst that could 
>> possibly happen is that Emacs would hang in some buffers in which 
>> long_line_optimizations_p is set, which is not a regression: Emacs 28 
>> and earlier would have hanged under the same conditions.
>
> Given the fanfare we announced the solution to long-line problems, the 
> above is precisely the definition of "not safe" from my POV.
>

We never announced that all possible problems in such buffers would be 
solved.  In fact, the NEWS entry explicitly mentions that slowdowns are 
still possible because of minor or major modes, and contains instructions 
to gradually disable minor and major modes when such problems happen.

My preference is still to remove that feature.

>
> I'm sorry I have to insist, but does this mean you will be able to fix 
> these documentation issues in less than a week?  I don't think I can 
> wait any longer than that.
>

I'll do that in less than a week, after hearing your final verdict, if you 
decide that it should not be removed.

bug#56682: feature/improved-locked-narrowing 9dee6df39c: Reworked locked narrowing.

[Eli Zaretskii]

> Date: Fri, 03 Feb 2023 22:12:39 +0000
> From: Gregory Heytings <gregory@heytings.org>
> cc: 56682@debbugs.gnu.org, monnier@iro.umontreal.ca
> 
> > I'm sorry I have to insist, but does this mean you will be able to fix 
> > these documentation issues in less than a week?  I don't think I can 
> > wait any longer than that.
> >
> 
> I'll do that in less than a week, after hearing your final verdict, if you 
> decide that it should not be removed.

It will not be removed in Emacs 29.1, given what we know now.

Thanks in advance for handling the documentation update.

bug#56682: feature/improved-locked-narrowing 9dee6df39c: Reworked locked narrowing.

[Gregory Heytings]

As promised, I worked on this.  Alas, while doing that I found out that 
the existing code had a grave bug, which I corrected.  However, this 
touches src/bytecode.c, lisp/emacs-lisp/bytecomp.el and src/comp.c, so I'm 
(to say the least) uncomfortable pushing that fix to emacs-29.  The result 
of the bug fix, a few renames, the documentation update, and an added 
test, is in scratch/fix-locked-narrowing.  I did a make bootstrap and a 
make check after each step, all of them succeeded.

bug#56682: feature/improved-locked-narrowing 9dee6df39c: Reworked locked narrowing.

[Eli Zaretskii]

> Date: Thu, 09 Feb 2023 01:57:50 +0000
> From: Gregory Heytings <gregory@heytings.org>
> cc: monnier@iro.umontreal.ca, 56682@debbugs.gnu.org
> 
> 
> As promised, I worked on this.  Alas, while doing that I found out that 
> the existing code had a grave bug, which I corrected.  However, this 
> touches src/bytecode.c, lisp/emacs-lisp/bytecomp.el and src/comp.c, so I'm 
> (to say the least) uncomfortable pushing that fix to emacs-29.  The result 
> of the bug fix, a few renames, the documentation update, and an added 
> test, is in scratch/fix-locked-narrowing.  I did a make bootstrap and a 
> make check after each step, all of them succeeded.

Thanks.

Please describe the bug you discovered and the changes made to fix it
(and maybe also alternatives if you considered them).  It is not easy
to glean that information from the changes, as they involve also
renaming of variables (which, at this late stage, I don't think you
should have done without discussion, btw), documentation changes, and
perhaps more.

Also, did you test this both with and without native-compilation?

Andrea, could you please see if the changes on that branch are okay as
far as native-compilation is concerned?

bug#56682: feature/improved-locked-narrowing 9dee6df39c: Reworked locked narrowing.

[Gregory Heytings]

>
> Please describe the bug you discovered and the changes made to fix it 
> (and maybe also alternatives if you considered them).
>

The bug is that narrowing locks were not saved and restored by 
save-restriction as they were supposed to be.  AFAIK there are no 
alternatives to what I did.

I did on purpose separate that bugfix (commit 1 on the branch) from the 
renaming ones (commits 2-5), the documentation one (commit 6) and the one 
that adds a test (commit 7) to make the reviewing task easier.

>
> Also, did you test this both with and without native-compilation?
>

Yes, without and with native-compilation.  Though with native-compilation 
I only checked with the complete changeset.

>
> Andrea, could you please see if the changes on that branch are okay as 
> far as native-compilation is concerned?
>

Note that the only commit you should check is 7691a84d2a.

bug#56682: feature/improved-locked-narrowing 9dee6df39c: Reworked locked narrowing.

[Eli Zaretskii]

> Date: Thu, 09 Feb 2023 10:33:34 +0000
> From: Gregory Heytings <gregory@heytings.org>
> cc: Andrea Corallo <akrl@sdf.org>, monnier@iro.umontreal.ca, 
>     56682@debbugs.gnu.org
> 
> I did on purpose separate that bugfix (commit 1 on the branch) from the 
> renaming ones (commits 2-5), the documentation one (commit 6) and the one 
> that adds a test (commit 7) to make the reviewing task easier.

OK, thanks.

There's one thing I cannot seem to be able to find in the
documentation you added: how can a Lisp program know that it is being
run under a "labeled narrowing", and in particular what is the label?
Without knowing that, how can Lisp programs adapt their behavior to
this special kind of narrowing, or even just break out of it using
without-narrowing?

bug#56682: feature/improved-locked-narrowing 9dee6df39c: Reworked locked narrowing.

[Gregory Heytings]

Thanks for your feedback.

>
> There's one thing I cannot seem to be able to find in the documentation 
> you added: how can a Lisp program know that it is being run under a 
> "labeled narrowing",
>

A function/macro to check that could indeed be added, its body would be:

(save-restriction (widen) (buffer-narrowed-p))

>
> and in particular what is the label? Without knowing that, how can Lisp 
> programs adapt their behavior to this special kind of narrowing, or even 
> just break out of it using without-narrowing?
>

That information should be given in the docstring of the function that 
creates that narrowing.  It is now present in the docstrings of the three 
hooks where it is used:

pre-command-hook and post-command-hook: ... these functions are called as 
if they were in a `with-narrowing' form, with a 
`long-line-optimizations-in-command-hooks' label ...

fontification-functions: ... these functions are called as if they were in 
a `with-narrowing' form, with a 
`long-line-optimizations-in-fontification-functions' label ...

bug#56682: feature/improved-locked-narrowing 9dee6df39c: Reworked locked narrowing.

[Eli Zaretskii]

> Date: Thu, 09 Feb 2023 14:39:39 +0000
> From: Gregory Heytings <gregory@heytings.org>
> cc: akrl@sdf.org, monnier@iro.umontreal.ca, 56682@debbugs.gnu.org
> 
> > There's one thing I cannot seem to be able to find in the documentation 
> > you added: how can a Lisp program know that it is being run under a 
> > "labeled narrowing",
> 
> A function/macro to check that could indeed be added, its body would be:
> 
> (save-restriction (widen) (buffer-narrowed-p))

We should add it and document it, but I'm surprised that there's no
easier way.  One problem with the above is that it could cause a more
thorough redisplay because it fiddles with buffer restrictions.

Also, this doesn't return the label itself.

> > and in particular what is the label? Without knowing that, how can Lisp 
> > programs adapt their behavior to this special kind of narrowing, or even 
> > just break out of it using without-narrowing?
> 
> That information should be given in the docstring of the function that 
> creates that narrowing.  It is now present in the docstrings of the three 
> hooks where it is used:

Yes, but how do I know which one of these is in effect when my
function is called?

bug#56682: feature/improved-locked-narrowing 9dee6df39c: Reworked locked narrowing.

[Gregory Heytings]

>>> There's one thing I cannot seem to be able to find in the 
>>> documentation you added: how can a Lisp program know that it is being 
>>> run under a "labeled narrowing",
>>
>> A function/macro to check that could indeed be added, its body would 
>> be:
>>
>> (save-restriction (widen) (buffer-narrowed-p))
>
> We should add it and document it, but I'm surprised that there's no 
> easier way.  One problem with the above is that it could cause a more 
> thorough redisplay because it fiddles with buffer restrictions.
>

If necessary, a specific function which does not widen could be added to 
do that, using narrowing_lock_peek_tag:

DEFUN ("...", ...) (void)
{
   if (NILP (narrowing_lock_peek_tag (Fcurrent_buffer ())))
     return Qnil;
   else
     return Qt;
}

>
> Also, this doesn't return the label itself.
>

Indeed, but returning the label would defeat the purpose of the tool.  If 
a program can by itself determine that it is in a labeled narrowing and 
get its label, it can escape that narrowing without much ado.

>> That information should be given in the docstring of the function that 
>> creates that narrowing.  It is now present in the docstrings of the 
>> three hooks where it is used:
>
> Yes, but how do I know which one of these is in effect when my function 
> is called?
>

The only way is to look at the call tree of the function.

(Note that I'm writing the above to "defend" a feature that I still 
believe should better be removed from Emacs.)

bug#56682: feature/improved-locked-narrowing 9dee6df39c: Reworked locked narrowing.

[Eli Zaretskii]

> Date: Thu, 09 Feb 2023 16:11:36 +0000
> From: Gregory Heytings <gregory@heytings.org>
> cc: akrl@sdf.org, monnier@iro.umontreal.ca, 56682@debbugs.gnu.org
> 
> >> (save-restriction (widen) (buffer-narrowed-p))
> >
> > We should add it and document it, but I'm surprised that there's no 
> > easier way.  One problem with the above is that it could cause a more 
> > thorough redisplay because it fiddles with buffer restrictions.
> >
> 
> If necessary, a specific function which does not widen could be added to 
> do that, using narrowing_lock_peek_tag:
> 
> DEFUN ("...", ...) (void)
> {
>    if (NILP (narrowing_lock_peek_tag (Fcurrent_buffer ())))
>      return Qnil;
>    else
>      return Qt;
> }

I don't see how we can get away without doing something like that.
And I think it should return the tag itself, not just a boolean flag.

> > Also, this doesn't return the label itself.
> >
> 
> Indeed, but returning the label would defeat the purpose of the tool.  If 
> a program can by itself determine that it is in a labeled narrowing and 
> get its label, it can escape that narrowing without much ado.

We make a point of documenting these labeled narrowings in great
detail, including the labels and their effects.  It makes no sense to
describe them and not let programs adapt their behaviors to these
situations.  Without access to the label, the without-narrowing macro
is not useful, so why provide such a macro at all if we really don't
want Lisp programs to use it?  I don't want to assume malice on behalf
of Lisp programs, I want to give well-meaning programs reasonable
means to adapt their behaviors to these narrowings.  If and when we
see these facilities abused, we can take whatever actions are
necessary at that time.  But leaving legitimate programs limited
because we fear less legitimate programs will abuse them is not TRT in
my book.

> (Note that I'm writing the above to "defend" a feature that I still 
> believe should better be removed from Emacs.)

I'm not sure what to do with this remark.  I'm not accusing you of
anything, so you don't need to justify or defend yourself.  We are
discussing what should this feature include assuming that it will be
in Emacs.

bug#56682: feature/improved-locked-narrowing 9dee6df39c: Reworked locked narrowing.

[Stefan Monnier via Bug reports for GNU Emacs, the Swiss army knife of text editors]

>> A function/macro to check that could indeed be added, its body would be:
>> 
>> (save-restriction (widen) (buffer-narrowed-p))
>
> We should add it and document it, but I'm surprised that there's no
> easier way.  One problem with the above is that it could cause a more
> thorough redisplay because it fiddles with buffer restrictions.

Seeing how `buffer-narrowed-p` is not used very often, I'm not
too worried.  Even more so given that the redisplay does try to notice
when the restriction has been modified temporarily but "reset" to its
original value since then.


        Stefan

bug#56682: feature/improved-locked-narrowing 9dee6df39c: Reworked locked narrowing.

[Eli Zaretskii]

> From: Stefan Monnier <monnier@iro.umontreal.ca>
> Cc: Gregory Heytings <gregory@heytings.org>,  akrl@sdf.org,
>   56682@debbugs.gnu.org
> Date: Thu, 09 Feb 2023 12:31:44 -0500
> 
> >> A function/macro to check that could indeed be added, its body would be:
> >> 
> >> (save-restriction (widen) (buffer-narrowed-p))
> >
> > We should add it and document it, but I'm surprised that there's no
> > easier way.  One problem with the above is that it could cause a more
> > thorough redisplay because it fiddles with buffer restrictions.
> 
> Seeing how `buffer-narrowed-p` is not used very often, I'm not
> too worried.  Even more so given that the redisplay does try to notice
> when the restriction has been modified temporarily but "reset" to its
> original value since then.

The last part is only true for Lisp that's called from redisplay
itself, not for Lisp running between redisplay cycles.

bug#56682: feature/improved-locked-narrowing 9dee6df39c: Reworked locked narrowing.

[Juri Linkov]

>> >> (save-restriction (widen) (buffer-narrowed-p))
>> >
>> > We should add it and document it, but I'm surprised that there's no
>> > easier way.  One problem with the above is that it could cause a more
>> > thorough redisplay because it fiddles with buffer restrictions.
>>
>> If necessary, a specific function which does not widen could be added to
>> do that, using narrowing_lock_peek_tag:
>>
>> DEFUN ("...", ...) (void)
>> {
>>    if (NILP (narrowing_lock_peek_tag (Fcurrent_buffer ())))
>>      return Qnil;
>>    else
>>      return Qt;
>> }
>
> I don't see how we can get away without doing something like that.
> And I think it should return the tag itself, not just a boolean flag.

'buffer-narrowed-p' could return different symbols(tags) for narrowing lock.

bug#56682: feature/improved-locked-narrowing 9dee6df39c: Reworked locked narrowing.

[Juri Linkov]

>>> A function/macro to check that could indeed be added, its body would be:
>>> 
>>> (save-restriction (widen) (buffer-narrowed-p))
>>
>> We should add it and document it, but I'm surprised that there's no
>> easier way.  One problem with the above is that it could cause a more
>> thorough redisplay because it fiddles with buffer restrictions.
>
> Seeing how `buffer-narrowed-p` is not used very often, I'm not
> too worried.  Even more so given that the redisplay does try to notice
> when the restriction has been modified temporarily but "reset" to its
> original value since then.

It caused a problem in easy-mmode-define-navigation reported in bug#61215.

bug#56682: feature/improved-locked-narrowing 9dee6df39c: Reworked locked narrowing.

[Gregory Heytings]

>> Indeed, but returning the label would defeat the purpose of the tool. 
>> If a program can by itself determine that it is in a labeled narrowing 
>> and get its label, it can escape that narrowing without much ado.
>
> We make a point of documenting these labeled narrowings in great detail, 
> including the labels and their effects.  It makes no sense to describe 
> them and not let programs adapt their behaviors to these situations. 
> Without access to the label, the without-narrowing macro is not useful, 
> so why provide such a macro at all if we really don't want Lisp programs 
> to use it?
>

Why do you think it's not useful?  It is meant to be used for example in a 
function that is called in fontification-functions to temporarily escape 
the narrowing set around fontification-functions, when its author has 
determined that it is safe to do that, like this:

(without-narrowing
   :label long-line-optimizations-in-fontification-functions
   ... some code ...)

IMO, if it becomes possible to escape the narrowing programmatically (IOW, 
without even checking where and why the narrowing has been set), the 
exercise becomes even more pointless.  (Also, in that case it seems to me 
that there is no reason anymore to label these narrowings: their label 
serves no real purpose.)

>> (Note that I'm writing the above to "defend" a feature that I still 
>> believe should better be removed from Emacs.)
>
> I'm not sure what to do with this remark.  I'm not accusing you of 
> anything, so you don't need to justify or defend yourself.  We are 
> discussing what should this feature include assuming that it will be in 
> Emacs.
>

Don't worry, I don't feel accused at all.  I was only expressing that I'm 
reasoning against my intuition here.

bug#56682: feature/improved-locked-narrowing 9dee6df39c: Reworked locked narrowing.

[Drew Adams]

> (without-narrowing
   ^^^^^^^^^^^^^^^^^
>   ...)

Without entering into this thread's real content...

If we're now adding function/macro names that use
"narrowing" instead of (buffer) "restriction"
(which is OK by me), will you also please create
aliases for all of the existing thingies that use
the old "restriction" names?

It's not good to introduce multiple ways to refer
to the same thing (a narrowing, aka a buffer
restriction), and especially to do that only for
some such names.  Evolution isn't bad, but let's
not lose consistency for no good reason (dunno
whether there's a good reason here).

Currently we have, and for a long time we've had,
(at least) these that I'm aware of:

Functions/macros:

 byte-compile-save-restriction
 org-agenda-set-restriction-lock     (command)
 org-element-restriction 
 org-speedbar-set-agenda-restriction (command)
 save-restriction

Variables:

 byte-save-restriction
 org-element-object-restrictions  
 org-speedbar-restriction-lock-overlay

And I've added this function for my own library:

 isearchp-toggle-region-restriction  (command)

Maybe other 3rd-party code has also added some
such names.

bug#56682: feature/improved-locked-narrowing 9dee6df39c: Reworked locked narrowing.

[Drew Adams]

To be clear: I know there's already a macro with
"narrowing" in the name: `comment-with-narrowing'.

I know of no others, and no functions or vars,
but I don't use/load lots of Emacs libraries.

(And again, I too have named some functions,
macros, and variables with "narrowing".)

bug#56682: feature/improved-locked-narrowing 9dee6df39c: Reworked locked narrowing.

[Eli Zaretskii]

> Date: Thu, 09 Feb 2023 20:47:11 +0000
> From: Gregory Heytings <gregory@heytings.org>
> cc: 56682@debbugs.gnu.org, monnier@iro.umontreal.ca, akrl@sdf.org
> 
> > We make a point of documenting these labeled narrowings in great detail, 
> > including the labels and their effects.  It makes no sense to describe 
> > them and not let programs adapt their behaviors to these situations. 
> > Without access to the label, the without-narrowing macro is not useful, 
> > so why provide such a macro at all if we really don't want Lisp programs 
> > to use it?
> 
> Why do you think it's not useful?  It is meant to be used for example in a 
> function that is called in fontification-functions to temporarily escape 
> the narrowing set around fontification-functions, when its author has 
> determined that it is safe to do that, like this:
> 
> (without-narrowing
>    :label long-line-optimizations-in-fontification-functions
>    ... some code ...)

I'm thinking about code that doesn't necessarily run only from
fontification-functions.  If the code is low-level enough, it is quite
possible that it will be called in many different contexts by many
different callers.  Think about stuff like forward-sexp, and imagine
some major mode where doing that needs for some reason to look far
back in the buffer, or start from the beginning of a line.  Functions
like that cannot know where they are called, and if they want to
modify their behavior depending on the caller's context, they cannot
do it without knowing the label.

> IMO, if it becomes possible to escape the narrowing programmatically (IOW, 
> without even checking where and why the narrowing has been set), the 
> exercise becomes even more pointless.

From where I stand, the locked narrowing is there to prevent slowdown
due to Lisp programs that are either unaware of special long-line
handling, or are deliberately ignoring it.  It is IMO okay for a Lisp
program that is aware of the issue to adapt its behavior by widening
the buffer; if it does so indiscriminately or in a way that hurts
performance, we can consider that a bug and expect the developers to
fix it.

> (Also, in that case it seems to me that there is no reason anymore
> to label these narrowings: their label serves no real purpose.)

I think the label allows different methods of behavior adaptation.  No
one said that being called from fontification-functions calls for the
same behavior adaptation as being called from, say, post-command-hook.

> Don't worry, I don't feel accused at all.  I was only expressing that I'm 
> reasoning against my intuition here.

It is trivially correct that removing some code removes also the need
to discuss it.  This discussion would have not happened if we were to
decide to remove the narrowing in fontificiations and pre/post-command
hooks.

bug#56682: feature/improved-locked-narrowing 9dee6df39c: Reworked locked narrowing.

[Andrea Corallo]

Eli Zaretskii <eliz@gnu.org> writes:

>> Date: Thu, 09 Feb 2023 01:57:50 +0000
>> From: Gregory Heytings <gregory@heytings.org>
>> cc: monnier@iro.umontreal.ca, 56682@debbugs.gnu.org
>> 
>> 
>> As promised, I worked on this.  Alas, while doing that I found out that 
>> the existing code had a grave bug, which I corrected.  However, this 
>> touches src/bytecode.c, lisp/emacs-lisp/bytecomp.el and src/comp.c, so I'm 
>> (to say the least) uncomfortable pushing that fix to emacs-29.  The result 
>> of the bug fix, a few renames, the documentation update, and an added 
>> test, is in scratch/fix-locked-narrowing.  I did a make bootstrap and a 
>> make check after each step, all of them succeeded.
>
> Thanks.
>
> Please describe the bug you discovered and the changes made to fix it
> (and maybe also alternatives if you considered them).  It is not easy
> to glean that information from the changes, as they involve also
> renaming of variables (which, at this late stage, I don't think you
> should have done without discussion, btw), documentation changes, and
> perhaps more.
>
> Also, did you test this both with and without native-compilation?
>
> Andrea, could you please see if the changes on that branch are okay as
> far as native-compilation is concerned?

As far as I can see LGTM.  The branch should be okay as long as it's
bootstrapped and we have no regressions in the testsuite.

Best Regards

  Andrea

bug#56682: feature/improved-locked-narrowing 9dee6df39c: Reworked locked narrowing.

[Gregory Heytings]

>> Why do you think it's not useful?  It is meant to be used for example 
>> in a function that is called in fontification-functions to temporarily 
>> escape the narrowing set around fontification-functions, when its 
>> author has determined that it is safe to do that, like this:
>>
>> (without-narrowing
>>    :label long-line-optimizations-in-fontification-functions
>>    ... some code ...)
>
> I'm thinking about code that doesn't necessarily run only from 
> fontification-functions.  If the code is low-level enough, it is quite 
> possible that it will be called in many different contexts by many 
> different callers.  Think about stuff like forward-sexp, and imagine 
> some major mode where doing that needs for some reason to look far back 
> in the buffer, or start from the beginning of a line.  Functions like 
> that cannot know where they are called, and if they want to modify their 
> behavior depending on the caller's context, they cannot do it without 
> knowing the label.
>

They cannot do that themselves, indeed, but the idea is/was not to do that 
in some low-level code, but around some portions of code at a higher 
level, e.g. around a portion of a fontification function that would need 
for some reason to access the whole buffer (e.g. to check what the first 
few characters of the buffer or of the current line are).

If escaping such narrowings should be possible in some low-level code, it 
is not necessary or useful to return the label.  Instead another macro, 
say 'without-any-narrowings', should be added.  The reason is that 
narrowing locks can be nested, which means that using 'without-narrowing' 
with the label of the innermost such narrowing does not guarantee that the 
whole buffer will be accessible.

>
> From where I stand, the locked narrowing is there to prevent slowdown 
> due to Lisp programs that are either unaware of special long-line 
> handling, or are deliberately ignoring it.  It is IMO okay for a Lisp 
> program that is aware of the issue to adapt its behavior by widening the 
> buffer; if it does so indiscriminately or in a way that hurts 
> performance, we can consider that a bug and expect the developers to fix 
> it.
>

Yes, that is what the without-narrowing macro is for.

>> Don't worry, I don't feel accused at all.  I was only expressing that 
>> I'm reasoning against my intuition here.
>
> It is trivially correct that removing some code removes also the need to 
> discuss it.  This discussion would have not happened if we were to 
> decide to remove the narrowing in fontificiations and pre/post-command 
> hooks.
>

Just in case, so that you can study and try it, I created a 
scratch/remove-locked-narrowing branch, with four commits: commit 1 
deactivates the two places where it is used, commit 2 removes the core 
functions in which it is implemented, commit 3 removes the other functions 
used by the core functions, and commit 4 updates the documentation.

I also fixed another bug in the scratch/fix-locked-narrowing branch.

Again these commits pass make bootstrap and make check, with and without 
native-compilation.

bug#56682: feature/improved-locked-narrowing 9dee6df39c: Reworked locked narrowing.

[Eli Zaretskii]

> From: Andrea Corallo <akrl@sdf.org>
> Cc: Gregory Heytings <gregory@heytings.org>, monnier@iro.umontreal.ca,
>         56682@debbugs.gnu.org
> Date: Fri, 10 Feb 2023 16:46:57 +0000
> 
> Eli Zaretskii <eliz@gnu.org> writes:
> 
> > Andrea, could you please see if the changes on that branch are okay as
> > far as native-compilation is concerned?
> 
> As far as I can see LGTM.  The branch should be okay as long as it's
> bootstrapped and we have no regressions in the testsuite.

OK, thanks.  AFAIU, Gregory did test all that.

So Gregory, please merge the branch to emacs-29, and let's take it
from there.

bug#56682: feature/improved-locked-narrowing 9dee6df39c: Reworked locked narrowing.

[Gregory Heytings]

>
> So Gregory, please merge the branch to emacs-29, and let's take it from 
> there.
>

Now done.

bug#56682: feature/improved-locked-narrowing 9dee6df39c: Reworked locked narrowing.

[Eli Zaretskii]

> Date: Mon, 13 Feb 2023 11:00:09 +0000
> From: Gregory Heytings <gregory@heytings.org>
> cc: Andrea Corallo <akrl@sdf.org>, monnier@iro.umontreal.ca, 
>     56682@debbugs.gnu.org
> 
> > So Gregory, please merge the branch to emacs-29, and let's take it from 
> > there.
> 
> Now done.

Thanks!

bug#56682: feature/improved-locked-narrowing 9dee6df39c: Reworked locked narrowing.

[Eli Zaretskii]

> From: Drew Adams <drew.adams@oracle.com>
> CC: "56682@debbugs.gnu.org" <56682@debbugs.gnu.org>,
>         "monnier@iro.umontreal.ca" <monnier@iro.umontreal.ca>,
>         "akrl@sdf.org"
> 	<akrl@sdf.org>
> Date: Thu, 9 Feb 2023 22:46:02 +0000
> 
> > (without-narrowing
>    ^^^^^^^^^^^^^^^^^
> >   ...)
> 
> Without entering into this thread's real content...
> 
> If we're now adding function/macro names that use
> "narrowing" instead of (buffer) "restriction"
> (which is OK by me), will you also please create
> aliases for all of the existing thingies that use
> the old "restriction" names?

Good point.  I renamed them to with-restriction and
without-restriction, respectively.

bug#56682: feature/improved-locked-narrowing 9dee6df39c: Reworked locked narrowing.

[Gregory Heytings]

Thanks for your changes.

A few other notes:

1. There are three places (indent.c:line_number_display_width, 
xdisp.c:display_count_lines_logically, fileio.c:write_region) where we 
currently use

record_unwind_protect (save_restriction_restore, save_restriction_save ());
Fwiden ();

2. In another place (process.c:Finternal_default_process_filter) we 
currently have:

/* If the output marker is outside of the visible region, save
    the restriction and widen.  */
if (! (BEGV <= PT && PT <= ZV))
   Fwiden ();

(without the "save the restriction" mentioned in the comment).

3. Also, in Ferase_buffer, we do Fwiden to "delete the entire contents of 
the current buffer".

These five places probably need to be adapted to handle labeled narrowings 
correctly.  If you agree with that, I'll write a patch to that effect.

bug#56682: feature/improved-locked-narrowing 9dee6df39c: Reworked locked narrowing.

[Eli Zaretskii]

> Date: Tue, 14 Feb 2023 10:30:13 +0000
> From: Gregory Heytings <gregory@heytings.org>
> cc: 56682@debbugs.gnu.org, monnier@iro.umontreal.ca, akrl@sdf.org
> 
> 1. There are three places (indent.c:line_number_display_width, 
> xdisp.c:display_count_lines_logically, fileio.c:write_region) where we 
> currently use
> 
> record_unwind_protect (save_restriction_restore, save_restriction_save ());
> Fwiden ();

I'm not sure these should be subject to locked narrowing.  They don't
present performance problems with very long lines, AFAIK.

> 2. In another place (process.c:Finternal_default_process_filter) we 
> currently have:
> 
> /* If the output marker is outside of the visible region, save
>     the restriction and widen.  */
> if (! (BEGV <= PT && PT <= ZV))
>    Fwiden ();
> 
> (without the "save the restriction" mentioned in the comment).

This should be fixed to use unwind-protect of some kind, I think, and
that's orthogonal to locked narrowing.

> 3. Also, in Ferase_buffer, we do Fwiden to "delete the entire contents of 
> the current buffer".
> 
> These five places probably need to be adapted to handle labeled narrowings 
> correctly.  If you agree with that, I'll write a patch to that effect.

I'm not sure what you mean by "adapt to".  Please tell more.

bug#56682: feature/improved-locked-narrowing 9dee6df39c: Reworked locked narrowing.

[Gregory Heytings]

>> 1. There are three places (indent.c:line_number_display_width, 
>> xdisp.c:display_count_lines_logically, fileio.c:write_region) where we 
>> currently use
>>
>> record_unwind_protect (save_restriction_restore, save_restriction_save ());
>> Fwiden ();
>
> I'm not sure these should be subject to locked narrowing.  They don't 
> present performance problems with very long lines, AFAIK.
>
>> 2. In another place (process.c:Finternal_default_process_filter) we 
>> currently have:
>>
>> /* If the output marker is outside of the visible region, save
>>     the restriction and widen.  */
>> if (! (BEGV <= PT && PT <= ZV))
>>    Fwiden ();
>>
>> (without the "save the restriction" mentioned in the comment).
>
> This should be fixed to use unwind-protect of some kind, I think, and 
> that's orthogonal to locked narrowing.
>
>> 3. Also, in Ferase_buffer, we do Fwiden to "delete the entire contents 
>> of the current buffer".
>>
>> These five places probably need to be adapted to handle labeled 
>> narrowings correctly.  If you agree with that, I'll write a patch to 
>> that effect.
>
> I'm not sure what you mean by "adapt to".  Please tell more.
>

Fwiden in the functions above (which are AFAICS the only places in Emacs 
where Fwiden is called) is not prepared to the possibility of them being 
called inside a labeled narrowing, either one installed by the long lines 
code, or another one.  Basically we need to use a variant of 
reset_outermost_narrowings (for the current buffer only) where we use

record_unwind_protect (save_restriction_restore, save_restriction_save ());

bug#56682: feature/improved-locked-narrowing 9dee6df39c: Reworked locked narrowing.

[Eli Zaretskii]

> Date: Tue, 14 Feb 2023 14:59:22 +0000
> From: Gregory Heytings <gregory@heytings.org>
> cc: 56682@debbugs.gnu.org, monnier@iro.umontreal.ca, akrl@sdf.org
> 
> 
> >> 1. There are three places (indent.c:line_number_display_width, 
> >> xdisp.c:display_count_lines_logically, fileio.c:write_region) where we 
> >> currently use
> >>
> >> record_unwind_protect (save_restriction_restore, save_restriction_save ());
> >> Fwiden ();
> >
> > I'm not sure these should be subject to locked narrowing.  They don't 
> > present performance problems with very long lines, AFAIK.
> >
> >> 2. In another place (process.c:Finternal_default_process_filter) we 
> >> currently have:
> >>
> >> /* If the output marker is outside of the visible region, save
> >>     the restriction and widen.  */
> >> if (! (BEGV <= PT && PT <= ZV))
> >>    Fwiden ();
> >>
> >> (without the "save the restriction" mentioned in the comment).
> >
> > This should be fixed to use unwind-protect of some kind, I think, and 
> > that's orthogonal to locked narrowing.
> >
> >> 3. Also, in Ferase_buffer, we do Fwiden to "delete the entire contents 
> >> of the current buffer".
> >>
> >> These five places probably need to be adapted to handle labeled 
> >> narrowings correctly.  If you agree with that, I'll write a patch to 
> >> that effect.
> >
> > I'm not sure what you mean by "adapt to".  Please tell more.
> >
> 
> Fwiden in the functions above (which are AFAICS the only places in Emacs 
> where Fwiden is called) is not prepared to the possibility of them being 
> called inside a labeled narrowing, either one installed by the long lines 
> code, or another one.  Basically we need to use a variant of 
> reset_outermost_narrowings (for the current buffer only) where we use
> 
> record_unwind_protect (save_restriction_restore, save_restriction_save ());

Ah, okay.  Please do, and thanks.

bug#56682: feature/improved-locked-narrowing 9dee6df39c: Reworked locked narrowing.

[Gregory Heytings]

[-- Attachment #1: Type: text/plain, Size: 520 bytes --]


>> Fwiden in the functions above (which are AFAICS the only places in 
>> Emacs where Fwiden is called) is not prepared to the possibility of 
>> them being called inside a labeled narrowing, either one installed by 
>> the long lines code, or another one.  Basically we need to use a 
>> variant of reset_outermost_narrowings (for the current buffer only) 
>> where we use
>>
>> record_unwind_protect (save_restriction_restore, save_restriction_save ());
>
> Ah, okay.  Please do, and thanks.
>

And here's the patch!

[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: Remove-narrowing-locks-before-calling-Fwiden.patch --]
[-- Type: text/x-diff; name=Remove-narrowing-locks-before-calling-Fwiden.patch, Size: 4485 bytes --]

From c482389fd59edcbc738a4d48ca6b65d14af60b25 Mon Sep 17 00:00:00 2001
From: Gregory Heytings <gregory@heytings.org>
Date: Tue, 14 Feb 2023 22:41:14 +0000
Subject: [PATCH] Remove narrowing locks before calling Fwiden

* src/editfns.c (narrowing_locks_remove_in_current_buffer) : New
function.

* src/lisp.h: Make it externally visible.

* src/xdisp.c (display_count_lines_logically):
* src/lread.c (readevalloop):
* src/indent.c (line_number_display_width):
* src/fileio.c (write_region):
* src/callproc.c (Fcall_process_region):
* src/buffer.c (Ferase_buffer): Use it.
---
 src/buffer.c   | 1 +
 src/callproc.c | 1 +
 src/editfns.c  | 7 +++++++
 src/fileio.c   | 1 +
 src/indent.c   | 1 +
 src/lisp.h     | 1 +
 src/lread.c    | 1 +
 src/xdisp.c    | 1 +
 8 files changed, 14 insertions(+)

diff --git a/src/buffer.c b/src/buffer.c
index df1f5206668..a9d004d8727 100644
--- a/src/buffer.c
+++ b/src/buffer.c
@@ -2386,6 +2386,7 @@ DEFUN ("erase-buffer", Ferase_buffer, Serase_buffer, 0, 0, "*",
 so the buffer is truly empty after this.  */)
   (void)
 {
+  narrowing_locks_remove_in_current_buffer ();
   Fwiden ();
 
   del_range (BEG, Z);
diff --git a/src/callproc.c b/src/callproc.c
index 5e1e1a8cc0a..c144f6e66d9 100644
--- a/src/callproc.c
+++ b/src/callproc.c
@@ -1113,6 +1113,7 @@ t (mix it with ordinary output), or a file name string.
         {
           /* No need to save restrictions since we delete everything
              anyway.  */
+          narrowing_locks_remove_in_current_buffer ();
           Fwiden ();
           del_range (BEG, Z);
         }
diff --git a/src/editfns.c b/src/editfns.c
index f83c5c7259b..32f654af8c3 100644
--- a/src/editfns.c
+++ b/src/editfns.c
@@ -2814,6 +2814,13 @@ narrowing_locks_restore (Lisp_Object buf_and_saved_locks)
     narrowing_locks_add (buf, saved_locks);
 }
 
+/* Remove all narrowing locks in the current buffer.  */
+void
+narrowing_locks_remove_in_current_buffer (void)
+{
+  narrowing_locks_remove (Fcurrent_buffer ());
+}
+
 static void
 unwind_narrow_to_region_locked (Lisp_Object tag)
 {
diff --git a/src/fileio.c b/src/fileio.c
index f00c389a520..cf5a4ecf80a 100644
--- a/src/fileio.c
+++ b/src/fileio.c
@@ -5269,6 +5269,7 @@ write_region (Lisp_Object start, Lisp_Object end, Lisp_Object filename,
     }
 
   record_unwind_protect (save_restriction_restore, save_restriction_save ());
+  narrowing_locks_remove_in_current_buffer ();
 
   /* Special kludge to simplify auto-saving.  */
   if (NILP (start))
diff --git a/src/indent.c b/src/indent.c
index 6de18d749ca..5f6dc47f034 100644
--- a/src/indent.c
+++ b/src/indent.c
@@ -2065,6 +2065,7 @@ line_number_display_width (struct window *w, int *width, int *pixel_width)
 	{
 	  record_unwind_protect (save_restriction_restore,
 				 save_restriction_save ());
+	  narrowing_locks_remove_in_current_buffer ();
 	  Fwiden ();
 	  saved_restriction = true;
 	}
diff --git a/src/lisp.h b/src/lisp.h
index 1276285e2f2..637ede7051e 100644
--- a/src/lisp.h
+++ b/src/lisp.h
@@ -4689,6 +4689,7 @@ XMODULE_FUNCTION (Lisp_Object o)
 					    ptrdiff_t, bool);
 extern void narrow_to_region_locked (Lisp_Object, Lisp_Object, Lisp_Object);
 extern void reset_outermost_narrowings (void);
+extern void narrowing_locks_remove_in_current_buffer (void);
 extern void init_editfns (void);
 extern void syms_of_editfns (void);
 
diff --git a/src/lread.c b/src/lread.c
index d0dc85f51c8..28ca483b819 100644
--- a/src/lread.c
+++ b/src/lread.c
@@ -2255,6 +2255,7 @@ readevalloop (Lisp_Object readcharfun,
 	  record_unwind_protect_excursion ();
 	  /* Save ZV in it.  */
 	  record_unwind_protect (save_restriction_restore, save_restriction_save ());
+	  narrowing_locks_remove_in_current_buffer ();
 	  /* Those get unbound after we read one expression.  */
 
 	  /* Set point and ZV around stuff to be read.  */
diff --git a/src/xdisp.c b/src/xdisp.c
index 5c5ecaa2bcb..6f2b4e6cd41 100644
--- a/src/xdisp.c
+++ b/src/xdisp.c
@@ -24111,6 +24111,7 @@ display_count_lines_logically (ptrdiff_t start_byte, ptrdiff_t limit_byte,
   ptrdiff_t val;
   specpdl_ref pdl_count = SPECPDL_INDEX ();
   record_unwind_protect (save_restriction_restore, save_restriction_save ());
+  narrowing_locks_remove_in_current_buffer ();
   Fwiden ();
   val = display_count_lines (start_byte, limit_byte, count, byte_pos_ptr);
   unbind_to (pdl_count, Qnil);
-- 
2.39.0

bug#56682: feature/improved-locked-narrowing 9dee6df39c: Reworked locked narrowing.

[Eli Zaretskii]

> Date: Tue, 14 Feb 2023 22:50:24 +0000
> From: Gregory Heytings <gregory@heytings.org>
> cc: 56682@debbugs.gnu.org, monnier@iro.umontreal.ca, akrl@sdf.org
> 
> >> Fwiden in the functions above (which are AFAICS the only places in 
> >> Emacs where Fwiden is called) is not prepared to the possibility of 
> >> them being called inside a labeled narrowing, either one installed by 
> >> the long lines code, or another one.  Basically we need to use a 
> >> variant of reset_outermost_narrowings (for the current buffer only) 
> >> where we use
> >>
> >> record_unwind_protect (save_restriction_restore, save_restriction_save ());
> >
> > Ah, okay.  Please do, and thanks.
> >
> 
> And here's the patch!

Hmm... I'm probably missing something, but doesn't
narrowing_locks_remove permanently removes the lock from the
restriction in the buffer?  And if so, don't we want to restore the
lock after whatever we need to do in a widened buffer is done?  For
example, for displaying line numbers, we don't want to modify anything
in the locked restrictions just because we need to calculate a line
number.

bug#56682: feature/improved-locked-narrowing 9dee6df39c: Reworked locked narrowing.

[Gregory Heytings]

>
> Hmm... I'm probably missing something, but doesn't 
> narrowing_locks_remove permanently removes the lock from the restriction 
> in the buffer?  And if so, don't we want to restore the lock after 
> whatever we need to do in a widened buffer is done?
>

Yes: that's what the "record_unwind_protect (save_restriction_restore, 
save_restriction_save ());" does.  The two places where 
'narrowing_locks_remove_in_current_buffer' is not preceded by that 
statement are the two places where we indeed don't want to restore the 
restrictions after widening: Ferase_buffer and Fcall_process_region.

bug#56682: feature/improved-locked-narrowing 9dee6df39c: Reworked locked narrowing.

[Eli Zaretskii]

> Date: Wed, 15 Feb 2023 13:37:46 +0000
> From: Gregory Heytings <gregory@heytings.org>
> cc: 56682@debbugs.gnu.org, monnier@iro.umontreal.ca, akrl@sdf.org
> 
> > Hmm... I'm probably missing something, but doesn't 
> > narrowing_locks_remove permanently removes the lock from the restriction 
> > in the buffer?  And if so, don't we want to restore the lock after 
> > whatever we need to do in a widened buffer is done?
> 
> Yes: that's what the "record_unwind_protect (save_restriction_restore, 
> save_restriction_save ());" does.

So save_restriction_restore will reinstate the buffer in the
narrowing_locks list?

> The two places where 'narrowing_locks_remove_in_current_buffer' is
> not preceded by that statement are the two places where we indeed
> don't want to restore the restrictions after widening: Ferase_buffer
> and Fcall_process_region.

OK.

bug#56682: feature/improved-locked-narrowing 9dee6df39c: Reworked locked narrowing.

[Gregory Heytings]

>> Yes: that's what the "record_unwind_protect (save_restriction_restore, 
>> save_restriction_save ());" does.
>
> So save_restriction_restore will reinstate the buffer in the 
> narrowing_locks list?
>

Yes:

Lisp_Object
save_restriction_save (void)
{
   Lisp_Object restr = save_restriction_save_1 ();
   Lisp_Object locks = narrowing_locks_save ();
   return Fcons (restr, locks);
}

void
save_restriction_restore (Lisp_Object data)
{
   narrowing_locks_restore (XCDR (data));
   save_restriction_restore_1 (XCAR (data));
}

bug#56682: feature/improved-locked-narrowing 9dee6df39c: Reworked locked narrowing.

[Gregory Heytings]

[-- Attachment #1: Type: text/plain, Size: 881 bytes --]


>>> Yes: that's what the "record_unwind_protect (save_restriction_restore, 
>>> save_restriction_save ());" does.
>> 
>> So save_restriction_restore will reinstate the buffer in the 
>> narrowing_locks list?
>
> Yes:
>
> Lisp_Object
> save_restriction_save (void)
> {
>  Lisp_Object restr = save_restriction_save_1 ();
>  Lisp_Object locks = narrowing_locks_save ();
>  return Fcons (restr, locks);
> }
>
> void
> save_restriction_restore (Lisp_Object data)
> {
>  narrowing_locks_restore (XCDR (data));
>  save_restriction_restore_1 (XCAR (data));
> }
>

Eli, do you have further comments or objections, or can I install this? 
(Patch attached again for your convenience.)

BTW, I also would like to rename the narrowing_locks_* functions to make 
their names coherent with your change of the name of the macro from 
'with-narrowing' to 'with-restriction'.  I guess that's okay?

[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: Remove-narrowing-locks-before-calling-Fwiden.patch --]
[-- Type: text/x-diff; name=Remove-narrowing-locks-before-calling-Fwiden.patch, Size: 4485 bytes --]

From c482389fd59edcbc738a4d48ca6b65d14af60b25 Mon Sep 17 00:00:00 2001
From: Gregory Heytings <gregory@heytings.org>
Date: Tue, 14 Feb 2023 22:41:14 +0000
Subject: [PATCH] Remove narrowing locks before calling Fwiden

* src/editfns.c (narrowing_locks_remove_in_current_buffer) : New
function.

* src/lisp.h: Make it externally visible.

* src/xdisp.c (display_count_lines_logically):
* src/lread.c (readevalloop):
* src/indent.c (line_number_display_width):
* src/fileio.c (write_region):
* src/callproc.c (Fcall_process_region):
* src/buffer.c (Ferase_buffer): Use it.
---
 src/buffer.c   | 1 +
 src/callproc.c | 1 +
 src/editfns.c  | 7 +++++++
 src/fileio.c   | 1 +
 src/indent.c   | 1 +
 src/lisp.h     | 1 +
 src/lread.c    | 1 +
 src/xdisp.c    | 1 +
 8 files changed, 14 insertions(+)

diff --git a/src/buffer.c b/src/buffer.c
index df1f5206668..a9d004d8727 100644
--- a/src/buffer.c
+++ b/src/buffer.c
@@ -2386,6 +2386,7 @@ DEFUN ("erase-buffer", Ferase_buffer, Serase_buffer, 0, 0, "*",
 so the buffer is truly empty after this.  */)
   (void)
 {
+  narrowing_locks_remove_in_current_buffer ();
   Fwiden ();
 
   del_range (BEG, Z);
diff --git a/src/callproc.c b/src/callproc.c
index 5e1e1a8cc0a..c144f6e66d9 100644
--- a/src/callproc.c
+++ b/src/callproc.c
@@ -1113,6 +1113,7 @@ t (mix it with ordinary output), or a file name string.
         {
           /* No need to save restrictions since we delete everything
              anyway.  */
+          narrowing_locks_remove_in_current_buffer ();
           Fwiden ();
           del_range (BEG, Z);
         }
diff --git a/src/editfns.c b/src/editfns.c
index f83c5c7259b..32f654af8c3 100644
--- a/src/editfns.c
+++ b/src/editfns.c
@@ -2814,6 +2814,13 @@ narrowing_locks_restore (Lisp_Object buf_and_saved_locks)
     narrowing_locks_add (buf, saved_locks);
 }
 
+/* Remove all narrowing locks in the current buffer.  */
+void
+narrowing_locks_remove_in_current_buffer (void)
+{
+  narrowing_locks_remove (Fcurrent_buffer ());
+}
+
 static void
 unwind_narrow_to_region_locked (Lisp_Object tag)
 {
diff --git a/src/fileio.c b/src/fileio.c
index f00c389a520..cf5a4ecf80a 100644
--- a/src/fileio.c
+++ b/src/fileio.c
@@ -5269,6 +5269,7 @@ write_region (Lisp_Object start, Lisp_Object end, Lisp_Object filename,
     }
 
   record_unwind_protect (save_restriction_restore, save_restriction_save ());
+  narrowing_locks_remove_in_current_buffer ();
 
   /* Special kludge to simplify auto-saving.  */
   if (NILP (start))
diff --git a/src/indent.c b/src/indent.c
index 6de18d749ca..5f6dc47f034 100644
--- a/src/indent.c
+++ b/src/indent.c
@@ -2065,6 +2065,7 @@ line_number_display_width (struct window *w, int *width, int *pixel_width)
 	{
 	  record_unwind_protect (save_restriction_restore,
 				 save_restriction_save ());
+	  narrowing_locks_remove_in_current_buffer ();
 	  Fwiden ();
 	  saved_restriction = true;
 	}
diff --git a/src/lisp.h b/src/lisp.h
index 1276285e2f2..637ede7051e 100644
--- a/src/lisp.h
+++ b/src/lisp.h
@@ -4689,6 +4689,7 @@ XMODULE_FUNCTION (Lisp_Object o)
 					    ptrdiff_t, bool);
 extern void narrow_to_region_locked (Lisp_Object, Lisp_Object, Lisp_Object);
 extern void reset_outermost_narrowings (void);
+extern void narrowing_locks_remove_in_current_buffer (void);
 extern void init_editfns (void);
 extern void syms_of_editfns (void);
 
diff --git a/src/lread.c b/src/lread.c
index d0dc85f51c8..28ca483b819 100644
--- a/src/lread.c
+++ b/src/lread.c
@@ -2255,6 +2255,7 @@ readevalloop (Lisp_Object readcharfun,
 	  record_unwind_protect_excursion ();
 	  /* Save ZV in it.  */
 	  record_unwind_protect (save_restriction_restore, save_restriction_save ());
+	  narrowing_locks_remove_in_current_buffer ();
 	  /* Those get unbound after we read one expression.  */
 
 	  /* Set point and ZV around stuff to be read.  */
diff --git a/src/xdisp.c b/src/xdisp.c
index 5c5ecaa2bcb..6f2b4e6cd41 100644
--- a/src/xdisp.c
+++ b/src/xdisp.c
@@ -24111,6 +24111,7 @@ display_count_lines_logically (ptrdiff_t start_byte, ptrdiff_t limit_byte,
   ptrdiff_t val;
   specpdl_ref pdl_count = SPECPDL_INDEX ();
   record_unwind_protect (save_restriction_restore, save_restriction_save ());
+  narrowing_locks_remove_in_current_buffer ();
   Fwiden ();
   val = display_count_lines (start_byte, limit_byte, count, byte_pos_ptr);
   unbind_to (pdl_count, Qnil);
-- 
2.39.0

bug#56682: feature/improved-locked-narrowing 9dee6df39c: Reworked locked narrowing.

[Eli Zaretskii]

> Date: Sat, 18 Feb 2023 23:12:27 +0000
> From: Gregory Heytings <gregory@heytings.org>
> cc: 56682@debbugs.gnu.org, monnier@iro.umontreal.ca, akrl@sdf.org
> 
> Eli, do you have further comments or objections, or can I install this? 
> (Patch attached again for your convenience.)

Sorry, I forgot about this.  Please install, and thanks.

> BTW, I also would like to rename the narrowing_locks_* functions to make 
> their names coherent with your change of the name of the macro from 
> 'with-narrowing' to 'with-restriction'.  I guess that's okay?

I'm okay, but there's a thin line there I didn't want to cross: the
primitive is narrow-to-region, and several of its subroutines have
"narrow" in their names, which sounds right.  I'm not sure where to
draw the line.  What I did was to change names that are likely to
appear in Lisp and be exposed to the user/programmer.

bug#56682: feature/improved-locked-narrowing 9dee6df39c: Reworked locked narrowing.

[Gregory Heytings]

>
> I finally had enough free time to do that.  You will find the result in 
> the scratch/long-lines-cleanup branch.  I contains the following four 
> commits, in that order:
>
> 1. 85ed1c9ca6 with the renamings and documentation improvements
>
> 2. 7e26a5c774 with the patch mentioned above
>
> 3. afc2c6c13c with a bug fix, which makes cursor motion commands much 
> more accurate in long lines
>
> 4. 974e4f3333 with a minor cleanup
>

I just pushed a fifth commit (2093e010dc) to fix character motion commands 
in terminals.

bug#56682: feature/improved-locked-narrowing 9dee6df39c: Reworked locked narrowing.

[Gregory Heytings]

>> I finally had enough free time to do that.  You will find the result in 
>> the scratch/long-lines-cleanup branch.  I contains the following four 
>> commits, in that order:
>> 
>> 1. 85ed1c9ca6 with the renamings and documentation improvements
>> 
>> 2. 7e26a5c774 with the patch mentioned above
>> 
>> 3. afc2c6c13c with a bug fix, which makes cursor motion commands much 
>> more accurate in long lines
>> 
>> 4. 974e4f3333 with a minor cleanup
>
> I just pushed a fifth commit (2093e010dc) to fix character motion 
> commands in terminals.
>

Eli, do you have any comments?

bug#56682: feature/improved-locked-narrowing 9dee6df39c: Reworked locked narrowing.

[Eli Zaretskii]

> Date: Sat, 01 Apr 2023 00:27:14 +0000
> From: Gregory Heytings <gregory@heytings.org>
> cc: 56682@debbugs.gnu.org, monnier@iro.umontreal.ca
> 
> 
> >> I finally had enough free time to do that.  You will find the result in 
> >> the scratch/long-lines-cleanup branch.  I contains the following four 
> >> commits, in that order:
> >> 
> >> 1. 85ed1c9ca6 with the renamings and documentation improvements
> >> 
> >> 2. 7e26a5c774 with the patch mentioned above
> >> 
> >> 3. afc2c6c13c with a bug fix, which makes cursor motion commands much 
> >> more accurate in long lines
> >> 
> >> 4. 974e4f3333 with a minor cleanup
> >
> > I just pushed a fifth commit (2093e010dc) to fix character motion 
> > commands in terminals.
> >
> 
> Eli, do you have any comments?

I didn't yet have time to look at those, sorry.

bug#56682: feature/improved-locked-narrowing 9dee6df39c: Reworked locked narrowing.

[Gregory Heytings]

>> Eli, do you have any comments?
>
> I didn't yet have time to look at those, sorry.
>

Thanks, no worries!

bug#56682: feature/improved-locked-narrowing 9dee6df39c: Reworked locked narrowing.

[Eli Zaretskii]

> Date: Wed, 29 Mar 2023 14:52:08 +0000
> From: Gregory Heytings <gregory@heytings.org>
> cc: 56682@debbugs.gnu.org, monnier@iro.umontreal.ca
> 
> 
> >
> > I finally had enough free time to do that.  You will find the result in 
> > the scratch/long-lines-cleanup branch.  I contains the following four 
> > commits, in that order:
> >
> > 1. 85ed1c9ca6 with the renamings and documentation improvements
> >
> > 2. 7e26a5c774 with the patch mentioned above
> >
> > 3. afc2c6c13c with a bug fix, which makes cursor motion commands much 
> > more accurate in long lines
> >
> > 4. 974e4f3333 with a minor cleanup
> >
> 
> I just pushed a fifth commit (2093e010dc) to fix character motion commands 
> in terminals.

Thanks.

I took a look, and these commits do more than just renaming.  They
also change the code in non-trivial ways, and I'm not thrilled about
changing code in those parts at this late stage.

Specific comments:

afc2c6c13c:

  . Misses an optimization opportunity: where pos == BEGV, you can use
    BEGV_BYTE instead of calling CHAR_TO_BYTE.
  . I'm not sure I'm happy about calling find_newline in a loop where
    previous code just made a trivial calculation.  Imagine a buffer
    with a lot of short lines.  What problem exactly is being solved
    here, and how does this change solve it?

2093e010dc:

  . Why such a strange method of finding out whether we are on a TTY
    frame?  The usual method is FRAME_WINDOW_P (XFRAME (w->frame)).
  . The continuation glyph can be present not only on TTY frames, but
    also on GUI frames when one or both of the fringes are disabled,
    so the test needs to be augmented.  I think you need to use
    WINDOW_LEFT_FRINGE_WIDTH and WINDOW_RIGHT_FRINGE_WIDTH.

bug#56682: feature/improved-locked-narrowing 9dee6df39c: Reworked locked narrowing.

[Gregory Heytings]

Thanks for your review!

>
> They also change the code in non-trivial ways, and I'm not thrilled 
> about changing code in those parts at this late stage.
>

Two commits do change the code, yes, to fix (cursor motion command with 
C-n / C-p) bugs.  That code is only used for buffers with long lines.  It 
would be most regrettable to release Emacs 29 without these bugs fixed.

>
> Specific comments:
>
> afc2c6c13c:
>
> . Misses an optimization opportunity: where pos == BEGV, you can use 
> BEGV_BYTE instead of calling CHAR_TO_BYTE.
>

Okay, thanks, added in dce08cf05c.

>
> . I'm not sure I'm happy about calling find_newline in a loop where 
> previous code just made a trivial calculation.  Imagine a buffer with a 
> lot of short lines.  What problem exactly is being solved here, and how 
> does this change solve it?
>

The point is to find the buffer position of BOL.  But you're right, there 
is a missed optimization here, which I just added (also in dce08cf05c). 
Now the code searches for that position in [pos-500..pos], 
[pos-5500..pos-500], [pos-55500..pos-5500], [pos-555500..pos-55500], in 
that order, and buffers with lots of short lines (or more precisely: 
buffers with lots of short lines _and_ one or more long lines) are not 
negatively affected by that code anymore.

>
> 2093e010dc:
>
> . Why such a strange method of finding out whether we are on a TTY 
> frame?  The usual method is FRAME_WINDOW_P (XFRAME (w->frame)).
>

That's what I've been using since that function was introduced six months 
ago or so.  I admit I don't remember why that's what I chose.  If you tell 
me that using FRAME_WINDOW_P (XFRAME (w->frame)) has the same effect as 
terminal-live-p == t (and indeed after looking at the code ISTM that 
that's the case), I'll replace that.

>
> . The continuation glyph can be present not only on TTY frames, but also 
> on GUI frames when one or both of the fringes are disabled, so the test 
> needs to be augmented.  I think you need to use WINDOW_LEFT_FRINGE_WIDTH 
> and WINDOW_RIGHT_FRINGE_WIDTH.
>

Thanks, I did not realize that.  I just did that (again in dce08cf05c), 
but I'm not sure how WINDOW_LEFT_FRINGE_WIDTH should be used.  Using 
WINDOW_RIGHT_FRINGE_WIDTH seems enough, but I'm probably missing 
something.

bug#56682: feature/improved-locked-narrowing 9dee6df39c: Reworked locked narrowing.

[Eli Zaretskii]

> Date: Sat, 01 Apr 2023 14:26:50 +0000
> From: Gregory Heytings <gregory@heytings.org>
> cc: 56682@debbugs.gnu.org, monnier@iro.umontreal.ca
> 
> > . I'm not sure I'm happy about calling find_newline in a loop where 
> > previous code just made a trivial calculation.  Imagine a buffer with a 
> > lot of short lines.  What problem exactly is being solved here, and how 
> > does this change solve it?
> >
> 
> The point is to find the buffer position of BOL.  But you're right, there 
> is a missed optimization here, which I just added (also in dce08cf05c). 
> Now the code searches for that position in [pos-500..pos], 
> [pos-5500..pos-500], [pos-55500..pos-5500], [pos-555500..pos-55500], in 
> that order, and buffers with lots of short lines (or more precisely: 
> buffers with lots of short lines _and_ one or more long lines) are not 
> negatively affected by that code anymore.

OK, but still: why do you need to find the buffer position at BOL, and
how is that related to cursor movement?

> > . Why such a strange method of finding out whether we are on a TTY 
> > frame?  The usual method is FRAME_WINDOW_P (XFRAME (w->frame)).
> >
> 
> That's what I've been using since that function was introduced six months 
> ago or so.  I admit I don't remember why that's what I chose.  If you tell 
> me that using FRAME_WINDOW_P (XFRAME (w->frame)) has the same effect as 
> terminal-live-p == t (and indeed after looking at the code ISTM that 
> that's the case), I'll replace that.

FRAME_WINDOW_P is what we use all over the place, so it must be
correct.

> > . The continuation glyph can be present not only on TTY frames, but also 
> > on GUI frames when one or both of the fringes are disabled, so the test 
> > needs to be augmented.  I think you need to use WINDOW_LEFT_FRINGE_WIDTH 
> > and WINDOW_RIGHT_FRINGE_WIDTH.
> >
> 
> Thanks, I did not realize that.  I just did that (again in dce08cf05c), 
> but I'm not sure how WINDOW_LEFT_FRINGE_WIDTH should be used.  Using 
> WINDOW_RIGHT_FRINGE_WIDTH seems enough, but I'm probably missing 
> something.

You are missing the case of R2L paragraphs, where the continuation
glyph is displayed on the left.

bug#56682: feature/improved-locked-narrowing 9dee6df39c: Reworked locked narrowing.

[Gregory Heytings]

>
> OK, but still: why do you need to find the buffer position at BOL, and 
> how is that related to cursor movement?
>

Well, with the improved calculation:

max (bol_pos + ((pos - bol_pos) / len - 1) * len, BEGV)

instead of:

max ((pos / len - 1) * len, BEGV)

cursor movement becomes accurate.  You can see that for example in the 
'long-line.xml' file, in which the cursor does not move to a "wrong" 
column anymore (or sometimes even to another column on the same visual 
line).  The idea is that, instead of simply taking a position in the 
buffer that only depends on the value of 'pos' as a value for BEGV (with 
'get_small_narrowing_begv'), we take one that is, inside a long line, 
relative to BOL, IOW, it is always BOL + n * len, for some n.

>
> FRAME_WINDOW_P is what we use all over the place, so it must be correct.
>

Okay, thanks.

>
> You are missing the case of R2L paragraphs, where the continuation glyph 
> is displayed on the left.
>

Okay, so what would be the correct calculation here?  This?

int width = window_body_width (w, WINDOW_BODY_IN_CANONICAL_CHARS)
   - (WINDOW_RIGHT_FRINGE_WIDTH (w) == 0
      || WINDOW_RIGHT_FRINGE_WIDTH (w) == 0 ? 1 : 0);

bug#56682: feature/improved-locked-narrowing 9dee6df39c: Reworked locked narrowing.

[Eli Zaretskii]

> Date: Sat, 01 Apr 2023 15:41:13 +0000
> From: Gregory Heytings <gregory@heytings.org>
> cc: 56682@debbugs.gnu.org, monnier@iro.umontreal.ca
> 
> > You are missing the case of R2L paragraphs, where the continuation glyph 
> > is displayed on the left.
> 
> Okay, so what would be the correct calculation here?  This?
> 
> int width = window_body_width (w, WINDOW_BODY_IN_CANONICAL_CHARS)
>    - (WINDOW_RIGHT_FRINGE_WIDTH (w) == 0
>       || WINDOW_RIGHT_FRINGE_WIDTH (w) == 0 ? 1 : 0);

Yes.

bug#56682: feature/improved-locked-narrowing 9dee6df39c: Reworked locked narrowing.

[Gregory Heytings]

>> Okay, so what would be the correct calculation here?  This?
>
> Yes.
>

Thanks.  Done in 097c5ee8f5, together with the FRAME_WINDOW_P fix.  Do you 
have other comments/reservations?

bug#56682: feature/improved-locked-narrowing 9dee6df39c: Reworked locked narrowing.

[Eli Zaretskii]

> Date: Sat, 01 Apr 2023 17:01:03 +0000
> From: Gregory Heytings <gregory@heytings.org>
> cc: 56682@debbugs.gnu.org, monnier@iro.umontreal.ca
> 
> 
> >> Okay, so what would be the correct calculation here?  This?
> >
> > Yes.
> >
> 
> Thanks.  Done in 097c5ee8f5, together with the FRAME_WINDOW_P fix.  Do you 
> have other comments/reservations?

Not specific ones, no.  I just hope there are no hidden rocks there,
as this code was not well tested.

bug#56682: feature/improved-locked-narrowing 9dee6df39c: Reworked locked narrowing.

[Gregory Heytings]

>> Thanks.  Done in 097c5ee8f5, together with the FRAME_WINDOW_P fix.  Do 
>> you have other comments/reservations?
>
> Not specific ones, no.  I just hope there are no hidden rocks there, as 
> this code was not well tested.
>

That's what I hope, too.  But the risks are really small here, the only 
potential problem I can imagine would be some kind of error in 
'get_nearby_bol_pos', but I don't see where it could possibly fail.  I 
made a few final fixes; I guess I can now merge that branch?

bug#56682: feature/improved-locked-narrowing 9dee6df39c: Reworked locked narrowing.

[Eli Zaretskii]

> Date: Sat, 01 Apr 2023 21:56:20 +0000
> From: Gregory Heytings <gregory@heytings.org>
> cc: 56682@debbugs.gnu.org, monnier@iro.umontreal.ca
> 
> 
> >> Thanks.  Done in 097c5ee8f5, together with the FRAME_WINDOW_P fix.  Do 
> >> you have other comments/reservations?
> >
> > Not specific ones, no.  I just hope there are no hidden rocks there, as 
> > this code was not well tested.
> 
> That's what I hope, too.  But the risks are really small here, the only 
> potential problem I can imagine would be some kind of error in 
> 'get_nearby_bol_pos', but I don't see where it could possibly fail.

Famous last words...

> I made a few final fixes; I guess I can now merge that branch?

Yes, fingers crossed.

bug#56682: feature/improved-locked-narrowing 9dee6df39c: Reworked locked narrowing.

[Richard Stallman]

[[[ To any NSA and FBI agents reading my email: please consider    ]]]
[[[ whether defending the US Constitution against all enemies,     ]]]
[[[ foreign or domestic, requires you to follow Snowden's example. ]]]

  > > I made a few final fixes; I guess I can now merge that branch?

  > Yes, fingers crossed.

How could he type the command with fingers crossed?

-- 
Dr Richard Stallman (https://stallman.org)
Chief GNUisance of the GNU Project (https://gnu.org)
Founder, Free Software Foundation (https://fsf.org)
Internet Hall-of-Famer (https://internethalloffame.org)

bug#56682: feature/improved-locked-narrowing 9dee6df39c: Reworked locked narrowing.

[Eli Zaretskii]

> From: Richard Stallman <rms@gnu.org>
> Cc: gregory@heytings.org, 56682@debbugs.gnu.org,
> 	monnier@iro.umontreal.ca
> Date: Mon, 03 Apr 2023 22:55:14 -0400
> 
>   > > I made a few final fixes; I guess I can now merge that branch?
> 
>   > Yes, fingers crossed.
> 
> How could he type the command with fingers crossed?

By using the other extremities.

bug#56682: feature/improved-locked-narrowing 9dee6df39c: Reworked locked narrowing.

[Eli Zaretskii]

> Date: Wed, 03 May 2023 21:12:47 +0000
> From: Gregory Heytings <gregory@heytings.org>
> cc: 56682@debbugs.gnu.org, monnier@iro.umontreal.ca
> 
> >> I made a few final fixes; I guess I can now merge that branch?
> >
> > Yes, fingers crossed.
> >
> 
> Eli, I've been away for a month, and apparently I forgot to push the 
> merge.  Sorry for that.  Is this still okay now?

You've got to be kidding!  I was thinking about releasing Emacs 29.1
soon!

Please show the full diffs of the proposed merge, I will have to
think about this again, and think hard.

> By the way, I think we should add a hook and run it when long line 
> optimizations are activated, so that users can, for example, automatically 
> turn modes that are known to be problematic off.  WDYT?

I don't think we should add hooks without real-life use cases that we
understand and consider important.  Hooks called by redisplay are
especially problematic, and should be added very sparingly.

Thanks.

bug#56682: feature/improved-locked-narrowing 9dee6df39c: Reworked locked narrowing.

[Gregory Heytings]

[-- Attachment #1: Type: text/plain, Size: 1057 bytes --]


>> Eli, I've been away for a month, and apparently I forgot to push the 
>> merge.  Sorry for that.  Is this still okay now?
>
> You've got to be kidding!  I was thinking about releasing Emacs 29.1 
> soon!
>

Sorry again.

>
> Please show the full diffs of the proposed merge, I will have to think 
> about this again, and think hard.
>

You can get them by checking out the scratch/long-lines-cleanup and doing 
"git diff emacs-29..."  Just in case, I attach the full diff.  Most of it 
are renamings and documentation improvements.  The main code change is the 
function 'get_nearby_bol_pos' used by 'get_small_narrowing_begv'.

>> By the way, I think we should add a hook and run it when long line 
>> optimizations are activated, so that users can, for example, 
>> automatically turn modes that are known to be problematic off.  WDYT?
>
> I don't think we should add hooks without real-life use cases that we 
> understand and consider important.  Hooks called by redisplay are 
> especially problematic, and should be added very sparingly.
>

Okay.

[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: long-lines-cleanup.diff --]
[-- Type: text/x-diff; name=long-lines-cleanup.diff, Size: 44262 bytes --]

diff --git a/doc/lispref/positions.texi b/doc/lispref/positions.texi
index 1b32f18922c..e09fd4e7ca5 100644
--- a/doc/lispref/positions.texi
+++ b/doc/lispref/positions.texi
@@ -1154,6 +1154,7 @@ saved bounds.  In that case it is equivalent to
 @end example
 
 @cindex labeled narrowing
+@cindex labeled restriction
 When the optional argument @var{label}, a symbol, is present, the
 narrowing is @dfn{labeled}.  A labeled narrowing differs from a
 non-labeled one in several ways:
diff --git a/lisp/subr.el b/lisp/subr.el
index f82826e819c..ca1fc2886b4 100644
--- a/lisp/subr.el
+++ b/lisp/subr.el
@@ -3975,7 +3975,7 @@ same LABEL argument.
   "Helper function for `with-restriction', which see."
   (save-restriction
     (narrow-to-region start end)
-    (if label (internal--lock-narrowing label))
+    (if label (internal--label-restriction label))
     (funcall body)))
 
 (defmacro without-restriction (&rest rest)
@@ -3997,7 +3997,7 @@ are lifted.
 (defun internal--without-restriction (body &optional label)
   "Helper function for `without-restriction', which see."
   (save-restriction
-    (if label (internal--unlock-narrowing label))
+    (if label (internal--unlabel-restriction label))
     (widen)
     (funcall body)))
 
diff --git a/src/buffer.c b/src/buffer.c
index 0c740775e5b..252231357bc 100644
--- a/src/buffer.c
+++ b/src/buffer.c
@@ -2386,6 +2386,7 @@ Any narrowing restriction in effect (see `narrow-to-region') is removed,
 so the buffer is truly empty after this.  */)
   (void)
 {
+  labeled_restrictions_remove_in_current_buffer ();
   Fwiden ();
 
   del_range (BEG, Z);
diff --git a/src/callproc.c b/src/callproc.c
index 5e1e1a8cc0a..6f3d4fad9be 100644
--- a/src/callproc.c
+++ b/src/callproc.c
@@ -1113,6 +1113,7 @@ usage: (call-process-region START END PROGRAM &optional DELETE BUFFER DISPLAY &r
         {
           /* No need to save restrictions since we delete everything
              anyway.  */
+          labeled_restrictions_remove_in_current_buffer ();
           Fwiden ();
           del_range (BEG, Z);
         }
diff --git a/src/composite.c b/src/composite.c
index 164eeb39598..885db24df01 100644
--- a/src/composite.c
+++ b/src/composite.c
@@ -1075,7 +1075,7 @@ composition_compute_stop_pos (struct composition_it *cmp_it, ptrdiff_t charpos,
 	     with long lines, however, NL might be far away, so
 	     pretend that the buffer is smaller.  */
 	  if (current_buffer->long_line_optimizations_p)
-	    endpos = get_closer_narrowed_begv (cmp_it->parent_it->w, charpos);
+	    endpos = get_small_narrowing_begv (cmp_it->parent_it->w, charpos);
 	}
     }
   cmp_it->id = -1;
@@ -1654,7 +1654,7 @@ find_automatic_composition (ptrdiff_t pos, ptrdiff_t limit, ptrdiff_t backlim,
 	{
 	  /* In buffers with very long lines, this function becomes very
 	     slow.  Pretend that the buffer is narrowed to make it fast.  */
-	  ptrdiff_t begv = get_closer_narrowed_begv (w, window_point (w));
+	  ptrdiff_t begv = get_small_narrowing_begv (w, window_point (w));
 	  if (pos > begv)
 	    head = begv;
 	}
diff --git a/src/dispextern.h b/src/dispextern.h
index 4dcab113ea2..ece128949f5 100644
--- a/src/dispextern.h
+++ b/src/dispextern.h
@@ -2334,21 +2334,20 @@ struct it
      with which display_string was called.  */
   ptrdiff_t end_charpos;
 
-  /* Alternate begin position of the buffer that may be used to
-     optimize display (see the SET_WITH_NARROWED_BEGV macro).  */
-  ptrdiff_t narrowed_begv;
-
-  /* Alternate end position of the buffer that may be used to
-     optimize display.  */
-  ptrdiff_t narrowed_zv;
-
-  /* Begin position of the buffer for the locked narrowing around
-     low-level hooks.  */
-  ptrdiff_t locked_narrowing_begv;
-
-  /* End position of the buffer for the locked narrowing around
-     low-level hooks.  */
-  ptrdiff_t locked_narrowing_zv;
+  /* Alternate begin and end positions of the buffer that are used to
+     optimize display of buffers with long lines.  These two fields
+     hold the return value of the 'get_medium_narrowing_begv' and
+     'get_medium_narrowing_zv' functions.  */
+  ptrdiff_t medium_narrowing_begv;
+  ptrdiff_t medium_narrowing_zv;
+
+  /* Alternate begin and end positions of the buffer that are used for
+     labeled narrowings around low-level hooks in buffers with long
+     lines.  These two fields hold the return value of the
+     'get_large_narrowing_begv' and 'get_large_narrowing_zv'
+     functions.  */
+  ptrdiff_t large_narrowing_begv;
+  ptrdiff_t large_narrowing_zv;
 
   /* C string to iterate over.  Non-null means get characters from
      this string, otherwise characters are read from current_buffer
@@ -3410,11 +3409,9 @@ void mark_window_display_accurate (Lisp_Object, bool);
 void redisplay_preserve_echo_area (int);
 void init_iterator (struct it *, struct window *, ptrdiff_t,
                     ptrdiff_t, struct glyph_row *, enum face_id);
-ptrdiff_t get_narrowed_begv (struct window *, ptrdiff_t);
-ptrdiff_t get_narrowed_zv (struct window *, ptrdiff_t);
-ptrdiff_t get_closer_narrowed_begv (struct window *, ptrdiff_t);
-ptrdiff_t get_locked_narrowing_begv (ptrdiff_t);
-ptrdiff_t get_locked_narrowing_zv (ptrdiff_t);
+ptrdiff_t get_small_narrowing_begv (struct window *, ptrdiff_t);
+ptrdiff_t get_large_narrowing_begv (ptrdiff_t);
+ptrdiff_t get_large_narrowing_zv (ptrdiff_t);
 void init_iterator_to_row_start (struct it *, struct window *,
                                  struct glyph_row *);
 void start_display (struct it *, struct window *, struct text_pos);
diff --git a/src/editfns.c b/src/editfns.c
index f83c5c7259b..4c5b691eb50 100644
--- a/src/editfns.c
+++ b/src/editfns.c
@@ -2653,182 +2653,204 @@ DEFUN ("delete-and-extract-region", Fdelete_and_extract_region,
   return del_range_1 (XFIXNUM (start), XFIXNUM (end), 1, 1);
 }
 \f
-/* Alist of buffers in which locked narrowing is used.  The car of
-   each list element is a buffer, the cdr is a list of triplets (tag
-   begv-marker zv-marker).  The last element of that list always uses
-   the (uninterned) Qoutermost_narrowing tag and records the narrowing
-   bounds that were set by the user and that are visible on display.
-   This alist is used internally by narrow-to-region, widen,
-   internal--lock-narrowing, internal--unlock-narrowing and
-   save-restriction.  For efficiency reasons, an alist is used instead
-   of a buffer-local variable: otherwise reset_outermost_narrowings,
-   which is called during each redisplay cycle, would have to loop
-   through all live buffers.  */
-static Lisp_Object narrowing_locks;
-
-/* Add BUF with its LOCKS in the narrowing_locks alist.  */
+/* Alist of buffers in which labeled restrictions are used.  The car
+   of each list element is a buffer, the cdr is a list of triplets
+   (label begv-marker zv-marker).  The last triplet of that list
+   always uses the (uninterned) Qoutermost_restriction label, and
+   records the restriction bounds that were current when the first
+   labeled restriction was entered (which may be a narrowing that was
+   set by the user and is visible on display).  This alist is used
+   internally by narrow-to-region, widen, internal--label-restriction,
+   internal--unlabel-restriction and save-restriction.  For efficiency
+   reasons, an alist is used instead of a buffer-local variable:
+   otherwise reset_outermost_restrictions, which is called during each
+   redisplay cycle, would have to loop through all live buffers.  */
+static Lisp_Object labeled_restrictions;
+
+/* Add BUF with its list of labeled RESTRICTIONS in the
+   labeled_restrictions alist.  */
 static void
-narrowing_locks_add (Lisp_Object buf, Lisp_Object locks)
+labeled_restrictions_add (Lisp_Object buf, Lisp_Object restrictions)
 {
-  narrowing_locks = nconc2 (list1 (list2 (buf, locks)), narrowing_locks);
+  labeled_restrictions = nconc2 (list1 (list2 (buf, restrictions)),
+				 labeled_restrictions);
 }
 
-/* Remove BUF and its locks from the narrowing_locks alist.  Do
-   nothing if BUF is not present in narrowing_locks.  */
+/* Remove BUF and its list of labeled restrictions from the
+   labeled_restrictions alist.  Do nothing if BUF is not present in
+   labeled_restrictions.  */
 static void
-narrowing_locks_remove (Lisp_Object buf)
+labeled_restrictions_remove (Lisp_Object buf)
 {
-  narrowing_locks = Fdelq (Fassoc (buf, narrowing_locks, Qnil),
-			   narrowing_locks);
+  labeled_restrictions = Fdelq (Fassoc (buf, labeled_restrictions, Qnil),
+				labeled_restrictions);
 }
 
-/* Retrieve one of the BEGV/ZV bounds of a narrowing in BUF from the
-   narrowing_locks alist, as a pointer to a struct Lisp_Marker, or
-   NULL if BUF is not in narrowing_locks or is a killed buffer.  When
-   OUTERMOST is true, the bounds that were set by the user and that
-   are visible on display are returned.  Otherwise the innermost
-   locked narrowing bounds are returned.  */
+/* Retrieve one of the labeled restriction bounds in BUF from the
+   labeled_restrictions alist, as a pointer to a struct Lisp_Marker,
+   or return NULL if BUF is not in labeled_restrictions or is a killed
+   buffer.  When OUTERMOST is true, the restriction bounds that were
+   current when the first labeled restriction was entered are
+   returned.  Otherwise the bounds of the innermost labeled
+   restriction are returned.  */
 static struct Lisp_Marker *
-narrowing_lock_get_bound (Lisp_Object buf, bool begv, bool outermost)
+labeled_restrictions_get_bound (Lisp_Object buf, bool begv, bool outermost)
 {
   if (NILP (Fbuffer_live_p (buf)))
     return NULL;
-  Lisp_Object buffer_locks = assq_no_quit (buf, narrowing_locks);
-  if (NILP (buffer_locks))
+  Lisp_Object restrictions = assq_no_quit (buf, labeled_restrictions);
+  if (NILP (restrictions))
     return NULL;
-  buffer_locks = XCAR (XCDR (buffer_locks));
+  restrictions = XCAR (XCDR (restrictions));
   Lisp_Object bounds
     = outermost
-      ? XCDR (assq_no_quit (Qoutermost_narrowing, buffer_locks))
-      : XCDR (XCAR (buffer_locks));
+      ? XCDR (assq_no_quit (Qoutermost_restriction, restrictions))
+      : XCDR (XCAR (restrictions));
   eassert (! NILP (bounds));
   Lisp_Object marker = begv ? XCAR (bounds) : XCAR (XCDR (bounds));
   eassert (EQ (Fmarker_buffer (marker), buf));
   return XMARKER (marker);
 }
 
-/* Retrieve the tag of the innermost narrowing in BUF.  Return nil if
-   BUF is not in narrowing_locks or is a killed buffer.  */
+/* Retrieve the label of the innermost labeled restriction in BUF.
+   Return nil if BUF is not in labeled_restrictions or is a killed
+   buffer.  */
 static Lisp_Object
-narrowing_lock_peek_tag (Lisp_Object buf)
+labeled_restrictions_peek_label (Lisp_Object buf)
 {
   if (NILP (Fbuffer_live_p (buf)))
     return Qnil;
-  Lisp_Object buffer_locks = assq_no_quit (buf, narrowing_locks);
-  if (NILP (buffer_locks))
+  Lisp_Object restrictions = assq_no_quit (buf, labeled_restrictions);
+  if (NILP (restrictions))
     return Qnil;
-  Lisp_Object tag = XCAR (XCAR (XCAR (XCDR (buffer_locks))));
-  eassert (! NILP (tag));
-  return tag;
+  Lisp_Object label = XCAR (XCAR (XCAR (XCDR (restrictions))));
+  eassert (! NILP (label));
+  return label;
 }
 
-/* Add a LOCK for BUF in the narrowing_locks alist.  */
+/* Add a labeled RESTRICTION for BUF in the labeled_restrictions
+   alist.  */
 static void
-narrowing_lock_push (Lisp_Object buf, Lisp_Object lock)
+labeled_restrictions_push (Lisp_Object buf, Lisp_Object restriction)
 {
-  Lisp_Object buffer_locks = assq_no_quit (buf, narrowing_locks);
-  if (NILP (buffer_locks))
-    narrowing_locks_add (buf, list1 (lock));
+  Lisp_Object restrictions = assq_no_quit (buf, labeled_restrictions);
+  if (NILP (restrictions))
+    labeled_restrictions_add (buf, list1 (restriction));
   else
-    XSETCDR (buffer_locks, list1 (nconc2 (list1 (lock),
-					  XCAR (XCDR (buffer_locks)))));
+    XSETCDR (restrictions, list1 (nconc2 (list1 (restriction),
+					  XCAR (XCDR (restrictions)))));
 }
 
-/* Remove the innermost lock in BUF from the narrowing_locks alist.
-   Do nothing if BUF is not present in narrowing_locks.  */
+/* Remove the innermost labeled restriction in BUF from the
+   labeled_restrictions alist.  Do nothing if BUF is not present in
+   labeled_restrictions.  */
 static void
-narrowing_lock_pop (Lisp_Object buf)
+labeled_restrictions_pop (Lisp_Object buf)
 {
-  Lisp_Object buffer_locks = assq_no_quit (buf, narrowing_locks);
-  if (NILP (buffer_locks))
+  Lisp_Object restrictions = assq_no_quit (buf, labeled_restrictions);
+  if (NILP (restrictions))
     return;
-  if (EQ (narrowing_lock_peek_tag (buf), Qoutermost_narrowing))
-    narrowing_locks_remove (buf);
+  if (EQ (labeled_restrictions_peek_label (buf), Qoutermost_restriction))
+    labeled_restrictions_remove (buf);
   else
-    XSETCDR (buffer_locks, list1 (XCDR (XCAR (XCDR (buffer_locks)))));
+    XSETCDR (restrictions, list1 (XCDR (XCAR (XCDR (restrictions)))));
+}
+
+/* Unconditionally remove all labeled restrictions in current_buffer.  */
+void
+labeled_restrictions_remove_in_current_buffer (void)
+{
+  labeled_restrictions_remove (Fcurrent_buffer ());
 }
 
 static void
-unwind_reset_outermost_narrowing (Lisp_Object buf)
+unwind_reset_outermost_restriction (Lisp_Object buf)
 {
-  struct Lisp_Marker *begv = narrowing_lock_get_bound (buf, true, false);
-  struct Lisp_Marker *zv = narrowing_lock_get_bound (buf, false, false);
+  struct Lisp_Marker *begv
+    = labeled_restrictions_get_bound (buf, true, false);
+  struct Lisp_Marker *zv
+    = labeled_restrictions_get_bound (buf, false, false);
   if (begv != NULL && zv != NULL)
     {
       SET_BUF_BEGV_BOTH (XBUFFER (buf), begv->charpos, begv->bytepos);
       SET_BUF_ZV_BOTH (XBUFFER (buf), zv->charpos, zv->bytepos);
     }
   else
-    narrowing_locks_remove (buf);
+    labeled_restrictions_remove (buf);
 }
 
-/* Restore the narrowing bounds that were set by the user, and restore
-   the bounds of the locked narrowing upon return.
+/* Restore the restriction bounds that were current when the first
+   labeled restriction was entered, and restore the bounds of the
+   innermost labeled restriction upon return.
    In particular, this function is called when redisplay starts, so
    that if a Lisp function executed during redisplay calls (redisplay)
-   while a locked narrowing is in effect, the locked narrowing will
-   not be visible on display.
+   while labeled restrictions are in effect, these restrictions will
+   not become visible on display.
    See https://debbugs.gnu.org/cgi/bugreport.cgi?bug=57207#140 and
    https://debbugs.gnu.org/cgi/bugreport.cgi?bug=57207#254 for example
    recipes that demonstrate why this is necessary.  */
 void
-reset_outermost_narrowings (void)
+reset_outermost_restrictions (void)
 {
   Lisp_Object val, buf;
-  for (val = narrowing_locks; CONSP (val); val = XCDR (val))
+  for (val = labeled_restrictions; CONSP (val); val = XCDR (val))
     {
       buf = XCAR (XCAR (val));
       eassert (BUFFERP (buf));
-      struct Lisp_Marker *begv = narrowing_lock_get_bound (buf, true, true);
-      struct Lisp_Marker *zv = narrowing_lock_get_bound (buf, false, true);
+      struct Lisp_Marker *begv
+	= labeled_restrictions_get_bound (buf, true, true);
+      struct Lisp_Marker *zv
+	= labeled_restrictions_get_bound (buf, false, true);
       if (begv != NULL && zv != NULL)
 	{
 	  SET_BUF_BEGV_BOTH (XBUFFER (buf), begv->charpos, begv->bytepos);
 	  SET_BUF_ZV_BOTH (XBUFFER (buf), zv->charpos, zv->bytepos);
-	  record_unwind_protect (unwind_reset_outermost_narrowing, buf);
+	  record_unwind_protect (unwind_reset_outermost_restriction, buf);
 	}
       else
-	narrowing_locks_remove (buf);
+	labeled_restrictions_remove (buf);
     }
 }
 
-/* Helper functions to save and restore the narrowing locks of the
-   current buffer in Fsave_restriction.  */
+/* Helper functions to save and restore the labeled restrictions of
+   the current buffer in Fsave_restriction.  */
 static Lisp_Object
-narrowing_locks_save (void)
+labeled_restrictions_save (void)
 {
   Lisp_Object buf = Fcurrent_buffer ();
-  Lisp_Object locks = assq_no_quit (buf, narrowing_locks);
-  if (!NILP (locks))
-    locks = XCAR (XCDR (locks));
-  return Fcons (buf, Fcopy_sequence (locks));
+  Lisp_Object restrictions = assq_no_quit (buf, labeled_restrictions);
+  if (! NILP (restrictions))
+    restrictions = XCAR (XCDR (restrictions));
+  return Fcons (buf, Fcopy_sequence (restrictions));
 }
 
 static void
-narrowing_locks_restore (Lisp_Object buf_and_saved_locks)
+labeled_restrictions_restore (Lisp_Object buf_and_restrictions)
 {
-  Lisp_Object buf = XCAR (buf_and_saved_locks);
-  Lisp_Object saved_locks = XCDR (buf_and_saved_locks);
-  narrowing_locks_remove (buf);
-  if (!NILP (saved_locks))
-    narrowing_locks_add (buf, saved_locks);
+  Lisp_Object buf = XCAR (buf_and_restrictions);
+  Lisp_Object restrictions = XCDR (buf_and_restrictions);
+  labeled_restrictions_remove (buf);
+  if (! NILP (restrictions))
+    labeled_restrictions_add (buf, restrictions);
 }
 
 static void
-unwind_narrow_to_region_locked (Lisp_Object tag)
+unwind_labeled_narrow_to_region (Lisp_Object label)
 {
-  Finternal__unlock_narrowing (tag);
+  Finternal__unlabel_restriction (label);
   Fwiden ();
 }
 
-/* Narrow current_buffer to BEGV-ZV with a narrowing locked with TAG.  */
+/* Narrow current_buffer to BEGV-ZV with a restriction labeled with
+   LABEL.  */
 void
-narrow_to_region_locked (Lisp_Object begv, Lisp_Object zv, Lisp_Object tag)
+labeled_narrow_to_region (Lisp_Object begv, Lisp_Object zv,
+			  Lisp_Object label)
 {
   Fnarrow_to_region (begv, zv);
-  Finternal__lock_narrowing (tag);
+  Finternal__label_restriction (label);
   record_unwind_protect (restore_point_unwind, Fpoint_marker ());
-  record_unwind_protect (unwind_narrow_to_region_locked, tag);
+  record_unwind_protect (unwind_labeled_narrow_to_region, label);
 }
 
 DEFUN ("widen", Fwiden, Swiden, 0, 0, "",
@@ -2842,11 +2864,11 @@ To gain access to other portions of the buffer, use
 `without-restriction' with the same label.  */)
   (void)
 {
-  Fset (Qoutermost_narrowing, Qnil);
+  Fset (Qoutermost_restriction, Qnil);
   Lisp_Object buf = Fcurrent_buffer ();
-  Lisp_Object tag = narrowing_lock_peek_tag (buf);
+  Lisp_Object label = labeled_restrictions_peek_label (buf);
 
-  if (NILP (tag))
+  if (NILP (label))
     {
       if (BEG != BEGV || Z != ZV)
 	current_buffer->clip_changed = 1;
@@ -2856,19 +2878,21 @@ To gain access to other portions of the buffer, use
     }
   else
     {
-      struct Lisp_Marker *begv = narrowing_lock_get_bound (buf, true, false);
-      struct Lisp_Marker *zv = narrowing_lock_get_bound (buf, false, false);
+      struct Lisp_Marker *begv
+	= labeled_restrictions_get_bound (buf, true, false);
+      struct Lisp_Marker *zv
+	= labeled_restrictions_get_bound (buf, false, false);
       eassert (begv != NULL && zv != NULL);
       if (begv->charpos != BEGV || zv->charpos != ZV)
 	current_buffer->clip_changed = 1;
       SET_BUF_BEGV_BOTH (current_buffer, begv->charpos, begv->bytepos);
       SET_BUF_ZV_BOTH (current_buffer, zv->charpos, zv->bytepos);
-      /* If the only remaining bounds in narrowing_locks for
+      /* If the only remaining bounds in labeled_restrictions for
 	 current_buffer are the bounds that were set by the user, no
-	 locked narrowing is in effect in current_buffer anymore:
-	 remove it from the narrowing_locks alist.  */
-      if (EQ (tag, Qoutermost_narrowing))
-	narrowing_lock_pop (buf);
+	 labeled restriction is in effect in current_buffer anymore:
+	 remove it from the labeled_restrictions alist.  */
+      if (EQ (label, Qoutermost_restriction))
+	labeled_restrictions_pop (buf);
     }
   /* Changing the buffer bounds invalidates any recorded current column.  */
   invalidate_current_column ();
@@ -2905,13 +2929,15 @@ argument.  To gain access to other portions of the buffer, use
     args_out_of_range (start, end);
 
   Lisp_Object buf = Fcurrent_buffer ();
-  if (! NILP (narrowing_lock_peek_tag (buf)))
+  if (! NILP (labeled_restrictions_peek_label (buf)))
     {
-      struct Lisp_Marker *begv = narrowing_lock_get_bound (buf, true, false);
-      struct Lisp_Marker *zv = narrowing_lock_get_bound (buf, false, false);
+      /* Limit the start and end positions to those of the innermost
+	 labeled restriction.  */
+      struct Lisp_Marker *begv
+	= labeled_restrictions_get_bound (buf, true, false);
+      struct Lisp_Marker *zv
+	= labeled_restrictions_get_bound (buf, false, false);
       eassert (begv != NULL && zv != NULL);
-      /* Limit the start and end positions to those of the locked
-	 narrowing.  */
       if (s < begv->charpos) s = begv->charpos;
       if (s > zv->charpos) s = zv->charpos;
       if (e < begv->charpos) e = begv->charpos;
@@ -2919,11 +2945,11 @@ argument.  To gain access to other portions of the buffer, use
     }
 
   /* Record the accessible range of the buffer when narrow-to-region
-     is called, that is, before applying the narrowing.  It is used
-     only by internal--lock-narrowing.  */
-  Fset (Qoutermost_narrowing, list3 (Qoutermost_narrowing,
-				     Fpoint_min_marker (),
-				     Fpoint_max_marker ()));
+     is called, that is, before applying the narrowing.  That
+     information is used only by internal--label-restriction.  */
+  Fset (Qoutermost_restriction, list3 (Qoutermost_restriction,
+				       Fpoint_min_marker (),
+				       Fpoint_max_marker ()));
 
   if (BEGV != s || ZV != e)
     current_buffer->clip_changed = 1;
@@ -2940,38 +2966,38 @@ argument.  To gain access to other portions of the buffer, use
   return Qnil;
 }
 
-DEFUN ("internal--lock-narrowing", Finternal__lock_narrowing,
-       Sinternal__lock_narrowing, 1, 1, 0,
-       doc: /* Lock the current narrowing with LABEL.
+DEFUN ("internal--label-restriction", Finternal__label_restriction,
+       Sinternal__label_restriction, 1, 1, 0,
+       doc: /* Label the current restriction with LABEL.
 
 This is an internal function used by `with-restriction'.  */)
-  (Lisp_Object tag)
+  (Lisp_Object label)
 {
   Lisp_Object buf = Fcurrent_buffer ();
-  Lisp_Object outermost_narrowing
-    = buffer_local_value (Qoutermost_narrowing, buf);
-  /* If internal--lock-narrowing is ever called without being preceded
-     by narrow-to-region, do nothing.  */
-  if (NILP (outermost_narrowing))
+  Lisp_Object outermost_restriction
+    = buffer_local_value (Qoutermost_restriction, buf);
+  /* If internal--label-restriction is ever called without being
+     preceded by narrow-to-region, do nothing.  */
+  if (NILP (outermost_restriction))
     return Qnil;
-  if (NILP (narrowing_lock_peek_tag (buf)))
-    narrowing_lock_push (buf, outermost_narrowing);
-  narrowing_lock_push (buf, list3 (tag,
-				   Fpoint_min_marker (),
-				   Fpoint_max_marker ()));
+  if (NILP (labeled_restrictions_peek_label (buf)))
+    labeled_restrictions_push (buf, outermost_restriction);
+  labeled_restrictions_push (buf, list3 (label,
+					 Fpoint_min_marker (),
+					 Fpoint_max_marker ()));
   return Qnil;
 }
 
-DEFUN ("internal--unlock-narrowing", Finternal__unlock_narrowing,
-       Sinternal__unlock_narrowing, 1, 1, 0,
-       doc: /* Unlock a narrowing locked with LABEL.
+DEFUN ("internal--unlabel-restriction", Finternal__unlabel_restriction,
+       Sinternal__unlabel_restriction, 1, 1, 0,
+       doc: /* If the current restriction is labeled with LABEL, remove its label.
 
 This is an internal function used by `without-restriction'.  */)
-  (Lisp_Object tag)
+  (Lisp_Object label)
 {
   Lisp_Object buf = Fcurrent_buffer ();
-  if (EQ (narrowing_lock_peek_tag (buf), tag))
-    narrowing_lock_pop (buf);
+  if (EQ (labeled_restrictions_peek_label (buf), label))
+    labeled_restrictions_pop (buf);
   return Qnil;
 }
 
@@ -3071,15 +3097,15 @@ save_restriction_restore_1 (Lisp_Object data)
 Lisp_Object
 save_restriction_save (void)
 {
-  Lisp_Object restr = save_restriction_save_1 ();
-  Lisp_Object locks = narrowing_locks_save ();
-  return Fcons (restr, locks);
+  Lisp_Object restriction = save_restriction_save_1 ();
+  Lisp_Object labeled_restrictions = labeled_restrictions_save ();
+  return Fcons (restriction, labeled_restrictions);
 }
 
 void
 save_restriction_restore (Lisp_Object data)
 {
-  narrowing_locks_restore (XCDR (data));
+  labeled_restrictions_restore (XCDR (data));
   save_restriction_restore_1 (XCAR (data));
 }
 
@@ -4748,7 +4774,7 @@ syms_of_editfns (void)
   DEFSYM (Qwall, "wall");
   DEFSYM (Qpropertize, "propertize");
 
-  staticpro (&narrowing_locks);
+  staticpro (&labeled_restrictions);
 
   DEFVAR_LISP ("inhibit-field-text-motion", Vinhibit_field_text_motion,
 	       doc: /* Non-nil means text motion commands don't notice fields.  */);
@@ -4809,12 +4835,12 @@ This variable is experimental; email 32252@debbugs.gnu.org if you need
 it to be non-nil.  */);
   binary_as_unsigned = false;
 
-  DEFVAR_LISP ("outermost-narrowing", Voutermost_narrowing,
+  DEFVAR_LISP ("outermost-restriction", Voutermost_restriction,
 	       doc: /* Outermost narrowing bounds, if any.  Internal use only.  */);
-  Voutermost_narrowing = Qnil;
-  Fmake_variable_buffer_local (Qoutermost_narrowing);
-  DEFSYM (Qoutermost_narrowing, "outermost-narrowing");
-  Funintern (Qoutermost_narrowing, Qnil);
+  Voutermost_restriction = Qnil;
+  Fmake_variable_buffer_local (Qoutermost_restriction);
+  DEFSYM (Qoutermost_restriction, "outermost-restriction");
+  Funintern (Qoutermost_restriction, Qnil);
 
   defsubr (&Spropertize);
   defsubr (&Schar_equal);
@@ -4907,8 +4933,8 @@ it to be non-nil.  */);
   defsubr (&Sdelete_and_extract_region);
   defsubr (&Swiden);
   defsubr (&Snarrow_to_region);
-  defsubr (&Sinternal__lock_narrowing);
-  defsubr (&Sinternal__unlock_narrowing);
+  defsubr (&Sinternal__label_restriction);
+  defsubr (&Sinternal__unlabel_restriction);
   defsubr (&Ssave_restriction);
   defsubr (&Stranspose_regions);
 }
diff --git a/src/fileio.c b/src/fileio.c
index f00c389a520..b50b3c6b935 100644
--- a/src/fileio.c
+++ b/src/fileio.c
@@ -5269,6 +5269,7 @@ write_region (Lisp_Object start, Lisp_Object end, Lisp_Object filename,
     }
 
   record_unwind_protect (save_restriction_restore, save_restriction_save ());
+  labeled_restrictions_remove_in_current_buffer ();
 
   /* Special kludge to simplify auto-saving.  */
   if (NILP (start))
diff --git a/src/indent.c b/src/indent.c
index 08d2bf5ea28..aef394dab88 100644
--- a/src/indent.c
+++ b/src/indent.c
@@ -2065,6 +2065,7 @@ line_number_display_width (struct window *w, int *width, int *pixel_width)
 	{
 	  record_unwind_protect (save_restriction_restore,
 				 save_restriction_save ());
+	  labeled_restrictions_remove_in_current_buffer ();
 	  Fwiden ();
 	  saved_restriction = true;
 	}
diff --git a/src/keyboard.c b/src/keyboard.c
index f7aa496bb81..b1ccf4acde4 100644
--- a/src/keyboard.c
+++ b/src/keyboard.c
@@ -318,6 +318,8 @@ static Lisp_Object command_loop (void);
 static void echo_now (void);
 static ptrdiff_t echo_length (void);
 
+static void safe_run_hooks_maybe_narrowed (Lisp_Object, struct window *);
+
 /* Incremented whenever a timer is run.  */
 unsigned timers_run;
 
@@ -1909,7 +1911,7 @@ safe_run_hooks (Lisp_Object hook)
   unbind_to (count, Qnil);
 }
 
-void
+static void
 safe_run_hooks_maybe_narrowed (Lisp_Object hook, struct window *w)
 {
   specpdl_ref count = SPECPDL_INDEX ();
@@ -1919,11 +1921,11 @@ safe_run_hooks_maybe_narrowed (Lisp_Object hook, struct window *w)
   if (current_buffer->long_line_optimizations_p
       && long_line_optimizations_region_size > 0)
     {
-      ptrdiff_t begv = get_locked_narrowing_begv (PT);
-      ptrdiff_t zv = get_locked_narrowing_zv (PT);
+      ptrdiff_t begv = get_large_narrowing_begv (PT);
+      ptrdiff_t zv = get_large_narrowing_zv (PT);
       if (begv != BEG || zv != Z)
-	narrow_to_region_locked (make_fixnum (begv), make_fixnum (zv),
-				 Qlong_line_optimizations_in_command_hooks);
+	labeled_narrow_to_region (make_fixnum (begv), make_fixnum (zv),
+				  Qlong_line_optimizations_in_command_hooks);
     }
 
   run_hook_with_args (2, ((Lisp_Object []) {hook, hook}),
diff --git a/src/lisp.h b/src/lisp.h
index 1276285e2f2..9c02d975a74 100644
--- a/src/lisp.h
+++ b/src/lisp.h
@@ -4687,8 +4687,9 @@ extern void save_restriction_restore (Lisp_Object);
 extern Lisp_Object make_buffer_string (ptrdiff_t, ptrdiff_t, bool);
 extern Lisp_Object make_buffer_string_both (ptrdiff_t, ptrdiff_t, ptrdiff_t,
 					    ptrdiff_t, bool);
-extern void narrow_to_region_locked (Lisp_Object, Lisp_Object, Lisp_Object);
-extern void reset_outermost_narrowings (void);
+extern void labeled_narrow_to_region (Lisp_Object, Lisp_Object, Lisp_Object);
+extern void reset_outermost_restrictions (void);
+extern void labeled_restrictions_remove_in_current_buffer (void);
 extern void init_editfns (void);
 extern void syms_of_editfns (void);
 
@@ -4857,7 +4858,6 @@ extern bool detect_input_pending (void);
 extern bool detect_input_pending_ignore_squeezables (void);
 extern bool detect_input_pending_run_timers (bool);
 extern void safe_run_hooks (Lisp_Object);
-extern void safe_run_hooks_maybe_narrowed (Lisp_Object, struct window *);
 extern void safe_run_hooks_2 (Lisp_Object, Lisp_Object, Lisp_Object);
 extern void cmd_error_internal (Lisp_Object, const char *);
 extern Lisp_Object command_loop_2 (Lisp_Object);
diff --git a/src/lread.c b/src/lread.c
index d0dc85f51c8..342d367d985 100644
--- a/src/lread.c
+++ b/src/lread.c
@@ -2255,6 +2255,7 @@ readevalloop (Lisp_Object readcharfun,
 	  record_unwind_protect_excursion ();
 	  /* Save ZV in it.  */
 	  record_unwind_protect (save_restriction_restore, save_restriction_save ());
+	  labeled_restrictions_remove_in_current_buffer ();
 	  /* Those get unbound after we read one expression.  */
 
 	  /* Set point and ZV around stuff to be read.  */
diff --git a/src/xdisp.c b/src/xdisp.c
index 0b190529404..30a32896aba 100644
--- a/src/xdisp.c
+++ b/src/xdisp.c
@@ -3482,7 +3482,7 @@ init_iterator (struct it *it, struct window *w,
 
       /* This is set only when long_line_optimizations_p is non-zero
 	 for the current buffer.  */
-      it->narrowed_begv = 0;
+      it->medium_narrowing_begv = 0;
 
       /* Compute faces etc.  */
       reseat (it, it->current.pos, true);
@@ -3491,17 +3491,104 @@ init_iterator (struct it *it, struct window *w,
   CHECK_IT (it);
 }
 
-/* Compute a suitable alternate value for BEGV and ZV that may be used
-   temporarily to optimize display if the buffer in window W contains
-   long lines.  */
+/* How Emacs deals with long lines.
+
+   (1) When a buffer is about to be (re)displayed, 'redisplay_window'
+   detects, with a heuristic, whether it contains long lines.
+
+   This happens in 'redisplay_window' because it is only displaying
+   buffers with long lines that is problematic.  In other words, none
+   of the optimizations described below is ever used in buffers that
+   are never displayed.
+
+   This happens with a heuristic, which checks whether a buffer
+   contains long lines, each time its contents have changed "enough"
+   between two redisplay cycles, because a buffer without long lines
+   can become a buffer with long lines at any time, for example after
+   a yank command, or after a replace command, or while the output of
+   an external process is inserted in a buffer.
+
+   When Emacs has detected that a buffer contains long lines, the
+   buffer-local variable 'long_line_optimizations_p' (in 'struct
+   buffer') is set, and Emacs does not try to detect whether the
+   buffer does or does not contain long lines anymore.
+
+   What a long line is depends on the variable 'long-line-threshold',
+   whose default value is 50000 (characters).
+
+   (2) When a buffer with long lines is (re)displayed, the amount of
+   data that the display routines consider is, in a few well-chosen
+   places, limited with a temporary restriction, whose bounds are
+   calculated with the functions below.
+
+   (2.1) 'get_small_narrowing_begv' is used to create a restriction
+   which starts a few hundred characters before point.  The exact
+   number of characters depends on the width of the window in which
+   the buffer is displayed.
+
+   There is no corresponding 'get_small_narrowing_zv' function,
+   because it is not necessary to set the end limit of that
+   restriction.
+
+   This restriction is used in four places, namely:
+   'back_to_previous_line_start' and 'move_it_vertically_backward'
+   (with the 'SET_WITH_NARROWED_BEGV' macro), and in
+   'composition_compute_stop_pos' and 'find_automatic_composition' (in
+   a conditional statement depending on 'long_line_optimizations_p').
+
+   (2.2) 'get_medium_narrowing_begv' is used to create a restriction
+   which starts a few thousand characters before point.  The exact
+   number of characters depends on the size (width and height) of the
+   window in which the buffer is displayed.  For performance reasons,
+   the return value of that function is cached in 'struct it', in the
+   'medium_narrowing_begv' field.
+
+   The corresponding function 'get_medium_narrowing_zv' (and
+   'medium_narrowing_zv' field in 'struct it') is not used to set the
+   end limit of a the restriction, which is again unnecessary, but to
+   determine, in 'reseat', whether the iterator has moved far enough
+   from its original position, and whether the start position of the
+   restriction must be computed anew.
+
+   This restriction is used in a single place:
+   'get_visually_first_element', with the 'SET_WITH_NARROWED_BEGV'
+   macro.
+
+   (2.3) 'get_large_narrowing_begv' and 'get_large_narrowing_zv' are
+   used to create a restriction which starts a few hundred thousand
+   characters before point and ends a few hundred thousand characters
+   after point.  The size of that restriction depends on the variable
+   'long-line-optimizations-region-size', whose default value is
+   500000 (characters); it can be adjusted by a few hundred characters
+   depending on 'long-line-optimizations-bol-search-limit', whose
+   default value is 128 (characters).
+
+   For performance reasons again, the return values of these functions
+   are stored in the 'large_narrowing_begv' and 'large_narrowing_zv'
+   fields in 'struct it'.
+
+   The restriction defined by these values is used around three
+   low-level hooks: around 'fontification-functions', in
+   'handle_fontified_prop', and around 'pre-command-hook' and
+   'post-command-hook', in 'safe_run_hooks_maybe_narrowed', which is
+   called in 'command_loop_1'.  These restrictions are set around
+   these hooks with 'labeled_narrow_to_region'; the restrictions are
+   labeled, and cannot be removed with a call to 'widen', but can be
+   removed with 'without-restriction' with a :label argument.
+*/
 
 static int
 get_narrowed_width (struct window *w)
 {
   /* In a character-only terminal, only one font size is used, so we
      can use a smaller factor.  */
-  int fact = EQ (Fterminal_live_p (Qnil), Qt) ? 2 : 3;
-  int width = window_body_width (w, WINDOW_BODY_IN_CANONICAL_CHARS);
+  int fact = FRAME_WINDOW_P (XFRAME (w->frame)) ? 3 : 2;
+  /* If the window has no fringes (in a character-only terminal or in
+     a GUI frame without fringes), subtract 1 from the width for the
+     '\' line wrapping character.  */
+  int width = window_body_width (w, WINDOW_BODY_IN_CANONICAL_CHARS)
+    - ((WINDOW_RIGHT_FRINGE_WIDTH (w) == 0
+	|| WINDOW_LEFT_FRINGE_WIDTH (w) == 0) ? 1 : 0);
   return fact * max (1, width);
 }
 
@@ -3512,29 +3599,57 @@ get_narrowed_len (struct window *w)
   return get_narrowed_width (w) * max (1, height);
 }
 
-ptrdiff_t
-get_narrowed_begv (struct window *w, ptrdiff_t pos)
+static ptrdiff_t
+get_medium_narrowing_begv (struct window *w, ptrdiff_t pos)
 {
   int len = get_narrowed_len (w);
   return max ((pos / len - 1) * len, BEGV);
 }
 
-ptrdiff_t
-get_narrowed_zv (struct window *w, ptrdiff_t pos)
+static ptrdiff_t
+get_medium_narrowing_zv (struct window *w, ptrdiff_t pos)
 {
   int len = get_narrowed_len (w);
   return min ((pos / len + 1) * len, ZV);
 }
 
+static ptrdiff_t
+get_nearby_bol_pos (ptrdiff_t pos)
+{
+  ptrdiff_t start, pos_bytepos, cur, next, found, bol = BEGV - 1;
+  int dist;
+  for (dist = 500; dist <= 500000; dist *= 10)
+    {
+      pos_bytepos = pos == BEGV ? BEGV_BYTE : CHAR_TO_BYTE (pos);
+      start = pos - dist < BEGV ? BEGV : pos - dist;
+      for (cur = start; cur < pos; cur = next)
+	{
+	  next = find_newline1 (cur, CHAR_TO_BYTE (cur),
+				pos, pos_bytepos,
+				1, &found, NULL, false);
+	  if (found)
+	    bol = next;
+	  else
+	    break;
+	}
+      if (bol >= BEGV || start == BEGV)
+	return bol;
+      else
+	pos = pos - dist < BEGV ? BEGV : pos - dist;
+    }
+  return bol;
+}
+
 ptrdiff_t
-get_closer_narrowed_begv (struct window *w, ptrdiff_t pos)
+get_small_narrowing_begv (struct window *w, ptrdiff_t pos)
 {
   int len = get_narrowed_width (w);
-  return max ((pos / len - 1) * len, BEGV);
+  ptrdiff_t bol_pos = max (get_nearby_bol_pos (pos), BEGV);
+  return max (bol_pos + ((pos - bol_pos) / len - 1) * len, BEGV);
 }
 
 ptrdiff_t
-get_locked_narrowing_begv (ptrdiff_t pos)
+get_large_narrowing_begv (ptrdiff_t pos)
 {
   if (long_line_optimizations_region_size <= 0)
     return BEGV;
@@ -3552,7 +3667,7 @@ get_locked_narrowing_begv (ptrdiff_t pos)
 }
 
 ptrdiff_t
-get_locked_narrowing_zv (ptrdiff_t pos)
+get_large_narrowing_zv (ptrdiff_t pos)
 {
   if (long_line_optimizations_region_size <= 0)
     return ZV;
@@ -3571,7 +3686,7 @@ unwind_narrowed_begv (Lisp_Object point_min)
 
 #define SET_WITH_NARROWED_BEGV(IT,DST,EXPR,BV)				\
   do {									\
-    if (IT->narrowed_begv)						\
+    if (IT->medium_narrowing_begv)					\
       {									\
 	specpdl_ref count = SPECPDL_INDEX ();				\
 	record_unwind_protect (unwind_narrowed_begv, Fpoint_min ());	\
@@ -4396,17 +4511,17 @@ handle_fontified_prop (struct it *it)
       if (current_buffer->long_line_optimizations_p
 	  && long_line_optimizations_region_size > 0)
 	{
-	  ptrdiff_t begv = it->locked_narrowing_begv;
-	  ptrdiff_t zv = it->locked_narrowing_zv;
+	  ptrdiff_t begv = it->large_narrowing_begv;
+	  ptrdiff_t zv = it->large_narrowing_zv;
 	  ptrdiff_t charpos = IT_CHARPOS (*it);
 	  if (charpos < begv || charpos > zv)
 	    {
-	      begv = get_locked_narrowing_begv (charpos);
-	      zv = get_locked_narrowing_zv (charpos);
+	      begv = get_large_narrowing_begv (charpos);
+	      zv = get_large_narrowing_zv (charpos);
 	    }
 	  if (begv != BEG || zv != Z)
-	    narrow_to_region_locked (make_fixnum (begv), make_fixnum (zv),
-				     Qlong_line_optimizations_in_fontification_functions);
+	    labeled_narrow_to_region (make_fixnum (begv), make_fixnum (zv),
+				      Qlong_line_optimizations_in_fontification_functions);
 	}
 
       /* Don't allow Lisp that runs from 'fontification-functions'
@@ -7041,7 +7156,7 @@ back_to_previous_line_start (struct it *it)
   dec_both (&cp, &bp);
   SET_WITH_NARROWED_BEGV (it, IT_CHARPOS (*it),
 			  find_newline_no_quit (cp, bp, -1, &IT_BYTEPOS (*it)),
-			  get_closer_narrowed_begv (it->w, IT_CHARPOS (*it)));
+			  get_small_narrowing_begv (it->w, IT_CHARPOS (*it)));
 }
 
 /* Find in the current buffer the first display or overlay string
@@ -7345,7 +7460,7 @@ back_to_previous_visible_line_start (struct it *it)
   it->continuation_lines_width = 0;
 
   eassert (IT_CHARPOS (*it) >= BEGV);
-  eassert (it->narrowed_begv > 0 /* long-line optimizations: all bets off */
+  eassert (it->medium_narrowing_begv > 0 /* long-line optimizations: all bets off */
 	   || IT_CHARPOS (*it) == BEGV
 	   || FETCH_BYTE (IT_BYTEPOS (*it) - 1) == '\n');
   CHECK_IT (it);
@@ -7463,24 +7578,29 @@ reseat (struct it *it, struct text_pos pos, bool force_p)
 
   if (current_buffer->long_line_optimizations_p)
     {
-      if (!it->narrowed_begv)
+      if (!it->medium_narrowing_begv)
 	{
-	  it->narrowed_begv = get_narrowed_begv (it->w, window_point (it->w));
-	  it->narrowed_zv = get_narrowed_zv (it->w, window_point (it->w));
-	  it->locked_narrowing_begv
-	    = get_locked_narrowing_begv (window_point (it->w));
-	  it->locked_narrowing_zv
-	    = get_locked_narrowing_zv (window_point (it->w));
+	  it->medium_narrowing_begv
+	    = get_medium_narrowing_begv (it->w, window_point (it->w));
+	  it->medium_narrowing_zv
+	    = get_medium_narrowing_zv (it->w, window_point (it->w));
+	  it->large_narrowing_begv
+	    = get_large_narrowing_begv (window_point (it->w));
+	  it->large_narrowing_zv
+	    = get_large_narrowing_zv (window_point (it->w));
 	}
-      else if ((pos.charpos < it->narrowed_begv || pos.charpos > it->narrowed_zv)
+      else if ((pos.charpos < it->medium_narrowing_begv
+		|| pos.charpos > it->medium_narrowing_zv)
 		&& (!redisplaying_p || it->line_wrap == TRUNCATE))
 	{
-	  it->narrowed_begv = get_narrowed_begv (it->w, pos.charpos);
-	  it->narrowed_zv = get_narrowed_zv (it->w, pos.charpos);
-	  it->locked_narrowing_begv
-	    = get_locked_narrowing_begv (window_point (it->w));
-	  it->locked_narrowing_zv
-	    = get_locked_narrowing_zv (window_point (it->w));
+	  it->medium_narrowing_begv
+	    = get_medium_narrowing_begv (it->w, pos.charpos);
+	  it->medium_narrowing_zv
+	    = get_medium_narrowing_zv (it->w, pos.charpos);
+	  it->large_narrowing_begv
+	    = get_large_narrowing_begv (window_point (it->w));
+	  it->large_narrowing_zv
+	    = get_large_narrowing_zv (window_point (it->w));
 	}
     }
 
@@ -8789,7 +8909,7 @@ get_visually_first_element (struct it *it)
   SET_WITH_NARROWED_BEGV (it, bob,
 			  string_p ? 0 :
 			  IT_CHARPOS (*it) < BEGV ? obegv : BEGV,
-			  it->narrowed_begv);
+			  it->medium_narrowing_begv);
 
   if (STRINGP (it->string))
     {
@@ -8833,7 +8953,7 @@ get_visually_first_element (struct it *it)
 				find_newline_no_quit (IT_CHARPOS (*it),
 						      IT_BYTEPOS (*it), -1,
 						      &it->bidi_it.bytepos),
-				it->narrowed_begv);
+				it->medium_narrowing_begv);
       bidi_paragraph_init (it->paragraph_embedding, &it->bidi_it, true);
       do
 	{
@@ -10722,7 +10842,7 @@ move_it_vertically_backward (struct it *it, int dy)
 	  dec_both (&cp, &bp);
 	  SET_WITH_NARROWED_BEGV (it, cp,
 				  find_newline_no_quit (cp, bp, -1, NULL),
-				  get_closer_narrowed_begv (it->w, IT_CHARPOS (*it)));
+				  get_small_narrowing_begv (it->w, IT_CHARPOS (*it)));
 	  move_it_to (it, cp, -1, -1, -1, MOVE_TO_POS);
 	}
       bidi_unshelve_cache (it3data, true);
@@ -16394,7 +16514,7 @@ redisplay_internal (void)
   FOR_EACH_FRAME (tail, frame)
     XFRAME (frame)->already_hscrolled_p = false;
 
-  reset_outermost_narrowings ();
+  reset_outermost_restrictions ();
 
  retry:
   /* Remember the currently selected window.  */
@@ -24112,6 +24232,7 @@ display_count_lines_logically (ptrdiff_t start_byte, ptrdiff_t limit_byte,
   ptrdiff_t val;
   specpdl_ref pdl_count = SPECPDL_INDEX ();
   record_unwind_protect (save_restriction_restore, save_restriction_save ());
+  labeled_restrictions_remove_in_current_buffer ();
   Fwiden ();
   val = display_count_lines (start_byte, limit_byte, count, byte_pos_ptr);
   unbind_to (pdl_count, Qnil);

bug#56682: feature/improved-locked-narrowing 9dee6df39c: Reworked locked narrowing.

[Eli Zaretskii]

> Date: Thu, 04 May 2023 15:45:29 +0000
> From: Gregory Heytings <gregory@heytings.org>
> cc: 56682@debbugs.gnu.org, monnier@iro.umontreal.ca
> 
> > Please show the full diffs of the proposed merge, I will have to think 
> > about this again, and think hard.
> >
> 
> You can get them by checking out the scratch/long-lines-cleanup and doing 
> "git diff emacs-29..."  Just in case, I attach the full diff.  Most of it 
> are renamings and documentation improvements.  The main code change is the 
> function 'get_nearby_bol_pos' used by 'get_small_narrowing_begv'.

That's not all it changes, see below.

> -  for (val = narrowing_locks; CONSP (val); val = XCDR (val))
> +  for (val = labeled_restrictions; CONSP (val); val = XCDR (val))
>      {
>        buf = XCAR (XCAR (val));
>        eassert (BUFFERP (buf));
> -      struct Lisp_Marker *begv = narrowing_lock_get_bound (buf, true, true);
> -      struct Lisp_Marker *zv = narrowing_lock_get_bound (buf, false, true);
> +      struct Lisp_Marker *begv
> +	= labeled_restrictions_get_bound (buf, true, true);
> +      struct Lisp_Marker *zv
> +	= labeled_restrictions_get_bound (buf, false, true);
>        if (begv != NULL && zv != NULL)
>  	{

Why the strange design of having a function return a pointer to a
'struct Lisp_Marker'? why not return the marker itself instead?  (I
realize that this was so in the code we already have, but I still
don't understand why you did it that way, and prefer that function to
return a marker instead.)

> diff --git a/src/fileio.c b/src/fileio.c
> index f00c389a520..b50b3c6b935 100644
> --- a/src/fileio.c
> +++ b/src/fileio.c
> @@ -5269,6 +5269,7 @@ write_region (Lisp_Object start, Lisp_Object end, Lisp_Object filename,
>      }
>  
>    record_unwind_protect (save_restriction_restore, save_restriction_save ());
> +  labeled_restrictions_remove_in_current_buffer ();

Why are we removing the restrictions as part of write-region?

> diff --git a/src/indent.c b/src/indent.c
> index 08d2bf5ea28..aef394dab88 100644
> --- a/src/indent.c
> +++ b/src/indent.c
> @@ -2065,6 +2065,7 @@ line_number_display_width (struct window *w, int *width, int *pixel_width)
>  	{
>  	  record_unwind_protect (save_restriction_restore,
>  				 save_restriction_save ());
> +	  labeled_restrictions_remove_in_current_buffer ();
>  	  Fwiden ();
>  	  saved_restriction = true;
>  	}

And why here?

> --- a/src/lread.c
> +++ b/src/lread.c
> @@ -2255,6 +2255,7 @@ readevalloop (Lisp_Object readcharfun,
>  	  record_unwind_protect_excursion ();
>  	  /* Save ZV in it.  */
>  	  record_unwind_protect (save_restriction_restore, save_restriction_save ());
> +	  labeled_restrictions_remove_in_current_buffer ();
>  	  /* Those get unbound after we read one expression.  */

And here?

> +   The corresponding function 'get_medium_narrowing_zv' (and
> +   'medium_narrowing_zv' field in 'struct it') is not used to set the
> +   end limit of a the restriction, which is again unnecessary, but to
                   ^^^^^
Typo.

> +static ptrdiff_t
> +get_nearby_bol_pos (ptrdiff_t pos)
> +{
> +  ptrdiff_t start, pos_bytepos, cur, next, found, bol = BEGV - 1;
> +  int dist;
> +  for (dist = 500; dist <= 500000; dist *= 10)
> +    {
> +      pos_bytepos = pos == BEGV ? BEGV_BYTE : CHAR_TO_BYTE (pos);
> +      start = pos - dist < BEGV ? BEGV : pos - dist;
> +      for (cur = start; cur < pos; cur = next)
> +	{
> +	  next = find_newline1 (cur, CHAR_TO_BYTE (cur),
> +				pos, pos_bytepos,
> +				1, &found, NULL, false);
> +	  if (found)
> +	    bol = next;
> +	  else
> +	    break;
> +	}
> +      if (bol >= BEGV || start == BEGV)
> +	return bol;
> +      else
> +	pos = pos - dist < BEGV ? BEGV : pos - dist;
> +    }
> +  return bol;
> +}

This function should have a commentary describing what it does.

Is it okay for this function to return a position > POS, its input?

> @@ -24112,6 +24232,7 @@ display_count_lines_logically (ptrdiff_t start_byte, ptrdiff_t limit_byte,
>    ptrdiff_t val;
>    specpdl_ref pdl_count = SPECPDL_INDEX ();
>    record_unwind_protect (save_restriction_restore, save_restriction_save ());
> +  labeled_restrictions_remove_in_current_buffer ();
>    Fwiden ();
>    val = display_count_lines (start_byte, limit_byte, count, byte_pos_ptr);
>    unbind_to (pdl_count, Qnil);

Why do we remove the restrictions here?

bug#56682: feature/improved-locked-narrowing 9dee6df39c: Reworked locked narrowing.

[Gregory Heytings]

Thanks for your detailed review.

>> +      struct Lisp_Marker *begv
>> +	= labeled_restrictions_get_bound (buf, true, true);
>> +      struct Lisp_Marker *zv
>> +	= labeled_restrictions_get_bound (buf, false, true);
>
> Why the strange design of having a function return a pointer to a 
> 'struct Lisp_Marker'? why not return the marker itself instead?  (I 
> realize that this was so in the code we already have, but I still don't 
> understand why you did it that way, and prefer that function to return a 
> marker instead.)
>

Good question.  You mean that it would have been better to return a 
Lisp_Object, right?  I don't recall exactly, I think it was because in the 
calls to SET_BUF_BEGV_BOTH/SET_BUF_ZV_BOTH (which are the only places 
where the return value of labeled_restrictions_get_bound are used) one can 
use the pointer to a struct Lisp_Marker immediately, whereas a call to 
XMARKER would have been necessary if a Lisp_Object had been used.

>>   record_unwind_protect (save_restriction_restore, save_restriction_save ());
>> + labeled_restrictions_remove_in_current_buffer ();
>
> Why are we removing the restrictions as part of write-region?
>

We are removing them temporarily, just before the Fwiden, and they are 
restored by save_restriction_restore.

>>  	  record_unwind_protect (save_restriction_restore,
>>  				 save_restriction_save ());
>> +	  labeled_restrictions_remove_in_current_buffer ();
>>  	  Fwiden ();
>
> And why here?
>

For the same reason: calls to Fwiden which are preceded by a 
"record_unwind_protect (save_restriction_restore, save_restriction_save 
());" are meant to temporarily widen the buffer, and restore the 
restrictions upon returning from the function.  So we temporarily remove 
labeled restrictions as well (and they are restored by 
save_restriction_restore, too).

>>  	  record_unwind_protect (save_restriction_restore, save_restriction_save ());
>> +	  labeled_restrictions_remove_in_current_buffer ();
>
> And here?
>

For the same reason again ;-)

>>    record_unwind_protect (save_restriction_restore, save_restriction_save ());
>> +  labeled_restrictions_remove_in_current_buffer ();
>>    Fwiden ();
>>    val = display_count_lines (start_byte, limit_byte, count, byte_pos_ptr);
>
> Why do we remove the restrictions here?
>

... and again ;-)

>> +   The corresponding function 'get_medium_narrowing_zv' (and
>> +   'medium_narrowing_zv' field in 'struct it') is not used to set the
>> +   end limit of a the restriction, which is again unnecessary, but to
>                   ^^^^^
> Typo.
>

Good catch, thanks!

>> +static ptrdiff_t
>> +get_nearby_bol_pos (ptrdiff_t pos)
>> +{
>> +  ptrdiff_t start, pos_bytepos, cur, next, found, bol = BEGV - 1;
>> +  int dist;
>> +  for (dist = 500; dist <= 500000; dist *= 10)
>> +    {
>> +      pos_bytepos = pos == BEGV ? BEGV_BYTE : CHAR_TO_BYTE (pos);
>> +      start = pos - dist < BEGV ? BEGV : pos - dist;
>> +      for (cur = start; cur < pos; cur = next)
>> +	{
>> +	  next = find_newline1 (cur, CHAR_TO_BYTE (cur),
>> +				pos, pos_bytepos,
>> +				1, &found, NULL, false);
>> +	  if (found)
>> +	    bol = next;
>> +	  else
>> +	    break;
>> +	}
>> +      if (bol >= BEGV || start == BEGV)
>> +	return bol;
>> +      else
>> +	pos = pos - dist < BEGV ? BEGV : pos - dist;
>> +    }
>> +  return bol;
>> +}
>
> This function should have a commentary describing what it does.
>

Okay, I'll add that.

>
> Is it okay for this function to return a position > POS, its input?
>

Unless I misunderstood something, it cannot, because find_newline1 is 
called with end = pos and end_byte = pos_bytepos.

bug#56682: feature/improved-locked-narrowing 9dee6df39c: Reworked locked narrowing.

[Eli Zaretskii]

> Date: Fri, 05 May 2023 21:29:16 +0000
> From: Gregory Heytings <gregory@heytings.org>
> cc: 56682@debbugs.gnu.org, monnier@iro.umontreal.ca
> 
> >> +      struct Lisp_Marker *begv
> >> +	= labeled_restrictions_get_bound (buf, true, true);
> >> +      struct Lisp_Marker *zv
> >> +	= labeled_restrictions_get_bound (buf, false, true);
> >
> > Why the strange design of having a function return a pointer to a 
> > 'struct Lisp_Marker'? why not return the marker itself instead?  (I 
> > realize that this was so in the code we already have, but I still don't 
> > understand why you did it that way, and prefer that function to return a 
> > marker instead.)
> >
> 
> Good question.  You mean that it would have been better to return a 
> Lisp_Object, right?  I don't recall exactly, I think it was because in the 
> calls to SET_BUF_BEGV_BOTH/SET_BUF_ZV_BOTH (which are the only places 
> where the return value of labeled_restrictions_get_bound are used) one can 
> use the pointer to a struct Lisp_Marker immediately, whereas a call to 
> XMARKER would have been necessary if a Lisp_Object had been used.

I'd prefer to use a marker there, but that can be a separate
changeset.

> > Is it okay for this function to return a position > POS, its input?
> 
> Unless I misunderstood something, it cannot, because find_newline1 is 
> called with end = pos and end_byte = pos_bytepos.

The logic is quite convoluted, so I think we should have an assertion
about this before the function returns, because callers depend on the
returned position not to exceed POS, AFAICT.

Please install this after fixing those nits, and please ack this time
after installing.

Thanks.

bug#56682: feature/improved-locked-narrowing 9dee6df39c: Reworked locked narrowing.

[Gregory Heytings]

[-- Attachment #1: Type: text/plain, Size: 1490 bytes --]


>>>> +      struct Lisp_Marker *begv
>>>> +	= labeled_restrictions_get_bound (buf, true, true);
>>>> +      struct Lisp_Marker *zv
>>>> +	= labeled_restrictions_get_bound (buf, false, true);
>>>
>>> Why the strange design of having a function return a pointer to a 
>>> 'struct Lisp_Marker'? why not return the marker itself instead?  (I 
>>> realize that this was so in the code we already have, but I still 
>>> don't understand why you did it that way, and prefer that function to 
>>> return a marker instead.)
>>
>> Good question.  You mean that it would have been better to return a 
>> Lisp_Object, right?  I don't recall exactly, I think it was because in 
>> the calls to SET_BUF_BEGV_BOTH/SET_BUF_ZV_BOTH (which are the only 
>> places where the return value of labeled_restrictions_get_bound are 
>> used) one can use the pointer to a struct Lisp_Marker immediately, 
>> whereas a call to XMARKER would have been necessary if a Lisp_Object 
>> had been used.
>
> I'd prefer to use a marker there, but that can be a separate changeset.
>

Can you confirm that the attached patch is what you would prefer?  IMHO it 
is better/clearer to have a single call to XMARKER in 
'labeled_restrictions_get_bound', instead of having to repeat it 
everywhere its return value is used.  The price is indeed that the 
signature of that (internal) function is unusual.  (Note that we cannot 
wrap the calls to 'labeled_restrictions_get_bound' in a call to XMARKER, 
because it can return nil.)

[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: editfns.patch --]
[-- Type: text/x-diff; name=editfns.patch, Size: 6106 bytes --]

diff --git a/src/editfns.c b/src/editfns.c
index 4c5b691eb50..9d35491a6d8 100644
--- a/src/editfns.c
+++ b/src/editfns.c
@@ -2687,20 +2687,19 @@ labeled_restrictions_remove (Lisp_Object buf)
 }
 
 /* Retrieve one of the labeled restriction bounds in BUF from the
-   labeled_restrictions alist, as a pointer to a struct Lisp_Marker,
-   or return NULL if BUF is not in labeled_restrictions or is a killed
-   buffer.  When OUTERMOST is true, the restriction bounds that were
-   current when the first labeled restriction was entered are
-   returned.  Otherwise the bounds of the innermost labeled
-   restriction are returned.  */
-static struct Lisp_Marker *
+   labeled_restrictions alist, as a marker, or return nil if BUF is
+   not in labeled_restrictions or is a killed buffer.  When OUTERMOST
+   is true, the restriction bounds that were current when the first
+   labeled restriction was entered are returned.  Otherwise the bounds
+   of the innermost labeled restriction are returned.  */
+static Lisp_Object
 labeled_restrictions_get_bound (Lisp_Object buf, bool begv, bool outermost)
 {
   if (NILP (Fbuffer_live_p (buf)))
-    return NULL;
+    return Qnil;
   Lisp_Object restrictions = assq_no_quit (buf, labeled_restrictions);
   if (NILP (restrictions))
-    return NULL;
+    return Qnil;
   restrictions = XCAR (XCDR (restrictions));
   Lisp_Object bounds
     = outermost
@@ -2709,7 +2708,7 @@ labeled_restrictions_get_bound (Lisp_Object buf, bool begv, bool outermost)
   eassert (! NILP (bounds));
   Lisp_Object marker = begv ? XCAR (bounds) : XCAR (XCDR (bounds));
   eassert (EQ (Fmarker_buffer (marker), buf));
-  return XMARKER (marker);
+  return marker;
 }
 
 /* Retrieve the label of the innermost labeled restriction in BUF.
@@ -2766,14 +2765,14 @@ labeled_restrictions_remove_in_current_buffer (void)
 static void
 unwind_reset_outermost_restriction (Lisp_Object buf)
 {
-  struct Lisp_Marker *begv
-    = labeled_restrictions_get_bound (buf, true, false);
-  struct Lisp_Marker *zv
-    = labeled_restrictions_get_bound (buf, false, false);
-  if (begv != NULL && zv != NULL)
+  Lisp_Object begv = labeled_restrictions_get_bound (buf, true, false);
+  Lisp_Object zv = labeled_restrictions_get_bound (buf, false, false);
+  if (! NILP (begv) && ! NILP (zv))
     {
-      SET_BUF_BEGV_BOTH (XBUFFER (buf), begv->charpos, begv->bytepos);
-      SET_BUF_ZV_BOTH (XBUFFER (buf), zv->charpos, zv->bytepos);
+      SET_BUF_BEGV_BOTH (XBUFFER (buf),
+			 XMARKER (begv)->charpos, XMARKER (begv)->bytepos);
+      SET_BUF_ZV_BOTH (XBUFFER (buf),
+		       XMARKER (zv)->charpos, XMARKER (zv)->bytepos);
     }
   else
     labeled_restrictions_remove (buf);
@@ -2797,14 +2796,14 @@ reset_outermost_restrictions (void)
     {
       buf = XCAR (XCAR (val));
       eassert (BUFFERP (buf));
-      struct Lisp_Marker *begv
-	= labeled_restrictions_get_bound (buf, true, true);
-      struct Lisp_Marker *zv
-	= labeled_restrictions_get_bound (buf, false, true);
-      if (begv != NULL && zv != NULL)
+      Lisp_Object begv = labeled_restrictions_get_bound (buf, true, true);
+      Lisp_Object zv = labeled_restrictions_get_bound (buf, false, true);
+      if (! NILP (begv) && ! NILP (zv))
 	{
-	  SET_BUF_BEGV_BOTH (XBUFFER (buf), begv->charpos, begv->bytepos);
-	  SET_BUF_ZV_BOTH (XBUFFER (buf), zv->charpos, zv->bytepos);
+	  SET_BUF_BEGV_BOTH (XBUFFER (buf),
+			     XMARKER (begv)->charpos, XMARKER (begv)->bytepos);
+	  SET_BUF_ZV_BOTH (XBUFFER (buf),
+			   XMARKER (zv)->charpos, XMARKER (zv)->bytepos);
 	  record_unwind_protect (unwind_reset_outermost_restriction, buf);
 	}
       else
@@ -2878,15 +2877,15 @@ DEFUN ("widen", Fwiden, Swiden, 0, 0, "",
     }
   else
     {
-      struct Lisp_Marker *begv
-	= labeled_restrictions_get_bound (buf, true, false);
-      struct Lisp_Marker *zv
-	= labeled_restrictions_get_bound (buf, false, false);
-      eassert (begv != NULL && zv != NULL);
-      if (begv->charpos != BEGV || zv->charpos != ZV)
+      Lisp_Object begv = labeled_restrictions_get_bound (buf, true, false);
+      Lisp_Object zv = labeled_restrictions_get_bound (buf, false, false);
+      eassert (! NILP (begv) && ! NILP (zv));
+      if (XMARKER (begv)->charpos != BEGV || XMARKER (zv)->charpos != ZV)
 	current_buffer->clip_changed = 1;
-      SET_BUF_BEGV_BOTH (current_buffer, begv->charpos, begv->bytepos);
-      SET_BUF_ZV_BOTH (current_buffer, zv->charpos, zv->bytepos);
+      SET_BUF_BEGV_BOTH (current_buffer,
+			 XMARKER (begv)->charpos, XMARKER (begv)->bytepos);
+      SET_BUF_ZV_BOTH (current_buffer,
+		       XMARKER (zv)->charpos, XMARKER (zv)->bytepos);
       /* If the only remaining bounds in labeled_restrictions for
 	 current_buffer are the bounds that were set by the user, no
 	 labeled restriction is in effect in current_buffer anymore:
@@ -2933,15 +2932,13 @@ positions (integers or markers) bounding the text that should
     {
       /* Limit the start and end positions to those of the innermost
 	 labeled restriction.  */
-      struct Lisp_Marker *begv
-	= labeled_restrictions_get_bound (buf, true, false);
-      struct Lisp_Marker *zv
-	= labeled_restrictions_get_bound (buf, false, false);
-      eassert (begv != NULL && zv != NULL);
-      if (s < begv->charpos) s = begv->charpos;
-      if (s > zv->charpos) s = zv->charpos;
-      if (e < begv->charpos) e = begv->charpos;
-      if (e > zv->charpos) e = zv->charpos;
+      Lisp_Object begv = labeled_restrictions_get_bound (buf, true, false);
+      Lisp_Object zv = labeled_restrictions_get_bound (buf, false, false);
+      eassert (! NILP (begv) && ! NILP (zv));
+      if (s < XMARKER (begv)->charpos) s = XMARKER (begv)->charpos;
+      if (s > XMARKER (zv)->charpos) s = XMARKER (zv)->charpos;
+      if (e < XMARKER (begv)->charpos) e = XMARKER (begv)->charpos;
+      if (e > XMARKER (zv)->charpos) e = XMARKER (zv)->charpos;
     }
 
   /* Record the accessible range of the buffer when narrow-to-region

bug#56682: feature/improved-locked-narrowing 9dee6df39c: Reworked locked narrowing.

[Eli Zaretskii]

> Date: Tue, 09 May 2023 21:48:42 +0000
> From: Gregory Heytings <gregory@heytings.org>
> cc: 56682@debbugs.gnu.org, monnier@iro.umontreal.ca
> 
> > I'd prefer to use a marker there, but that can be a separate changeset.
> >
> 
> Can you confirm that the attached patch is what you would prefer?  IMHO it 
> is better/clearer to have a single call to XMARKER in 
> 'labeled_restrictions_get_bound', instead of having to repeat it 
> everywhere its return value is used.  The price is indeed that the 
> signature of that (internal) function is unusual.  (Note that we cannot 
> wrap the calls to 'labeled_restrictions_get_bound' in a call to XMARKER, 
> because it can return nil.)

Yes, this is what I meant, but please use marker_position and
marker_byte_position instead of accessing the object directly.

Thanks.

bug#56682: feature/improved-locked-narrowing 9dee6df39c: Reworked locked narrowing.

[Eli Zaretskii]

> Cc: 56682@debbugs.gnu.org, monnier@iro.umontreal.ca
> Date: Wed, 10 May 2023 17:00:35 +0300
> From: Eli Zaretskii <eliz@gnu.org>
> 
> > Can you confirm that the attached patch is what you would prefer?  IMHO it 
> > is better/clearer to have a single call to XMARKER in 
> > 'labeled_restrictions_get_bound', instead of having to repeat it 
> > everywhere its return value is used.  The price is indeed that the 
> > signature of that (internal) function is unusual.  (Note that we cannot 
> > wrap the calls to 'labeled_restrictions_get_bound' in a call to XMARKER, 
> > because it can return nil.)
> 
> Yes, this is what I meant, but please use marker_position and
> marker_byte_position instead of accessing the object directly.

Could you please try installing this and merge the branch soon?  I'd
like to released the next pretest with these changes.

Thanks.

bug#56682: feature/improved-locked-narrowing 9dee6df39c: Reworked locked narrowing.

[Gregory Heytings]

>
> Could you please try installing this and merge the branch soon?  I'd 
> like to released the next pretest with these changes.
>

I'll do that in at most a few hours.

bug#56682: feature/improved-locked-narrowing 9dee6df39c: Reworked locked narrowing.

[Gregory Heytings]

>> Could you please try installing this and merge the branch soon?  I'd 
>> like to released the next pretest with these changes.
>
> I'll do that in at most a few hours.
>

Done.

bug#56682: feature/improved-locked-narrowing 9dee6df39c: Reworked locked narrowing.

[Eli Zaretskii]

> Date: Fri, 12 May 2023 22:18:07 +0000
> From: Gregory Heytings <gregory@heytings.org>
> cc: 56682@debbugs.gnu.org, monnier@iro.umontreal.ca
> 
> 
> >> Could you please try installing this and merge the branch soon?  I'd 
> >> like to released the next pretest with these changes.
> >
> > I'll do that in at most a few hours.
> >
> 
> Done.

Thanks.