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

From: Gregory Heytings <[email-masked]>
Date: Thu, 07 Jul 2022 18:38:13 +0000

Thanks for your detailed feedback, Gerd!

>
> From my POV it's ready to go to master.  The sooner more people get to 
> use this the better.
>

Eli (almost) convinced me that there might be a way to do even better.  I 
don't yet see how it can be done, but I think it's worth trying to see if 
it is indeed possible to do better before going to master.

>
> What happens when evaluating an expression in *scratch* that returns a 
> really large result?  Or maybe in a Shell buffer some large output? Does 
> Auto Narrow kick in?  I'm not sure it does.  Should it?
>

It doesn't, indeed.  I'm not sure it should, but ideally it would.

> +      else
> +     {
> +       whole = BUF_Z (buf) - BUF_BEG (buf);
> +       start = marker_position (w->start) - BUF_BEG (buf);
> +       end = BUF_Z (buf) - w->window_end_pos - BUF_BEG (buf);
> +     }
>
> I'd find it easier to read if the if/else were reversed to that the ! 
> isn't needed.
>

I'm not sure what the convention is here (or even if there is a 
convention).  But my feeling is that it's better to place the "normal" 
case first, and the exceptional case in the else part.

> +  if (BUFFER_NEEDS_AUTO_NARROWING_P (current_buffer))
> +    {
> +      safe_call (1, Qauto_narrow_mode);
> +      /* Normally set by auto-narrow-mode, set it here anyway as a safety measure.  */
> +      bset_auto_narrow__narrowing_state (current_buffer, Qauto);
> +      message1 ("Auto-Narrow mode enabled in current buffer");
> +    }
>
> Could you please tell in what circumstances the call would not set the 
> variable?  And wouldn't the minot mode print something, also?  In other 
> words, can we remove it more or less safely?  (If the user screws up, 
> all bets are off anyway.)
>

It should always set the variable indeed, but who knows what a user could 
do if they decide for some reason to override the definition of 
auto-narrow-mode in their init file?  Setting the narrowing state again 
here is a cheap safety measure that avoids calling that function again 
during each redisplay cycle.

I don't know why the minor mode doesn't print something when it's called 
from C, but it doesn't.