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

[Eli Zaretskii <eliz@gnu.org>]

> From: Stefan Monnier <monnier@iro.umontreal.ca>
> Cc: 43519@debbugs.gnu.org
> Date: Sat, 19 Sep 2020 18:06:20 -0400
> 
> > 	     ;;"-prospects" - more than one candidate
> > 	     (prospects-len (+ (string-width
> > 				(or determ (concat open-bracket close-bracket)))
> > 			       (string-width icomplete-separator)
> > 			       (+ 2 (string-width ellipsis)) ;; take {…} into account
> > 			       (string-width (buffer-string))))
> >              (prospects-max
> >               ;; Max total length to use, including the minibuffer content.
> >               (* (+ icomplete-prospects-height
> >                     ;; If the minibuffer content already uses up more than
> >                     ;; one line, increase the allowable space accordingly.
> >                     (/ prospects-len (window-width)))
> >                  (window-width)))
> 
> That's not relevant to the issue at hand.  I used `icomplete-mode` only
> as a vehicle to show the underlying behavior with a short recipe which
> exhibits a real-life problem.

TL;DR: Please describe those real-life problems in more detail.  I
hope my explanations below clarify why this is needed.

Details:

If you want the display engine to behave with after-strings the same
as with buffer text in this and similar cases, then this is AFAIU
impossible under the current design of the display code.  In
particular, functions that allow layout decisions by simulating
display treat overlay strings as a single unbreakable chunk of text,
and will not stop inside such strings, and so cannot provide the same
information they do when the text in the mini-window comes from a
buffer.  So the code in resize_mini_window and elsewhere cannot
possibly behave the same in the two variants of mini-window display
you provided in your test case.  Or at least I don't know how to make
it behave the same.

IOW, to the best of my knowledge and understanding, this is not a bug,
but a direct consequence of how the display code was designed.

Therefore, we are left with 2 possibilities:

  . Fix these problems on the application level.  In the case in
    point, that would mean for icomplete.el to augment its
    calculations of where to truncate the list of candidates so that
    the problem doesn't happen (and AFAICT it doesn't happen if the
    stuff to be displayed in the mini-window fits the mini-window
    after resizing it to max-mini-window-height).

  . Define in more detail what situations we would like to fix in the
    display code, so that we could install special ad-hoc changes
    there to handle those situations.  For example: is it true that in
    all of these situations starting the mini-window display at BOB
    would DTRT?  If so, I think this could be arranged.  If not, why
    not, and what is the more correct definition of the situations we
    want to handle?

> > What do you think is the underlying problem?
> 
> That the redisplay performed horizontal scrolling when it was not needed
> since the cursor was already visible without such scrolling.

There's no horizontal scrolling.  The issue is with determining the
mini-window's start position.  In the case with the overlay, we
compute that position as EOB, whereas in the case with buffer text, we
compute it to be at BOB.  The reason is what I said: the very
different behavior of the move_it_* functions when they need to
traverse overlay strings.

The basic logic of resize_mini_window is like this:

  . compute the number of screen lines required for displaying the
    mini-window
  . if the computed number of screen lines is more than
    max-mini-window-height allows, then compute where to start the
    mini-window display, as follows:
    - start at the end of the stuff to be displayed in the mini-window
    - move back max-mini-window-height screen lines
    - use the start of the screen line where we wind up as the
      mini-window's start point

IOW, the basic logic is to show the last max-mini-window-height screen
lines of what's in mini-window.

However, when the window-start so computed is then examined by the
code which actually redisplays the mini-window, that code can override
the computed window-start if the position of point will not be visible
in the mini-window with that window-start in effect.  This actually
happens when the test code uses buffer text (not an overlay string) --
the computed window-start, which is in the middle of the "askdjf..."
text, is abandoned, and BOB is used instead.  This does NOT happen
with the overlay-string version, because the window-start point
computed by resize_mini_window is EOB, and that position is visible in
the window.

> >> (and as you know it's wickedly difficult to construct a string which
> >> will have "just the right size" to fit into the minibuffer window).
> > It doesn't have to be "just the right size", it could err on the safe
> > side.  It already attempts to do so, by avoiding truncation in the
> > middle of a candidate.  It should just do a better job, that's all.
> 
> And how do we generalize that to the case where the overlay contains
> newlines, TABs, chars in different scripts using different fonts,
> different faces, images, etc.... ?

Doesn't window-text-pixel-size provide a tool to solve at least some
of those problems?

>     (minibuffer-with-setup-hook
>         (lambda ()
>           (insert "hello")
>           (let ((ol (make-overlay (point) (point)))
>                 (max-mini-window-height 1)
>                 (text "askdjfhaklsjdfhlkasjdfhklasdhflkasdhflkajsdhflkashdfkljahsdlfkjahsdlfkjhasldkfhalskdjfhalskdfhlaksdhfklasdhflkasdhflkasdhflkajsdhklajsdgh"))
>             (save-excursion (insert text))
>             (sit-for 2)
>             (delete-region (point) (point-max))
>             (put-text-property 0 1 'cursor t text)
>             (overlay-put ol 'after-string text)
>             (sit-for 2)
>             (delete-overlay ol)))
>       (read-string "toto: "))

(Btw, people who read this should be aware that binding
max-mini-window-height like this doesn't work in general: the setting
must be in effect when redisplay runs.  It works here only because
there are sit-for calls.)

I believe I explained the issues above; if not, please ask specific
questions.

> So the question is: how to get the same behavior as what we'd get with
> `insert` but without actually modifying the buffer's contents?

You can't, not without redesigning the display code.  At least not in
the general way you describe the issue, and not to the best of my
knowledge.

Without such a redesign we can only make ad-hoc changes for specific
situations.  If such ad-hoc changes are to be done in the display
engine, I need a better, more detailed (and more friendly) spec.