bug#74617: 30.0.92; ffap-menu always displays the *Completions* buffer

bug#74617: 30.0.92; ffap-menu always displays the *Completions* buffer

[Daniel Mendler via Bug reports for GNU Emacs, the Swiss army knife of text editors]

`ffap-menu' automatically displays the *Completions* buffer by calling
`minibuffer-completion-help'. If an alternative minibuffer completion
system like Icomplete or Vertico is used, the *Completions* buffer is
not needed since the candidates are already displayed in the minibuffer.

I propose to either detect these alternative completion systems (e.g.,
by checking the value of the completing-read-function and/or the mode
variables) or to provide a way to disable the call to
`minibuffer-completion-help'.

Since the same problem is present in tmm.el, maybe a generic solution
could be provided by minibuffer.el? Option 1: A function
`minibuffer-completion-help-if-needed' could call
`minibuffer-completion-help' only if no other completion system is
detected. Option 2: A new function
`completing-read-display-help-function' could be added which defaults to
`minibuffer-completion-help' and which could be set to nil/ignore by
alternative completion UIs like Vertico. This function could be used by
tmm/ffap. Option 3: A new variable `minibuffer-inhibit-completion-help'
could be added which is checked by `minibuffer-completion-help' and
which could be set to t by alternative completion UIs.

I am happy to provide a patch for any of these approaches.

bug#74617: 30.0.92; ffap-menu always displays the *Completions* buffer

[Eli Zaretskii]

> Cc: Stefan Monnier <monnier@iro.umontreal.ca>
> Date: Sat, 30 Nov 2024 08:02:20 +0100
> From:  Daniel Mendler via "Bug reports for GNU Emacs,
>  the Swiss army knife of text editors" <bug-gnu-emacs@gnu.org>
> 
> `ffap-menu' automatically displays the *Completions* buffer by calling
> `minibuffer-completion-help'. If an alternative minibuffer completion
> system like Icomplete or Vertico is used, the *Completions* buffer is
> not needed since the candidates are already displayed in the minibuffer.

That's not what I see here.  Recipe:

  emacs -Q
  M-x icomplete-mode RET
  C-x C-f nt/INSTALL.W64 RET
  M-x ffap-menu RET

I see only the *Completions* buffer, no other display of the
candidates.  What did I miss?

> I propose to either detect these alternative completion systems (e.g.,
> by checking the value of the completing-read-function and/or the mode
> variables) or to provide a way to disable the call to
> `minibuffer-completion-help'.
> 
> Since the same problem is present in tmm.el, maybe a generic solution
> could be provided by minibuffer.el? Option 1: A function
> `minibuffer-completion-help-if-needed' could call
> `minibuffer-completion-help' only if no other completion system is
> detected. Option 2: A new function
> `completing-read-display-help-function' could be added which defaults to
> `minibuffer-completion-help' and which could be set to nil/ignore by
> alternative completion UIs like Vertico. This function could be used by
> tmm/ffap. Option 3: A new variable `minibuffer-inhibit-completion-help'
> could be added which is checked by `minibuffer-completion-help' and
> which could be set to t by alternative completion UIs.

Option 2 is adding complexity to an already devilishly complex code,
so I like it the least.

But first I think we need to understand the problem better; see above.

bug#74617: 30.0.92; ffap-menu always displays the *Completions* buffer

[Daniel Mendler via Bug reports for GNU Emacs, the Swiss army knife of text editors]

Eli Zaretskii <eliz@gnu.org> writes:

>> Cc: Stefan Monnier <monnier@iro.umontreal.ca>
>> Date: Sat, 30 Nov 2024 08:02:20 +0100
>> From:  Daniel Mendler via "Bug reports for GNU Emacs,
>>  the Swiss army knife of text editors" <bug-gnu-emacs@gnu.org>
>> 
>> `ffap-menu' automatically displays the *Completions* buffer by calling
>> `minibuffer-completion-help'. If an alternative minibuffer completion
>> system like Icomplete or Vertico is used, the *Completions* buffer is
>> not needed since the candidates are already displayed in the minibuffer.
>
> That's not what I see here.  Recipe:
>
>   emacs -Q
>   M-x icomplete-mode RET
>   C-x C-f nt/INSTALL.W64 RET
>   M-x ffap-menu RET
>
> I see only the *Completions* buffer, no other display of the
> candidates.  What did I miss?

