cpitclaudel / biblio.el

Browse and import bibliographic references from CrossRef, DBLP, HAL, arXiv, Dissemin, and doi.org from Emacs
GNU General Public License v3.0
180 stars 14 forks source link

Replace the `biblio-completing-read-function` with a customized variable #55

Closed nicehiro closed 1 year ago

nicehiro commented 2 years ago

Hi, thanks for your really great work!

The default biblio-completing-read-function will return ido-completing-function if we use default completing-read function.

vertico is a enhancement completion UI which uses default completing-read function, so it would be better to set this function directly to what completing-read-function as you what.

cpitclaudel commented 2 years ago

At the time the code was written, however (and still now, unfortunately, I think), it was not possible to rebind completing-read-default to ido-completing-read, and I really wanted there to be some form of completion on the two places where biblio prompts the user. I was hoping that checking for completing-read-default would be enough.

How does vertico actually work? Does it not set completing-read-function?

nicehiro commented 2 years ago

How does vertico actually work? Does it not set completing-read-function?

As written in the vertico repository, vertico only provide a completion UI to default completing system. So the completing-read-function is still completing-read-default.

Maybe you can check vertico here if you have time.

At the time the code was written, however (and still now, unfortunately, I think), it was not possible to rebind completing-read-default to ido-completing-read

Maybe we can just rebind biblio-completing-read-function? Like:

(defvar biblio-completing-read-function #'ido-completing-read) ;; or completing-read-default

(defun biblio-completing-read (prompt collection &optional predicate require-match
                                      initial-input hist def inherit-input-method)
  "Complete using `biblio-completing-read-function'.
PROMPT, COLLECTION, PREDICATE, REQUIRE-MATCH, INITIAL-INPUT,
HIST, DEF, INHERIT-INPUT-METHOD: see `completing-read'."
  (let ((completing-read-function biblio-completing-read-function))
    (completing-read prompt collection predicate require-match
                     initial-input hist def inherit-input-method)))
cpitclaudel commented 2 years ago

Maybe you can check vertico here if you have time.

Thanks, I just looked. Do you know why vertico advises the default completing-read-function instead of setting it? In any case, that suggests to me that we could check for advice in addition to checking for completing-read-function having its default value. How does that sound to you?

Maybe we can just rebind biblio-completing-read-function?

Unfortunately, this will break things for anyone who has completing-read-function set to a non-default value (it will ignore their choice).

nicehiro commented 2 years ago

Do you know why vertico advises the default completing-read-function instead of setting it?

I guess it's for simple and don't want to invent new API in Emacs.

check for advice in addition to checking for completing-read-function having its default value

I think this will work.

(defun biblio--completing-read-function ()
  "Return ido, unless user picked another completion package."
  (if (and (eq completing-read-function #'completing-read-default)
           (not (advice--p (advice--symbol-function #'completing-read-default))))
      #'ido-completing-read
    completing-read-function))
nicolas-graves commented 1 year ago

I've tried to use this last function and can confirm it seems to work.