Subject: bug#43519: 28.0.50; Overlay at end of minibuf hides minibuf's real content

From: Stefan Monnier <[email-masked]>
Date: Mon, 21 Sep 2020 13:25:01 -0400

>> Do you know why we don't do it this way, IOW why don't we first try to
>> keep window-start unchanged and see if point ends up within view?
> Because this way we have no control on where the mini-window's display
> will start, and consequently what will be visible in the mini-window.

Indeed, then we'll just rely on the "generic" behavior, which is
admittedly not focused on single-line (or few lines) windows, but at
least in the current case it works better (and simplifies the code
slightly).

> In particular, if point is at EOB, redisplay could (and normally does)
> decide not to position point on the last screen line of the window,
> which means we may have some of the text not visible for no good
> reason -- not a good thing when user interaction is concerned.

Not sure I understand what you mean.
If point is at EOB, redisplay will make sure EOB is visible.

Or do you mean a situation like:
- minibuffer holds "foo\n"
- the mini window is a single line
- point is at EOB
In which case we'd end up displaying the empty line instead of display "foo"?

AFAICT our ad-hoc scrolling code gives the same result as the generic
scrolling code in that case.

I've been trying out the patch below and haven't bumped into any
surprising behavior yet, but admittedly, I probably lack creativity.

> More generally, since the mini-window is usually small and user
> interaction requires to make sure the user sees the important parts of
> the buffer text (if not all of it), we want tighter control on what
> will end up on display, and the way to do that is via setting
> window-start.

That makes sense in general, but I'm curious to see what are the
concrete cases where our "generic" scrolling logic leads to worse
behavior than the ad-hoc one used here.

So far the patch below fixed the original problem and I haven't been
able to see any regression yet.

>>     (setq max-mini-window-height 1)
>> with
>>     (setq resize-mini-windows nil)
>> the problem doesn't appear, even though resizing is in fact disabled in
>> both cases.
> Disabling resize-mini-windows means the mini-window is not resized at
> all as part of redisplay.

Yes, of course, I was just using it to illustrate what happens when we
don't use the ad-hoc scrolling code from resize_mini_window in the case
where there is in practice no resizing anyway.

> And besides, this is a user option, so having some code disregard it
> is unfriendly, to say the least, even if we do that temporarily.

Oh yes,

        Stefan

diff --git a/src/xdisp.c b/src/xdisp.c
index dfcb1d73e4..b25aa07f1f 100644
--- a/src/xdisp.c
+++ b/src/xdisp.c
@@ -11885,14 +11885,18 @@ resize_mini_window (struct window *w, bool exact_p)
       if (height > max_height)
 	{
 	  height = (max_height / unit) * unit;
-	  init_iterator (&it, w, ZV, ZV_BYTE, NULL, DEFAULT_FACE_ID);
-	  move_it_vertically_backward (&it, height - unit);
-	  start = it.current.pos;
+	  /* bug#43519: Let the redisplay choose the window start!
+           *
+           * init_iterator (&it, w, ZV, ZV_BYTE, NULL, DEFAULT_FACE_ID);
+	   * move_it_vertically_backward (&it, height - unit);
+	   * start = it.current.pos; */
 	}
       else
-	SET_TEXT_POS (start, BEGV, BEGV_BYTE);
+	{
+	  SET_TEXT_POS (start, BEGV, BEGV_BYTE);
 
-      SET_MARKER_FROM_TEXT_POS (w->start, start);
+          SET_MARKER_FROM_TEXT_POS (w->start, start);
+        }
 
       if (EQ (Vresize_mini_windows, Qgrow_only))
 	{