Subject: bug#68688: 30.0.50; next-line-completion doesn't work with multiline completions

From: Juri Linkov <[email-masked]>
Date: Wed, 24 Jan 2024 18:45:08 +0200

> Patch which fixes this attached.

Thanks for fixing the multi-line case.  Could you please
add a test in minibuffer-tests.el.  Maybe easier is
to replace '("aa" "ab" "ac") with '("aa" "a\nb" "ac").
Or a new test would be better, I don't know.

> It simply adds an additional loop around forward-line which runs
> until we leave the completion at point.

Alternatively without a loop maybe simpler would be just to move
to the end of the current completion before calling forward-line?

> BTW, I think we should have some helper functions for "get start of
> completion candidate or point-min" and "get end of completion candidate
> or point-max", or possibly "move to start of candidate" and "move to end
> of candidate"; it's pretty hard to get right.

Completely agreed.  We need more helper functions for all 4 functions:
previous-completion, next-completion, previous-line-completion,
next-line-completion.

BTW, do you think that these parts below are not needed anymore
since you implemented a hidden placeholder at the top
in case of no header?

diff --git a/lisp/simple.el b/lisp/simple.el
index 692c0dacefc..ca3599e6cb3 100644
--- a/lisp/simple.el
+++ b/lisp/simple.el
@@ -9913,11 +9913,11 @@ first-completion
   "Move to the first item in the completions buffer."
   (interactive)
   (goto-char (point-min))
-  (if (get-text-property (point) 'mouse-face)
-      (unless (get-text-property (point) 'first-completion)
-        (let ((inhibit-read-only t))
-          (add-text-properties (point) (min (1+ (point)) (point-max))
-                               '(first-completion t))))
+  (if t ;; (get-text-property (point) 'mouse-face)
+      ;; (unless (get-text-property (point) 'first-completion)
+      ;;   (let ((inhibit-read-only t))
+      ;;     (add-text-properties (point) (min (1+ (point)) (point-max))
+      ;;                          '(first-completion t))))
     (when-let ((pos (next-single-property-change (point) 'mouse-face)))
       (goto-char pos))))
 
@@ -9950,13 +9950,13 @@ next-completion
   (let ((tabcommand (member (this-command-keys) '("\t" [backtab])))
         pos)
     (catch 'bound
-      (when (and (bobp)
-                 (> n 0)
-                 (get-text-property (point) 'mouse-face)
-                 (not (get-text-property (point) 'first-completion)))
-        (let ((inhibit-read-only t))
-          (add-text-properties (point) (1+ (point)) '(first-completion t)))
-        (setq n (1- n)))
+      ;; (when (and (bobp)
+      ;;            (> n 0)
+      ;;            (get-text-property (point) 'mouse-face)
+      ;;            (not (get-text-property (point) 'first-completion)))
+      ;;   (let ((inhibit-read-only t))
+      ;;     (add-text-properties (point) (1+ (point)) '(first-completion t)))
+      ;;   (setq n (1- n)))
 
       (while (> n 0)
         (setq pos (point))
@@ -10019,13 +10019,13 @@ next-line-completion
 Also see the `completion-auto-wrap' variable."
   (interactive "p")
   (let (line column pos found)
-    (when (and (bobp)
-               (> n 0)
-               (get-text-property (point) 'mouse-face)
-               (not (get-text-property (point) 'first-completion)))
-      (let ((inhibit-read-only t))
-        (add-text-properties (point) (1+ (point)) '(first-completion t)))
-      (setq n (1- n)))
+    ;; (when (and (bobp)
+    ;;            (> n 0)
+    ;;            (get-text-property (point) 'mouse-face)
+    ;;            (not (get-text-property (point) 'first-completion)))
+    ;;   (let ((inhibit-read-only t))
+    ;;     (add-text-properties (point) (1+ (point)) '(first-completion t)))
+    ;;   (setq n (1- n)))
 
     (if (get-text-property (point) 'mouse-face)
         ;; If in a completion, move to the start of it.