bug#68801: 30.0.50; minibuffer-visible-completions=t makes RET in completion-in-region a no-op with nothing selected

[Spencer Baugh]

Juri Linkov <juri@linkov.net> writes:
>> Since there are a few alternatives, perhaps we could have a defcustom
>> for the RET behavior in completion-in-region.  I think the default
>> should be closing *Completions* and inserting a newline, since that
>> matches minibuffer-visible-completions=nil.
>>
>> If this sounds reasonable I can write a patch implementing these.
>
> Sorry, I have no opinion - this is such a gray area, I never tried
> to type RET with an unselected completion candidate.
>
> But at least the current behavior makes sense since it's like
> in Isearch mode where RET just exits Isearch, whereas some other
> keys exit Isearch and do their usual job.

Fair.  So that may make sense as an optional behavior.

Still, I think the behavior of completion-in-region with
minibuffer-visible-completions=nil is more relevant, and the default
should match that.

Here's a simple patch: What if minibuffer-visible-completions just only
overrides RET when there's a selected completion?  See below.  Could
this make sense?

In general, now RET with minibuffer-visible-completions=t will do
whatever RET did when minibuffer-visible-completions=nil, except that
when there's a selected completion RET will choose it.  That seems
simple and easy to understand to me.

(Really, I think something like this behavior for RET could be made the
default, even when minibuffer-visible-completions=nil)

diff --git a/lisp/minibuffer.el b/lisp/minibuffer.el
index 45aab398078..688018fd07f 100644
--- a/lisp/minibuffer.el
+++ b/lisp/minibuffer.el
@@ -3161,7 +3161,7 @@ minibuffer-visible-completions
   :type 'boolean
   :version "30.1")
 
-(defun minibuffer-visible-completions-bind (binding)
+(defun minibuffer-visible-completions-bind (binding &optional require-selected)
   "Use BINDING when completions are visible.
 Return an item that is enabled only when a window
 displaying the *Completions* buffer exists."
@@ -3169,9 +3169,13 @@ minibuffer-visible-completions-bind
     "" ,binding
     :filter ,(lambda (cmd)
                (when-let ((window (get-buffer-window "*Completions*" 0)))
-                 (when (eq (buffer-local-value 'completion-reference-buffer
-                                               (window-buffer window))
-                           (window-buffer (active-minibuffer-window)))
+                 (when (and (eq (buffer-local-value 
'completion-reference-buffer
+                                                    (window-buffer window))
+                                (window-buffer (active-minibuffer-window)))
+                            (if require-selected
+                                (with-current-buffer (window-buffer window)
+                                  (get-text-property (point) 
'completion--string))
+                              t))
                    cmd)))))
 
 (defvar-keymap minibuffer-visible-completions-map
@@ -3180,7 +3184,7 @@ minibuffer-visible-completions-map
   "<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)
+  "RET"     (minibuffer-visible-completions-bind 
#'minibuffer-choose-completion-or-exit t)
   "C-g"     (minibuffer-visible-completions-bind 
#'minibuffer-hide-completions))
 
 ;;; Completion tables.