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

From: Spencer Baugh <[email-masked]>
Date: Mon, 26 Feb 2024 11:04:59 -0500

[email-masked] writes:
> Juri Linkov <[email-masked]> writes:
>>>> 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?
>>>
>>> Yes, this makes sense.  I'll try your patch for a while.
>>
>> Thanks for the patch, it works nicely.
>>
>> Could you please write a commit message for this patch.
>
> The annoying thing is that this breaks the enhancement to
> completion-show-help I recently made.  When a completion is not
> selected, RET isn't bound, so the help shows "Click or type M-x
> minibuffer-choose-completion-or-exit on a completion to select it."
>
> I'm not sure the right way to fix that.  Selectively binding RET based
> on whether a completion is selected feels sketchy anyway, it may confuse
> users because e.g. C-h c RET won't work.
>
> Maybe instead we should always bind RET, but if no completion is
> selected, we run the command that RET was bound to before
> completion-in-region-mode started?
>
> Alternatively... as a completely separate point, I'd like
> completion-in-region to select the first completion candidate by
> default.

Okay, after running with my patch doing this for a while, I don't think
it's a good idea.  It makes cir too different from minibuffer
completion, and it just generally makes things a bit more unpredictable.
Maybe we can think of a way to do it later.

So anyway, this brings me back to wanting RET to insert a newline.  I
tried changing minibuffer-choose-completion-or-exit like so:

diff --git a/lisp/minibuffer.el b/lisp/minibuffer.el
index 2a8439e03dd..cbf340f1dd6 100644
--- a/lisp/minibuffer.el
+++ b/lisp/minibuffer.el
@@ -4664,14 +4664,18 @@ minibuffer-choose-completion
 
 (defun minibuffer-choose-completion-or-exit (&optional no-exit no-quit)
   "Choose the completion from the minibuffer or exit the minibuffer.
-When `minibuffer-choose-completion' can't find a completion candidate
-in the completions window, then exit the minibuffer using its present
-contents."
+When `minibuffer-choose-completion' can't find a completion
+candidate in the completions window, then hide the completions
+window (which disables `minibuffer-visible-completions') and run
+the command which this key was previously bound to, usually
+exiting the minibuffer using its present contents."
   (interactive "P")
   (condition-case nil
       (let ((choose-completion-deselect-if-after t))
         (minibuffer-choose-completion no-exit no-quit))
-    (error (minibuffer-complete-and-exit))))
+    (error (minibuffer-hide-completions)
+           (mapc (lambda (key) (push key unread-command-events))
+                 (this-command-keys-vector)))))
 
 (defun minibuffer-complete-history ()
   "Complete the minibuffer history as far as possible.

but it doesn't work in a few situations.  Maybe there's a better way to
do it though?

I'm thinking the right thing to do is just to go with my original patch
which only binds RET when there's a selected completion.  That's simple
and straightforward.

But I'm not sure how to adapt my completion-show-help change for that: I
want to hint to the user that they should hit RET, but RET is only bound
when a completion is selected, which isn't the case when *Completions*
is first displayed.  Any ideas?  Could we just hardcode "RET" in the
help text?