bug#44070: 28.0.50; Minibuffer display "jumps" upon minor edit

[Stefan Monnier]

Package: Emacs
Version: 28.0.50


Step 1:

    % emacs -Q
    M-: a C-u 20 C-q C-j abc

You should now have "abc" on the last line of the miniwindow with the
prompt scrolled out of view.  So far so good.

Step 2:

    M-< M->

We scroll to the beginning and back to the end.  Now "abc" is not quite
on the last line of the miniwindow, more specifically there should be 2 empty
lines left after "abc".  The behavior is slightly different if instead of
`end-of-buffer` we use (goto-char (point-max)), but the problem at step
3 remains:

Step 3:

    DEL

This is the surprising part: this simple edit causes the minibuffer to
be "rescrolled" so that the resulting "ab" is now again placed on the
very last line of the miniwindow.

The same thing happens with any other simple edit because any buffer
modification triggers a call to `resize_mini_window`, which will always
try to arrange to see a maximum of text.

Not sure if it's better to fix this by changing step 2 or by changing
step 3, but the patch below fixes it by changing step 2.

WDYT?


        Stefan


diff --git a/lisp/simple.el b/lisp/simple.el
index d6fce922c4..41aba2ddc3 100644
--- a/lisp/simple.el
+++ b/lisp/simple.el
@@ -1129,7 +1129,7 @@ end-of-buffer
         ;; If the end of the buffer is not already on the screen,
         ;; then scroll specially to put it near, but not at, the bottom.
         (overlay-recenter (point))
-        (recenter -3))))
+        (recenter (if (window-minibuffer-p) -1 -3)))))
 
 (defcustom delete-active-region t
   "Whether single-char deletion commands delete an active region.
diff --git a/src/xdisp.c b/src/xdisp.c
index 5a62cd6eb5..c1105df3fc 100644
--- a/src/xdisp.c
+++ b/src/xdisp.c
@@ -18820,6 +18820,7 @@ redisplay_window (Lisp_Object window, bool 
just_this_one_p)
 
   /* Try to scroll by specified few lines.  */
   if ((0 < scroll_conservatively
+       || MINI_WINDOW_P (w)
        || 0 < emacs_scroll_step
        || temp_scroll_step
        || NUMBERP (BVAR (current_buffer, scroll_up_aggressively))
@@ -18830,7 +18831,9 @@ redisplay_window (Lisp_Object window, bool 
just_this_one_p)
       /* The function returns -1 if new fonts were loaded, 1 if
         successful, 0 if not successful.  */
       int ss = try_scrolling (window, just_this_one_p,
-                             scroll_conservatively,
+                             (MINI_WINDOW_P (w)
+                              ? SCROLL_LIMIT + 1
+                              : scroll_conservatively),
                              emacs_scroll_step,
                              temp_scroll_step, last_line_misfit);
       switch (ss)
diff --git a/test/src/xdisp-tests.el b/test/src/xdisp-tests.el
index 95c39dacc3..d513f5beba 100644
--- a/test/src/xdisp-tests.el
+++ b/test/src/xdisp-tests.el
@@ -21,34 +21,55 @@
 
 (require 'ert)
 
+(defmacro xdisp-tests--in-minibuffer (&rest body)
+  (declare (debug t) (indent 0))
+  `(catch 'result
+     (minibuffer-with-setup-hook
+         (lambda ()
+           (let ((redisplay-skip-initial-frame nil)
+                 (executing-kbd-macro nil)) ;Don't skip redisplay
+             (throw 'result (progn . ,body))))
+       (let ((executing-kbd-macro t)) ;Force real minibuffer in `read-string'.
+         (read-string "toto: ")))))
+
 (ert-deftest xdisp-tests--minibuffer-resizing () ;; bug#43519
-  ;; FIXME: This test returns success when run in batch but
-  ;; it's only a lucky accident: it also returned success
-  ;; when bug#43519 was not fixed.
   (should
    (equal
     t
-    (catch 'result
-      (minibuffer-with-setup-hook
-          (lambda ()
-            (insert "hello")
-            (let ((ol (make-overlay (point) (point)))
-                  (redisplay-skip-initial-frame nil)
-                  (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)
-              (let ((executing-kbd-macro nil)) ;Don't skip redisplay
-                (redisplay 'force))
-              (throw 'result
-                     ;; Make sure we do the see "hello" text.
-                     (prog1 (equal (window-start) (point-min))
-                       ;; (list (window-start) (window-end) (window-width))
-                       (delete-overlay ol)))))
-        (let ((executing-kbd-macro t)) ;Force real minibuffer in `read-string'.
-          (read-string "toto: ")))))))
+    (xdisp-tests--in-minibuffer
+      (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)
+        (redisplay 'force)
+        ;; Make sure we do the see "hello" text.
+        (prog1 (equal (window-start) (point-min))
+          ;; (list (window-start) (window-end) (window-width))
+          (delete-overlay ol)))))))
+
+(ert-deftest xdisp-tests--minibuffer-scroll () ;; bug#43519
+  (let ((posns
+         (xdisp-tests--in-minibuffer
+           (let ((max-mini-window-height 4))
+             (dotimes (_ 80) (insert "\nhello"))
+             (beginning-of-buffer)
+             (redisplay 'force)
+             (end-of-buffer)
+             ;; A simple edit like removing the last `o' shouldn't cause
+             ;; the rest of the minibuffer's text to move.
+             (list
+              (progn (redisplay 'force) (window-start))
+              (progn (delete-char -1)
+                     (redisplay 'force) (window-start))
+              (progn (goto-char (point-min)) (redisplay 'force)
+                     (goto-char (point-max)) (redisplay 'force)
+                     (window-start)))))))
+    (should (equal (nth 0 posns) (nth 1 posns)))
+    (should (equal (nth 1 posns) (nth 2 posns)))))
 
 ;;; xdisp-tests.el ends here