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

[sbaugh]

Juri Linkov <juri@linkov.net> writes:

>> 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?
>
> Then 'minibuffer-choose-completion-or-exit' could be more smart
> to run whatever command was bound to RET initially, instead of
> using the hard-coded 'minibuffer-complete-and-exit'.
> But this might require juggling with keymaps.

Indeed, tricky.  And since minibuffer-choose-completion-or-exit could be
bound to other keys, not just RET, we shouldn't hardcode it to run what
was bound to RET, too - it should run whatever it replaced.

Maybe if there's no completion selected, it could push this-command-keys
back on to unread-command-events, then disable
completion-in-region-mode?  Then Emacs would just do key lookup again
for us.

>> Alternatively... as a completely separate point, I'd like
>> completion-in-region to select the first completion candidate by
>> default.  I think that's useful in some cases and, for
>> completion-in-region, doesn't have any negatives: we couldn't do it in
>> the minibuffer because it would interfere with accepting the default,
>> but there are no defaults in completion-in-region.
>>
>> If we make c-i-r select the first completion candidate by default, that
>> would both:
>>
>> - Make the completion-show-help help render correctly with the "only
>>   override RET when there's a selected completion" patch.
>>
>> - Partially mitigate the RET issue all on its own
>
> Calling 'first-completion' makes sense even for the minibuffer,
> at least optionally.

What about this simple patch?

If minibuffer-visible-completions=nil, it just calls first-completion.
This doesn't meaningfully change anything, since previously M-RET would
just no-op in that situation.  And it's actually quite useful, because
it makes M-RET useful without having to ever actually do M-<down>; one
can just type text to narrow the completions and then hit M-RET.  Which
is sometimes nice.

If minibuffer-visible-completions=t, it calls first-completion only when
there's no minibuffer-default; that way a simple RET will still select
the minibuffer default.

diff --git a/lisp/simple.el b/lisp/simple.el
index 8246b9cab81..715ab672655 100644
--- a/lisp/simple.el
+++ b/lisp/simple.el
@@ -10355,7 +10355,10 @@ completion-setup-function
                      "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")))))))
+to move point between completions.\n\n"))))
+      (unless (and minibuffer-visible-completions minibuffer-default)
+        (with-minibuffer-completions-window
+          (first-completion))))))
 
 (add-hook 'completion-setup-hook #'completion-setup-function)