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

From: Gerd Möllmann <[email-masked]>
Date: Thu, 07 Jul 2022 13:29:10 +0200

Looking at the changes in 0463368a7b..7b19ce51fc, I don't have much to
bicker, i.e. except about Magit, lldb, gcc-11, dap-mode, ... :-).

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

Thanks again, Gregory!

P.S.

I currently can't compile Emacs because of reasons, so I can't try it
myself:

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?

modified   src/buffer.c
@@ -6363,6 +6366,10 @@ from (abs POSITION).  If POSITION is positive, point was at the front
 If value is a floating point number, it specifies the spacing relative
 to the default frame line height.  A value of nil means add no extra space.  */);

+  DEFVAR_PER_BUFFER ("auto-narrow--narrowing-state",
+		     &BVAR (current_buffer, auto_narrow__narrowing_state), Qnil,
+		     doc: /* Internal variable used by `auto-narrow-mode'.  */);
+

Don't know about the "--" in the name.  AFAICS, no other per-buffer
variable has that. Likewise the "__" in the name.

Not that it is important.  I just noticed it.  And, maybe it's some
convention that I don't know.

@@ -832,6 +835,11 @@ bset_width_table (struct buffer *b, Lisp_Object val)
 {
   b->width_table_ = val;
 }
+INLINE void
+bset_auto_narrow__narrowing_state (struct buffer *b, Lisp_Object val)
+{
+  b->auto_narrow__narrowing_state_ = val;
+}

If someone feels like it, could you tell me what the '[bw]set_.*'
business is for?  A serializer?  Or for setting breakpoints?

@@ -6557,6 +6557,11 @@ DEFUN ("recenter", Frecenter, Srecenter, 0, 2, "P\np",
   if (buf != current_buffer)
     error ("`recenter'ing a window that does not display current-buffer.");

+  /* Refuse to recenter auto-narrowed buffers that are not actually narrowed,
+     as this can be very slow.  */
+  if (BUFFER_AUTO_NARROWED_NON_NARROWED_P (buf))
+    return Qnil;
+

Hm, I don't know.  Is it always the right thing that recenter does
nothing in this case?  I'm not saying it isn't.

modified   src/xdisp.c
@@ -18872,11 +18872,20 @@ set_vertical_scroll_bar (struct window *w)
 	  && NILP (echo_area_buffer[0])))
     {
       struct buffer *buf = XBUFFER (w->contents);
-      whole = BUF_ZV (buf) - BUF_BEGV (buf);
-      start = marker_position (w->start) - BUF_BEGV (buf);
-      /* I don't think this is guaranteed to be right.  For the
-	 moment, we'll pretend it is.  */
-      end = BUF_Z (buf) - w->window_end_pos - BUF_BEGV (buf);
+      if (! BUFFER_AUTO_NARROWED_P (buf))
+	{
+	  whole = BUF_ZV (buf) - BUF_BEGV (buf);
+	  start = marker_position (w->start) - BUF_BEGV (buf);
+	  /* I don't think this is guaranteed to be right.  For the
+	     moment, we'll pretend it is.  */
+	  end = BUF_Z (buf) - w->window_end_pos - BUF_BEGV (buf);

I can almost guarantee that it's not guaranteed that window_end_pos is
always right.  But I don't have an alternative, ATM.  Could you please
add a TODO or what's customary today in the comment, so it's easier to
find?

+	}
+      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.

@@ -19133,6 +19142,14 @@ redisplay_window (Lisp_Object window, bool just_this_one_p)
      variables.  */
   set_buffer_internal_1 (XBUFFER (w->contents));

+  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.)

@@ -27667,7 +27684,12 @@ decode_mode_spec (struct window *w, register int c, int field_width,

     case 'n':
       if (BUF_BEGV (b) > BUF_BEG (b) || BUF_ZV (b) < BUF_Z (b))
-	return " Narrow";
+	{
+	  if (! BUFFER_AUTO_NARROWED_P (b))
+	    return " Narrow";
+	  else
+	    return " Auto-Narrow";
+	}
       break;

This if/else I'd also reverse because of the !.

@@ -27675,17 +27697,27 @@ decode_mode_spec (struct window *w, register int c, int field_width,
       {
         ptrdiff_t toppos = marker_position (w->start);
         ptrdiff_t botpos = BUF_Z (b) - w->window_end_pos;
-        ptrdiff_t begv = BUF_BEGV (b);
-        ptrdiff_t zv = BUF_ZV (b);
+	ptrdiff_t beg, z;

-        if (zv <= botpos)
-          return toppos <= begv ? "All" : "Bottom";
-        else if (toppos <= begv)
+	if (! BUFFER_AUTO_NARROWED_P (b))
+	  {
+	    beg = BUF_BEGV (b);
+	    z = BUF_ZV (b);
+	  }
+	else
+	  {
+	    beg = BUF_BEG (b);
+	    z = BUF_Z (b);
+	  }

Reverse if/else?