Make RET choose the selected completion

Previously, one could select a completion via M-<up>/M-<down>,
but then RET would not actually select the chosen completion.
With the addition of completion-auto-deselect, this is not
actually necessary: we can reasonably assume that when a
completion is selected, the user wants to use that, since their
last action must have been to select it.  So, just choose the
selected completion on RET.  This lets us default
minibuffer-completion-auto-choose to nil.

For minibuffers with require-match completion, this can be done
by changing the existing command bound to RET.  For minibuffers
with nil require-match completion, RET was previously bound to
exit-minibuffer, and changing exit-minibuffer to have this logic
is risky.  We handle that case by adding a new
minibuffer-completion-exit which wraps exit-minibuffer and bind
RET to it.

* lisp/minibuffer.el (minibuffer-insert-completion-if-selected)
(minibuffer-completion-exit, completion--selected-candidate):
Add.
(minibuffer-complete-and-exit): Call
minibuffer-choose-completion. (bug#77253)
(minibuffer-local-completion-map): Bind RET to
minibuffer-completion-exit, overriding exit-minibuffer.
(completion-in-region-mode-map): Bind RET to
minibuffer-choose-completion when there's a selected candidate.
(minibuffer-completion-auto-choose): Default to nil.
(minibuffer-visible-completions--filter)
(minibuffer-visible-completions-map): Delete RET binding, no
longer necessary.
* lisp/simple.el (completion-setup-function): Update completion
help text to show more correct bindings.
* test/lisp/minibuffer-tests.el (completions-header-format-test)
(minibuffer-next-completion): Set
minibuffer-completion-auto-choose=t explicitly.
(with-minibuffer-setup, minibuffer-completion-RET-prefix)
(completion-in-region-next-completion): Add new tests.
* etc/NEWS: Announce.

etc/NEWS

@@ -183,6 +183,14 @@ different completion categories by customizing
 be updated as you type, or nil to suppress this always.  Note that for
 large or inefficient completion tables this can slow down typing.
 
+---
+*** RET chooses the completion selected with M-<up>/M-<down>
+If a completion candidate is selected with M-<up> or M-<down>, hitting
+RET will exit completion with that as the result.  This works both in
+minibuffer completion and in-buffer completion.  This supersedes
+'minibuffer-completion-auto-choose', which previously provided similar
+behavior; that variable is now nil by default.
+
 +++
 *** New user option 'completion-pcm-leading-wildcard'.
 This option configures how the partial-completion style does completion.

lisp/minibuffer.el

@@ -1983,12 +1983,17 @@ DONT-CYCLE tells the function not to setup cycling."
 (defvar minibuffer--original-buffer nil
   "Buffer that was current when `completing-read' was called.")
 
-(defun minibuffer-complete-and-exit ()
+(defun minibuffer-complete-and-exit (&optional no-exit)
   "Exit if the minibuffer contains a valid completion.
 Otherwise, try to complete the minibuffer contents.  If
 completion leads to a valid completion, a repetition of this
 command will exit.
 
+If a completion candidate is selected in the *Completions* buffer, it
+will be inserted in the minibuffer first.  If NO-EXIT is non-nil, don't
+actually exit the minibuffer, just insert the selected completion if
+any.
+
 If `minibuffer-completion-confirm' is `confirm', do not try to
  complete; instead, ask for confirmation and accept any input if
  confirmed.
@@ -1997,9 +2002,12 @@ If `minibuffer-completion-confirm' is `confirm-after-completion',
  preceding minibuffer command was a member of
  `minibuffer-confirm-exit-commands', and accept the input
  otherwise."
-  (interactive)
-  (completion-complete-and-exit (minibuffer--completion-prompt-end) (point-max)
-                                #'exit-minibuffer))
+  (interactive "P")
+  (when (completion--selected-candidate)
+    (minibuffer-choose-completion t t))
+  (unless no-exit
+    (completion-complete-and-exit (minibuffer--completion-prompt-end) (point-max)
+                                  #'exit-minibuffer)))
 
 (defun completion-complete-and-exit (beg end exit-function)
   (completion--complete-and-exit
@@ -3010,6 +3018,11 @@ Also respects the obsolete wrapper hook `completion-in-region-functions'.
   ;; completion-at-point called directly.
   "M-?" #'completion-help-at-point
   "TAB" #'completion-at-point
+  ;; If a completion is selected, RET will choose it.
+  "RET" `(menu-item "" minibuffer-choose-completion :filter
+                    ,(lambda (cmd)
+                       (when (completion--selected-candidate)
+                         cmd)))
   "M-<up>"   #'minibuffer-previous-completion
   "M-<down>" #'minibuffer-next-completion
   "M-RET"    #'minibuffer-choose-completion)
@@ -3216,6 +3229,17 @@ The completion method is determined by `completion-at-point-functions'."
   (define-key map "\n" 'exit-minibuffer)
   (define-key map "\r" 'exit-minibuffer))
 
+(defun minibuffer-completion-exit (&optional no-exit)
+  "Call `exit-minibuffer', inserting the selected completion first if any.
+
+If NO-EXIT is non-nil, don't `exit-minibuffer', just insert the selected
+completion."
+  (interactive "P")
+  (when (completion--selected-candidate)
+    (minibuffer-choose-completion t t))
+  (unless no-exit
+    (exit-minibuffer)))
+
 (defvar-keymap minibuffer-local-completion-map
   :doc "Local keymap for minibuffer input with completion."
   :parent minibuffer-local-map
@@ -3225,6 +3249,7 @@ The completion method is determined by `completion-at-point-functions'."
   ;; another binding for it.
   ;; "M-TAB"  #'minibuffer-force-complete
   "SPC"       #'minibuffer-complete-word
+  "RET"       #'minibuffer-completion-exit
   "?"         #'minibuffer-completion-help
   "<prior>"   #'switch-to-completions
   "M-v"       #'switch-to-completions
@@ -3344,16 +3369,18 @@ and `RET' accepts the input typed into the minibuffer."
               (window-buffer (active-minibuffer-window)))
       window)))
 
+(defun completion--selected-candidate ()
+  "Return the selected completion candidate if any."
+  (when-let* ((window (minibuffer--completions-visible)))
+    (with-current-buffer (window-buffer window)
+      (get-text-property (point) 'completion--string))))
+
 (defun minibuffer-visible-completions--filter (cmd)
   "Return CMD if `minibuffer-visible-completions' bindings should be active."
   (if minibuffer-visible-completions--always-bind
       cmd
     (when-let* ((window (minibuffer--completions-visible)))
-      (when (if (eq cmd #'minibuffer-choose-completion-or-exit)
-                (with-current-buffer (window-buffer window)
-                  (get-text-property (point) 'completion--string))
-              t)
-        cmd))))
+      cmd)))
 
 (defun minibuffer-visible-completions--bind (binding)
   "Use BINDING when completions are visible.
@@ -3369,7 +3396,6 @@ displaying the *Completions* buffer exists."
   "<right>" (minibuffer-visible-completions--bind #'minibuffer-next-completion)
   "<up>"    (minibuffer-visible-completions--bind #'minibuffer-previous-line-completion)
   "<down>"  (minibuffer-visible-completions--bind #'minibuffer-next-line-completion)
-  "RET"     (minibuffer-visible-completions--bind #'minibuffer-choose-completion-or-exit)
   "C-g"     (minibuffer-visible-completions--bind #'minibuffer-hide-completions))
 
 ;;; Completion tables.
@@ -5125,13 +5151,13 @@ and execute the forms."
          (completion--lazy-insert-strings)
          ,@body))))
 
-(defcustom minibuffer-completion-auto-choose t
+(defcustom minibuffer-completion-auto-choose nil
   "Non-nil means to automatically insert completions to the minibuffer.
 When non-nil, then `minibuffer-next-completion' and
 `minibuffer-previous-completion' will insert the completion
 selected by these commands to the minibuffer."
   :type 'boolean
-  :version "29.1")
+  :version "31.1")
 
 (defun minibuffer-next-completion (&optional n vertical)
   "Move to the next item in its completions window from the minibuffer.

lisp/simple.el

@@ -10570,28 +10570,24 @@ Called from `temp-buffer-show-hook'."
       ;; Maybe insert help string.
       (when completion-show-help
 	(goto-char (point-min))
-        (if minibuffer-visible-completions
-            (let ((helps
-                   (with-current-buffer (window-buffer (active-minibuffer-window))
-                     (let ((minibuffer-visible-completions--always-bind t))
-                       (list
-                        (substitute-command-keys
-	                 (if (display-mouse-p)
-	                     "Click or type \\[minibuffer-choose-completion-or-exit] on a completion to select it.\n"
-                           "Type \\[minibuffer-choose-completion-or-exit] on a completion to select it.\n"))
+        (let ((helps
+               (with-current-buffer (window-buffer (active-minibuffer-window))
+                 (let ((minibuffer-visible-completions--always-bind t))
+                   (list
+                    (substitute-command-keys
+	             (if (display-mouse-p)
+	                 "Click or type \\[minibuffer-choose-completion] on a completion to select it.\n"
+                       "Type \\[minibuffer-choose-completion] on a completion to select it.\n"))
+                    (if minibuffer-visible-completions
                         (substitute-command-keys
 		         "Type \\[minibuffer-next-completion], \\[minibuffer-previous-completion], \
 \\[minibuffer-next-line-completion], \\[minibuffer-previous-line-completion] \
-to move point between completions.\n\n"))))))
-              (dolist (help helps)
-                (insert help)))
-          (insert (substitute-command-keys
-	           (if (display-mouse-p)
-	               "Click or type \\[minibuffer-choose-completion] on a completion to select it.\n"
-                     "Type \\[minibuffer-choose-completion] on a completion to select it.\n")))
-          (insert (substitute-command-keys
-		   "Type \\[minibuffer-next-completion] or \\[minibuffer-previous-completion] \
+to move point between completions.\n\n")
+                      (substitute-command-keys
+		       "Type \\[minibuffer-next-completion] or \\[minibuffer-previous-completion] \
 to move point between completions.\n\n")))))))
+          (dolist (help helps)
+            (insert help)))))))
 
 (add-hook 'completion-setup-hook #'completion-setup-function)
 

test/lisp/minibuffer-tests.el

@@ -433,6 +433,17 @@
            15)))
 
 
+(defmacro with-minibuffer-setup (completing-read &rest body)
+  (declare (indent 1) (debug (collection body)))
+  `(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 the real minibuffer
+         ,completing-read))))
+
 (defmacro completing-read-with-minibuffer-setup (collection &rest body)
   (declare (indent 1) (debug (collection body)))
   `(catch 'result
@@ -569,6 +580,7 @@
 
 (ert-deftest completions-header-format-test ()
   (let ((completion-show-help nil)
+        (minibuffer-completion-auto-choose t)
         (completions-header-format nil))
     (completing-read-with-minibuffer-setup
         '("aa" "ab" "ac")
@@ -718,11 +730,50 @@
       (should (equal (minibuffer-contents) "ccc")))))
 
 (ert-deftest minibuffer-next-completion ()
-  (let ((default-directory (ert-resource-directory)))
+  (let ((default-directory (ert-resource-directory))
+        (minibuffer-completion-auto-choose t))
     (completing-read-with-minibuffer-setup #'read-file-name-internal
       (insert "d/")
       (execute-kbd-macro (kbd "M-<down> M-<down> M-<down>"))
       (should (equal "data/minibuffer-test-cttq$$tion" (minibuffer-contents))))))
 
+(ert-deftest minibuffer-completion-RET-prefix ()
+  ;; REQUIRE-MATCH=nil
+  (with-minibuffer-setup
+      (completing-read ":" '("aaa" "bbb" "ccc") nil nil)
+    (execute-kbd-macro (kbd "M-<down> M-<down> C-u RET"))
+    (should (equal "bbb" (minibuffer-contents))))
+  ;; REQUIRE-MATCH=t
+  (with-minibuffer-setup
+   (completing-read ":" '("aaa" "bbb" "ccc") nil t)
+   (execute-kbd-macro (kbd "M-<down> M-<down> C-u RET"))
+   (should (equal "bbb" (minibuffer-contents)))))
+
+(defun test/completion-at-point ()
+  (list (point-min) (point) '("test:a" "test:b")))
+
+(ert-deftest completion-in-region-next-completion ()
+  (with-current-buffer (get-buffer-create "*test*")
+    ;; Put this buffer in the selected window so
+    ;; `minibuffer--completions-visible' works.
+    (pop-to-buffer (current-buffer))
+    (setq-local completion-at-point-functions (list #'test/completion-at-point))
+    (insert "test:")
+    (completion-help-at-point)
+    (should (minibuffer--completions-visible))
+    ;; C-u RET and RET have basically the same behavior for
+    ;; completion-in-region-mode, since they both dismiss *Completions*
+    ;; while leaving completion-in-region-mode still active.
+    (execute-kbd-macro (kbd "M-<down>"))
+    (should (equal (completion--selected-candidate) "test:a"))
+    (execute-kbd-macro (kbd "C-u RET"))
+    (should (equal (buffer-string) "test:a"))
+    (delete-char -1)
+    (completion-help-at-point)
+    (execute-kbd-macro (kbd "M-<down> M-<down>"))
+    (should (equal (completion--selected-candidate) "test:b"))
+    (execute-kbd-macro (kbd "RET"))
+    (should (equal (buffer-string) "test:b"))))
+
 (provide 'minibuffer-tests)
 ;;; minibuffer-tests.el ends here