Oh, right. I missed that. I think the problem here is that Icomplete
uses a delay (`icomplete-compute-delay'). Other completion UIs don't
have such a delay and show the candidates immediately. This makes the
problem a little more difficult, in particular with respect to auto
detection.

Another alternative to auto detection could be that the completion table
communicates to `completing-read' via metadata that immediate candidate
display is desired. The completion UI could then act accordingly.
Default completion would call `minibuffer-completion-help' and Icomplete
could update immediately, ignoring `icomplete-compute-delay'.

Daniel

bug#74617: 30.0.92; ffap-menu always displays the *Completions* buffer

[Eli Zaretskii]

> From: Daniel Mendler <mail@daniel-mendler.de>
> Cc: 74617@debbugs.gnu.org,  monnier@iro.umontreal.ca
> Date: Sat, 30 Nov 2024 09:34:11 +0100
> 
> Eli Zaretskii <eliz@gnu.org> writes:
> 
> >   emacs -Q
> >   M-x icomplete-mode RET
> >   C-x C-f nt/INSTALL.W64 RET
> >   M-x ffap-menu RET
> >
> > I see only the *Completions* buffer, no other display of the
> > candidates.  What did I miss?
> 
> Oh, right. I missed that. I think the problem here is that Icomplete
> uses a delay (`icomplete-compute-delay'). Other completion UIs don't
> have such a delay and show the candidates immediately. This makes the
> problem a little more difficult, in particular with respect to auto
> detection.

Alternatively, we could consider the cases where more than one
completion list is shown a bug in the mode which shows the completions
even though the application already did.  IOW, instead of considering
this a problem of the command the user invokes, consider this a bug in
the non-default completion UI currently in effect.  It is basically a
flaw in the design of those completion UIs.

> Another alternative to auto detection could be that the completion table
> communicates to `completing-read' via metadata that immediate candidate
> display is desired. The completion UI could then act accordingly.
> Default completion would call `minibuffer-completion-help' and Icomplete
> could update immediately, ignoring `icomplete-compute-delay'.

This sounds too indirect to me, it could cause unintended adverse
consequences, especially in nested scenarios.

There's a simpler alternative: we could say we don't care, as long as
only a few alternative UIs have this issue.  It isn't a catastrophe.

bug#74617: 30.0.92; ffap-menu always displays the *Completions* buffer

[Daniel Mendler via Bug reports for GNU Emacs, the Swiss army knife of text editors]

Eli Zaretskii <eliz@gnu.org> writes:

>> From: Daniel Mendler <mail@daniel-mendler.de>
>> Cc: 74617@debbugs.gnu.org,  monnier@iro.umontreal.ca
>> Date: Sat, 30 Nov 2024 09:34:11 +0100
>> 
>> Eli Zaretskii <eliz@gnu.org> writes:
>> 
>> >   emacs -Q
>> >   M-x icomplete-mode RET
>> >   C-x C-f nt/INSTALL.W64 RET
>> >   M-x ffap-menu RET
>> >
>> > I see only the *Completions* buffer, no other display of the
>> > candidates.  What did I miss?
>> 
>> Oh, right. I missed that. I think the problem here is that Icomplete
>> uses a delay (`icomplete-compute-delay'). Other completion UIs don't
>> have such a delay and show the candidates immediately. This makes the
>> problem a little more difficult, in particular with respect to auto
>> detection.
>
> Alternatively, we could consider the cases where more than one
> completion list is shown a bug in the mode which shows the completions
> even though the application already did.  IOW, instead of considering
> this a problem of the command the user invokes, consider this a bug in
> the non-default completion UI currently in effect.  It is basically a
> flaw in the design of those completion UIs.

I see it differently. The problem is in the tmm and ffap commands which
lead to a mixture of completion UIs. Even if another completion UI is in
effect, the default completion UI is called by tmm and ffap, bypassing
the `completing-read' abstraction. It is not the responsibility of
alternative completion UIs to work around this. Mixing multiple
completion UIs is not ideal since it will lead to an incoherent UI and
also to conflicts in how the user interacts with the minibuffer.

>> Another alternative to auto detection could be that the completion table
>> communicates to `completing-read' via metadata that immediate candidate
>> display is desired. The completion UI could then act accordingly.
>> Default completion would call `minibuffer-completion-help' and Icomplete
>> could update immediately, ignoring `icomplete-compute-delay'.
>
> This sounds too indirect to me, it could cause unintended adverse
> consequences, especially in nested scenarios.

By using the completion metadata as part of the completion table, the
effect on nested scenarios is explicitly avoided. Problems with nesting
only occur if a variable is let-bound around `completing-read'.

> There's a simpler alternative: we could say we don't care, as long as
> only a few alternative UIs have this issue.  It isn't a catastrophe.

I care. But I do agree with you that the issues are minor and far from a
catastrophe.

The pattern where completion commands want to display candidates
immediately is not uncommon. There are ffap, tmm and multiple
third-party packages which have such a requirement. So I suggest to not
necessarily treat "immediate candidate display" as a bug report, but
rather as a feature request for `completing-read'.

bug#74617: 30.0.92; ffap-menu always displays the *Completions* buffer

[Eli Zaretskii]

> From: Daniel Mendler <mail@daniel-mendler.de>
> Cc: 74617@debbugs.gnu.org,  monnier@iro.umontreal.ca
> Date: Sat, 30 Nov 2024 10:40:05 +0100
> 
> Eli Zaretskii <eliz@gnu.org> writes:
> > Alternatively, we could consider the cases where more than one
> > completion list is shown a bug in the mode which shows the completions
> > even though the application already did.  IOW, instead of considering
> > this a problem of the command the user invokes, consider this a bug in
> > the non-default completion UI currently in effect.  It is basically a
> > flaw in the design of those completion UIs.
> 
> I see it differently. The problem is in the tmm and ffap commands which
> lead to a mixture of completion UIs. Even if another completion UI is in
> effect, the default completion UI is called by tmm and ffap, bypassing
> the `completing-read' abstraction.

I don't understand: ffap-menu calls completing-read, so what
abstraction does it bypass, and how?

> It is not the responsibility of
> alternative completion UIs to work around this.

Work around what?

> Mixing multiple
> completion UIs is not ideal since it will lead to an incoherent UI and
> also to conflicts in how the user interacts with the minibuffer.

The question is: who should pay attention and understand that "the
other" completion is being called?

> >> Another alternative to auto detection could be that the completion table
> >> communicates to `completing-read' via metadata that immediate candidate
> >> display is desired. The completion UI could then act accordingly.
> >> Default completion would call `minibuffer-completion-help' and Icomplete
> >> could update immediately, ignoring `icomplete-compute-delay'.
> >
> > This sounds too indirect to me, it could cause unintended adverse
> > consequences, especially in nested scenarios.
> 
> By using the completion metadata as part of the completion table, the
> effect on nested scenarios is explicitly avoided. Problems with nesting
> only occur if a variable is let-bound around `completing-read'.

You assume that the completion table is never passed to inner levels?
Is that assumption solid?

bug#74617: 30.0.92; ffap-menu always displays the *Completions* buffer

[Daniel Mendler via Bug reports for GNU Emacs, the Swiss army knife of text editors]

Eli Zaretskii <eliz@gnu.org> writes:

>> From: Daniel Mendler <mail@daniel-mendler.de>
>> Cc: 74617@debbugs.gnu.org,  monnier@iro.umontreal.ca
>> Date: Sat, 30 Nov 2024 10:40:05 +0100
>> 
>> Eli Zaretskii <eliz@gnu.org> writes:
>> > Alternatively, we could consider the cases where more than one
>> > completion list is shown a bug in the mode which shows the completions
>> > even though the application already did.  IOW, instead of considering
>> > this a problem of the command the user invokes, consider this a bug in
>> > the non-default completion UI currently in effect.  It is basically a
>> > flaw in the design of those completion UIs.
>> 
>> I see it differently. The problem is in the tmm and ffap commands which
>> lead to a mixture of completion UIs. Even if another completion UI is in
>> effect, the default completion UI is called by tmm and ffap, bypassing
>> the `completing-read' abstraction.
>
> I don't understand: ffap-menu calls completing-read, so what
> abstraction does it bypass, and how?

It also calls `minibuffer-completion-help' which belongs to the default
completion UI but not strictly to the abstract `completing-read' API.
Unfortunately the API boundaries are not clearly defined. In principle
one could separate the generic `completing-read' code from the default
completion UI code. This could help keeping the complexity of the
completion code in check. Right now the default completion UI code is
scattered across minibuffer.el and simple.el. 

>> It is not the responsibility of
>> alternative completion UIs to work around this.
>
> Work around what?

Hiding the *Completions* buffer. It should not be shown in the first
place if the user replaced the `completing-read-function' with something
else by turning on the mode of an alternative UI.

>> >> Another alternative to auto detection could be that the completion table
>> >> communicates to `completing-read' via metadata that immediate candidate
>> >> display is desired. The completion UI could then act accordingly.
>> >> Default completion would call `minibuffer-completion-help' and Icomplete
>> >> could update immediately, ignoring `icomplete-compute-delay'.
>> >
>> > This sounds too indirect to me, it could cause unintended adverse
>> > consequences, especially in nested scenarios.
>> 
>> By using the completion metadata as part of the completion table, the
>> effect on nested scenarios is explicitly avoided. Problems with nesting
>> only occur if a variable is let-bound around `completing-read'.
>
> You assume that the completion table is never passed to inner levels?
> Is that assumption solid?

Yes, as far as I can tell. Completion can work well in recursive
minibuffers, such that the inner completion session does not interfere
with the outer session. Stefan even changed the scope of the
`minibuffer-completion-table' variables a while ago, such that it is
only bound buffer-locally in the minibuffer. Earlier it was let-bound
around the `read-from-minibuffer' call. (Of course it is still possible
to access the completion table by reading the buffer local variable from
the minibuffer directly, but this doesn't happen accidentally.)

bug#74617: 30.0.92; ffap-menu always displays the *Completions* buffer

[Eli Zaretskii]

> From: Daniel Mendler <mail@daniel-mendler.de>
> Cc: 74617@debbugs.gnu.org,  monnier@iro.umontreal.ca
> Date: Sat, 30 Nov 2024 17:25:07 +0100
> 
> Eli Zaretskii <eliz@gnu.org> writes:
> 
> >> I see it differently. The problem is in the tmm and ffap commands which
> >> lead to a mixture of completion UIs. Even if another completion UI is in
> >> effect, the default completion UI is called by tmm and ffap, bypassing
> >> the `completing-read' abstraction.
> >
> > I don't understand: ffap-menu calls completing-read, so what
> > abstraction does it bypass, and how?
> 
> It also calls `minibuffer-completion-help' which belongs to the default
> completion UI but not strictly to the abstract `completing-read' API.

So the problem is this single line?

      (let ((minibuffer-setup-hook 'minibuffer-completion-help))

bug#74617: 30.0.92; ffap-menu always displays the *Completions* buffer

[Daniel Mendler via Bug reports for GNU Emacs, the Swiss army knife of text editors]

Eli Zaretskii <eliz@gnu.org> writes:

>> From: Daniel Mendler <mail@daniel-mendler.de>
>> Cc: 74617@debbugs.gnu.org,  monnier@iro.umontreal.ca
>> Date: Sat, 30 Nov 2024 17:25:07 +0100
>> 
>> Eli Zaretskii <eliz@gnu.org> writes:
>> 
>> >> I see it differently. The problem is in the tmm and ffap commands which
>> >> lead to a mixture of completion UIs. Even if another completion UI is in
>> >> effect, the default completion UI is called by tmm and ffap, bypassing
>> >> the `completing-read' abstraction.
>> >
>> > I don't understand: ffap-menu calls completing-read, so what
>> > abstraction does it bypass, and how?
>> 
>> It also calls `minibuffer-completion-help' which belongs to the default
>> completion UI but not strictly to the abstract `completing-read' API.
>
> So the problem is this single line?
>
>       (let ((minibuffer-setup-hook 'minibuffer-completion-help))

Yes.

But maybe it makes sense to tackle the "immediate candidate display
problem" more generally. It would be great to get Stefan's input on
this.

For example in my osm.el package I have the following code to implement
immediate display of candidates. The lambda implements the
auto-detection, which I don't find particularly elegant.

(minibuffer-with-setup-hook
    (lambda ()
      (when (and (eq completing-read-function #'completing-read-default)
                 (not (bound-and-true-p vertico-mode))
                 (not (bound-and-true-p icomplete-mode)))
        (let ((message-log-max nil)
              (inhibit-message t))
          ;; Show matches immediately for default completion.
          (minibuffer-completion-help))))
     (completing-read ...))

Initially I had forgotten the call to `minibuffer-completion-help' since
most of them time I use a completion UI which displays the candidates
automatically anyway. Then Richard reported the bug, that `osm-search'
doesn't work conveniently in Emacs by default. Furthermore in my jinx.el
package I have almost exactly the same code in `jinx--correct-setup',
which is used like this:

(minibuffer-with-setup-hook
    #'jinx--correct-setup
  (completing-read ...))

Daniel

bug#74617: 30.0.92; ffap-menu always displays the *Completions* buffer

[Stefan Monnier via Bug reports for GNU Emacs, the Swiss army knife of text editors]

> It also calls `minibuffer-completion-help' which belongs to the default
> completion UI but not strictly to the abstract `completing-read' API.

Maybe `minibuffer-completion-help` should call
a `minibuffer-completion-help-function`, so that it shows the help using
the user's favorite UI?


        Stefan

bug#74617: 30.0.92; ffap-menu always displays the *Completions* buffer

[Daniel Mendler via Bug reports for GNU Emacs, the Swiss army knife of text editors]

Stefan Monnier <monnier@iro.umontreal.ca> writes:

>> It also calls `minibuffer-completion-help' which belongs to the default
>> completion UI but not strictly to the abstract `completing-read' API.
>
> Maybe `minibuffer-completion-help` should call
> a `minibuffer-completion-help-function`, so that it shows the help using
> the user's favorite UI?

Yes, this was one of the proposals I made. However this has a downside.
It always prevents displaying the *Completions* buffer, while sometimes
it may be desired to open it on demand.

Daniel

bug#74617: 30.0.92; ffap-menu always displays the *Completions* buffer

[Juri Linkov]

> The pattern where completion commands want to display candidates
> immediately is not uncommon. There are ffap, tmm and multiple
> third-party packages which have such a requirement. So I suggest to not
> necessarily treat "immediate candidate display" as a bug report, but
> rather as a feature request for `completing-read'.

And imenu.el calls minibuffer-completion-help conditionally
unless imenu-eager-completion-buffer is not nil.

bug#74617: 30.0.92; ffap-menu always displays the *Completions* buffer

[Daniel Mendler via Bug reports for GNU Emacs, the Swiss army knife of text editors]

Juri Linkov <juri@linkov.net> writes:

>> The pattern where completion commands want to display candidates
>> immediately is not uncommon. There are ffap, tmm and multiple
>> third-party packages which have such a requirement. So I suggest to not
>> necessarily treat "immediate candidate display" as a bug report, but
>> rather as a feature request for `completing-read'.
>
> And imenu.el calls minibuffer-completion-help conditionally
> unless imenu-eager-completion-buffer is not nil.

Thanks, I wasn't aware of this option. It seems the behavior is
inverted? The completion buffer pops up if imenu-eager-completion-buffer
is nil.

(defcustom imenu-eager-completion-buffer t
  "If non-nil, eagerly pop up the completion buffer."
  :type 'boolean
  :version "22.1")

It is likely that there are more such uses in the Emacs code and other
packages. One possible solution would be a display-eager metadata:

(completing-read "Test: "
 (lambda (string pred action)
   (if (eq action 'metadata)
       '(metadata (display-eager . t))
     (complete-with-action action #'read-file-name-internal string pred))))

This pushes the responsibility of displaying the candidates to
`completing-read'. In `completing-read-default' the following code
would have to be added to the `minibuffer-with-setup-hook':
                
(when (completion-metadata-get
       (completion-metadata "" minibuffer-completion-table nil)
       'display-eager)
  (minibuffer-completion-help))

Daniel

bug#74617: 30.0.92; ffap-menu always displays the *Completions* buffer

[Juri Linkov]

>> And imenu.el calls minibuffer-completion-help conditionally
>> unless imenu-eager-completion-buffer is not nil.
>
> Thanks, I wasn't aware of this option. It seems the behavior is
> inverted? The completion buffer pops up if imenu-eager-completion-buffer
> is nil.

Unfortunately, it is inverted indeed.  And there is even bug#52389
about this, but it can't be changed for backward-compatibility reasons.

> It is likely that there are more such uses in the Emacs code and other
> packages. One possible solution would be a display-eager metadata:
>
> (completing-read "Test: "
>  (lambda (string pred action)
>    (if (eq action 'metadata)
>        '(metadata (display-eager . t))
>      (complete-with-action action #'read-file-name-internal string pred))))
>
> This pushes the responsibility of displaying the candidates to
> `completing-read'. In `completing-read-default' the following code
> would have to be added to the `minibuffer-with-setup-hook':
>                 
> (when (completion-metadata-get
>        (completion-metadata "" minibuffer-completion-table nil)
>        'display-eager)
>   (minibuffer-completion-help))

Adding a new metadata item looks like the right thing to do.
Then it will be possible to configure it via completion-category-overrides.