Advice around completing-read-multiple leads to error when PREDICATE argument is non-nil?

[apc]

When a PREDICATE argument is passed to completing-read-multiple, I get a bit of unexpected behavior when vertico-mode is on. I'm not quite sure how best to describe it, but what seems to be going on is this:

• by default, completing-read-multiple will use its PREDICATE argument to filter the possible completions after each time I insert crm-separator.
• when vertico-mode is on, the previously selected items (after inserting a crm-separator) are prepended to possible completions before testing with PREDICATE.

There may be some conceptual confusion in my explanation above, so perhaps an example can help.

Consider the following minimal example, without using Vertico:

(defvar my/test-list '(("Apples" (:location "refrigerator" :amount 3))
                       ("Bananas" (:location "counter" :amount 6))
                       ("Oranges" (:location "counter" :amount 0))))

(defun my/have-some-crm (item)
  (> (plist-get (car (cdr (assoc item my/test-list))) :amount) 0))

(completing-read-multiple "Select some items: " my/test-list #'my/have-some-crm)

As expected, at first I'm only offered as completion "Apples" and "Bananas" ("Oranges" is not offered). After selecting one item and then entering a comma in the minibuffer, I'm again offered as possible completions "Apples" and "Bananas", not "Oranges".

If I enable vertico-mode, however, running the same code as above yields an error. Here's the beginning of the backtrace:

Debugger entered--Lisp error: (wrong-type-argument number-or-marker-p nil)
  >(nil 0)
  my/have-some-crm("Apples,Apples")
  #f(compiled-function (s) #<bytecode -0xe0346ec0898bd72>)(("Apples" (:location "refrigerator" :amount 3)))
  all-completions("" (("Apples" (:location "refrigerator" :amount 3)) ("Bananas" (:location "counter" :amount 6)) ("Oranges" (:location "counter" :amount 0))) #f(compiled-function (s) #<bytecode -0xe0346ec0898bd72>))
  complete-with-action(t (("Apples" (:location "refrigerator" :amount 3)) ("Bananas" (:location "counter" :amount 6)) ("Oranges" (:location "counter" :amount 0))) "" #f(compiled-function (s) #<bytecode -0xe0346ec0898bd72>))
  completion-table-with-context("Apples," (("Apples" (:location "refrigerator" :amount 3)) ("Bananas" (:location "counter" :amount 6)) ("Oranges" (:location "counter" :amount 0))) "" my/have-some-crm t)
  crm--collection-fn("Apples," my/have-some-crm t)
  all-completions("Apples," crm--collection-fn my/have-some-crm)
  completion-pcm--all-completions("Apples," (point) crm--collection-fn my/have-some-crm)
  completion-basic-all-completions("Apples," crm--collection-fn my/have-some-crm 7)
  #f(compiled-function (style) #<bytecode -0x69b0a22c44f9522>)(basic)
  completion--some(#f(compiled-function (style) #<bytecode -0x69b0a22c44f9522>) (basic partial-completion emacs22))
  completion--nth-completion(2 "Apples," crm--collection-fn my/have-some-crm 7 (metadata))
  completion-all-completions("Apples," crm--collection-fn my/have-some-crm 7 (metadata))
  apply(completion-all-completions ("Apples," crm--collection-fn my/have-some-crm 7 (metadata)))
  vertico--all-completions("Apples," crm--collection-fn my/have-some-crm 7 (metadata))
  vertico--recompute(7 "Apples,")
  vertico--update(interruptible)
  vertico--exhibit()

I'm assuming something about the advice to completing-read-multiple seems to be making it test "Apples,Apples", rather than "Apples" with my/have-some-crm. I can fix this issue by defining a new function (I'm using dash.el here) and using it instead as the `PREDICATE' argument:

(defun my/have-some-crm-str (str)
  (-all? #'my/have-some-crm (split-string str crm-separator)))

(completing-read-multiple "Select some items: " my/test-list #'my/have-some-crm-str)

Of course, this new function will work both with or without vertico-mode on, since the new test function is more general than the previous one. So in a sense I don't really need this issue addressed. But maybe I'm missing something?

At this point I mostly wanted to know whether this is the intended behavior. If so, would it be helpful to document it somewhere?

[minad]

I am not sure what is intended, but the error also occurs without Vertico on emacs -Q.

1. Evaluate your code
2. Enter Apples,Apples
3. Press M-RET

[apc]

Hmm. That's odd. On my end, evaluating this

(defvar my/test-list '(("Apples" (:location "refrigerator" :amount 3))
                       ("Bananas" (:location "counter" :amount 6))
                       ("Oranges" (:location "counter" :amount 0))))

(defun my/have-some-crm (item)
  (> (plist-get (car (cdr (assoc item my/test-list))) :amount) 0))

and then

(completing-read-multiple "Select some items: " my/test-list #'my/have-some-crm)

allows me to type "Apples,Apples" and get, as a result, the list ("Apples" "Apples").

(EDIT: pressing M-RET doesn't do anything on my end with emacs -Q ("M-RET is undefined"), I need to instead press RET and get the result I mentioned above.)

Here's what I get when I call emacs-version:

GNU Emacs 28.2 (build 1, aarch64-apple-darwin21.6.0, NS appkit-2113.60 Version 12.6.2 (Build 21G320)) of 2023-05-01

[apc]

Oh, and in response to the question about what was intended, I guess I expected the same result I would get calling completing-read-multiple with a nil PREDICATE argument? (Assuming of course the items selected satisfied the predicate in question.) Does that make sense?

[minad]

I see, M-RET is new in Emacs 29. You can instead invoke minibuffer-force-complete-and-exit to observe the error.

(define-key minibuffer-local-completion-map (kbd "M-RET") #'minibuffer-force-complete-and-exit)

[apc]

Ah, I see. I get the same error if I call minibuffer-force-complete-and-exit (or minibuffer-force-complete). I don't, however, if I call exit-minibuffer by pressing RET. I only get an error by pressing RET when vertico-mode is on—since the good case scenario is one where you don't need to force completion, but actually select one from among the possible completions, this is at least unexpected. No?

I'm guessing there's something I don't quite understand about how the PREDICATE argument is supposed to work. For one thing, I don't see why, when you call minibuffer-force-complete-and-exit, the string in the minibuffer is still being tested using PREDICATE. But if it does need to be tested, I sort of see why you'd look for a match for the string as is, rather than just testing what follows the latest occurrence of crm-separator.

[minad]

For one thing, I don't see why, when you call minibuffer-force-complete-and-exit, the string in the minibuffer is still being tested using PREDICATE. But if it does need to be tested, I sort of see why you'd look for a match for the string as is, rather than just testing what follows the latest occurrence of crm-separator.

Yes, this sounds like a bug. Somewhere in crm--collection-fn or completion-table-with-context perhaps?

[apc]

Could this explain why I’m getting the error with vertico-mode on when I try to just call exit-minibuffer?

[minad]

On 10/5/23 22:42, apc wrote: Could this explain why I’m getting the error with |vertico-mode| on when I try to just call |exit-minibuffer|?

Yes. Note that you actually invoke vertico-exit, which calls the completion table in a similar way as minibuffer-force-complete.