>> +(defcustom minibuffer-completion-visible t
>> +  "Non-nil means to navigate completions with arrows from the minibuffer.
>> +This has effect only when the window with the *Completions* buffer
>> +is visible on the screen."
>
> This doc string needs to be improved, as it currently doesn't explain
> its effect in enough detail, IMO.  Maybe because "non-nil means to
> navigate" is awkward/confusing English.

Now improved in a new patch.

> Regarding the last sentence of the doc string: does it mean that if
> the *Completions* buffer is not in any window, the arrow keys in the
> minibuffer will not move point in that buffer?

If the *Completions* buffer is not in any window, the arrow keys
move point in the minibuffer.  Now explained this in the docstring.

>> +(defun minibuffer-bind-visible (binding)
>> +  `(menu-item
>> +    "" ,binding
>> +    :filter ,(lambda (cmd)
>> +               (when (get-buffer-window "*Completions*" 0)
>> +                 cmd))))
>
> This function needs a doc string and possibly a better name, to match
> what it actually does.

Done.

> The argument zero to get-buffer-window AFAIU means that it will return
> non-nil when the buffer is shown in some window on an iconified frame,
> and I wonder why we would consider such a buffer "visible".

(get-buffer-window "*Completions*" 0) is used everywhere in minibuffer.el
and simple.el, so the argument zero is for compatibility with other
minibuffer completion commands.

>> +  "RET"     (minibuffer-bind-visible #'minibuffer-choose-completion)
>
> AFAICT, this important binding is not mentioned or hinted in the doc
> string of the new option.

Now mentioned.

>> -(defun minibuffer-next-completion (&optional n)
>> +(defun minibuffer-next-completion (&optional n vertical)
>>    "Move to the next item in its completions window from the minibuffer.
>> +When the optional argument VERTICAL is non-nil, move vertically.
>
> The "move vertically" part contradicts the "to the next item" part,
> doesn't it?

The next item can be on the axis x or y.

> Thus, I think the added sentence should be more detailed,
> and should explicitly say that the result will be a move to the next
> line, not the next item (and perhaps also include a link to
> next-line-completion).

Ok, improved with a link.

>> +(defun minibuffer-next-line-completion (&optional n)
>> +  "Move to the next completion line from the minibuffer.
>
> This sentence is misleading, IMO.  Assuming I understood what you
> mean, I would rephrase:
>
>   Move to the completion candidate on the next line in the completions buffer.

This line is too long and doesn't mention the minibuffer.
So I added it to the rest of the docstring.

>> +When `minibuffer-completion-auto-choose' is non-nil, then also
>> +insert the selected completion to the minibuffer."
>
> Please make a habit of talking about "completion candidate" in these
> cases, not about "completion".  The latter is ambiguous, since it can
> refer both to a candidate and to the action of performing completion.

Ok, fixed.

>> +(defun minibuffer-previous-line-completion (&optional n)
>> +  "Move to the previous completion line from the minibuffer.
>> +When `minibuffer-completion-auto-choose' is non-nil, then also
>> +insert the selected completion to the minibuffer."
>
> Same here.

Here is a new patch: