Subject: bug#56393: Actually fix the long lines display bug

From: Gregory Heytings <[email-masked]>
Date: Sat, 09 Jul 2022 08:56:16 +0000

>
> Thanks.  A few comments to what I see on the branch (apologies if these 
> are premature because you are still working on the code):
>

I am not, as far as I see the code is in a near-final state, except that I 
will of course try to take your (and other's) comments into account.

>
> . init_iterator is also called to display strings, and I'm not sure I 
> understand what you intended to do in that case, since BEGV/ZV are not 
> relevant to strings.
>

I guess moving the calculation of the restriction into start_display 
should fix that problem.  Then it->narrowed_begv will be 0 for strings.

>
> . The WITH_NARROWED_BEGV macro is IMO awkward and not very convenient to 
> use in C.  For starters, it cannot accommodate multi-line code, except 
> via the 'do { ... } while(0);' kludge, which I think will make the code 
> less readable.  It also should set up an unwind-protect handler, so that 
> any non-local exit from the code will restore BEGV/ZV.  So I think it 
> will be better to have two separate functions (and a third one to 
> unwind).
>

Hmmm...  I'll see what I can do.  (But I don't understand why you write 
that it cannot accomodate multi-line code: you can write 
"WITH_NARROWED_BEGV (frobnicate (foo); frobnicate (bar));".)

>
> . You currently only apply the "restriction" in a few places where the 
> code calls functions like find_newline_no_quit.  What about the rest of 
> display code -- are you saying that it doesn't need to be "restricted"? 
> or do you intend to add that in the future?
>

Yes, that's what I'm saying.  You asked me to make sure that the 
restriction would have the least possible impact, which I did.

>
> And what about restricting the Lisp code invoked via 
> Vfontification_functions (i.e. jit-lock.el)?
>

That would mean that some Lisp code would not have access to the entire 
buffer, which you wanted to avoid.

>
> . Don't we want to "restrict" ZV as well?
>

It's not necessary, no.  Doing so has no performance impact AFAICS.

>
> . I'm not sure the "restriction" should be computed relative to point: 
> that might be OK for redisplaying a window, but for commands that use 
> the display code the position with which we call init_iterator and 
> start_display might be a better/safer alternative, because that position 
> could be very far from point. In any case, get_narrowed_begv receives a 
> pointer to a window, so I think it should use the window-point, not PT, 
> which is the value of point in the current buffer.
>

Okay, I'll do that.

>
> . The 10000 number should at least be a C macro if not a variable 
> exposed to Lisp.
>

Okay.

>
> . The maximum number of characters in a window could be more than just 
> its width multiplied by its height, because the window could use smaller 
> fonts than the default.  I think it is safer to use the dimensions of 
> the window's glyph matrix for this, because there couldn't possibly be 
> more characters in the window than the glyph matrix allows, and we 
> always resize the matrix when we need to display more characters.
>

Okay.