My read-buffer-function doesn't work when called by switch-to-buffer

My read-buffer-function doesn't work when called by switch-to-buffer

[Tassilo Horn]

Hi all,

I would like that switch-to-buffer also suggests recent files.  Given
that it uses read-buffer which can be customized using a
read-buffer-function, I thought that should be easy.  That's what I have
come up with:

--8<---------------cut here---------------start------------->8---
(defun th/read-buffer-or-recentf (prompt &optional
                                         def require-match predicate)
  (when-let ((result (completing-read
                      prompt
                      (completion-table-in-turn #'internal-complete-buffer
                                                (completion-table-dynamic
                                                 (lambda (s) recentf-list)))
                      predicate require-match nil 'buffer-name-history def)))
    (cond
     ((get-buffer result) result)
     ((file-exists-p result) (buffer-name (find-file-noselect result)))
     (t result))))

(setq read-buffer-function #'th/read-buffer-or-recentf)
--8<---------------cut here---------------end--------------->8---

The function th/read-buffer-or-recentf works fine when called
standalone, i.e., when I evaluate

  (th/read-buffer-or-recentf "test: ")

and type something which won't complete to an existing buffer, recent
files pop up as completion candidates.  Yay!

However, when I do C-x b (switch-to-buffer), no matter what, recent
files are not provided as completion candidates.  But edebug convinces
me that my function th/read-buffer-or-recentf is called.  It just seems
that the same completing-read call behaves differently when called
directly and when being called by switch-to-buffer.  Why is that and
what can I do against it?

Thanks,
Tassilo

Re: My read-buffer-function doesn't work when called by switch-to-buffer

[Michael Heerdegen]

Tassilo Horn <tsdh@gnu.org> writes:

> (defun th/read-buffer-or-recentf (prompt &optional
>                                          def require-match predicate)
>   (when-let ((result (completing-read
>                       prompt
>                       (completion-table-in-turn #'internal-complete-buffer
>                                                 (completion-table-dynamic
>                                                  (lambda (s) recentf-list)))
>                       predicate require-match nil 'buffer-name-history def)))
>     (cond
>      ((get-buffer result) result)
>      ((file-exists-p result) (buffer-name (find-file-noselect result)))
>      (t result))))
>
> (setq read-buffer-function #'th/read-buffer-or-recentf)
> [...]
> However, when I do C-x b (switch-to-buffer), no matter what, recent
> files are not provided as completion candidates.  But edebug convinces
> me that my function th/read-buffer-or-recentf is called.  It just seems
> that the same completing-read call behaves differently when called
> directly and when being called by switch-to-buffer.  Why is that and
> what can I do against it?

Seems this happens because `read-buffer-to-switch' sets
`minibuffer-completion-table'.

Michael.

Re: My read-buffer-function doesn't work when called by switch-to-buffer

[Tassilo Horn]

Michael Heerdegen <michael_heerdegen@web.de> writes:

Hi Michael,

>> However, when I do C-x b (switch-to-buffer), no matter what, recent
>> files are not provided as completion candidates.  But edebug
>> convinces me that my function th/read-buffer-or-recentf is called.
>> It just seems that the same completing-read call behaves differently
>> when called directly and when being called by switch-to-buffer.  Why
>> is that and what can I do against it?
>
> Seems this happens because `read-buffer-to-switch' sets
> `minibuffer-completion-table'.

Ah, that's it!  This code works:

--8<---------------cut here---------------start------------->8---
(defun th/read-buffer-or-recentf (prompt &optional
                                         def require-match predicate)
  (let ((completion-table (completion-table-in-turn
                           #'internal-complete-buffer
                           (completion-table-dynamic
                            (lambda (s) recentf-list)))))
    (minibuffer-with-setup-hook
        (:append (lambda ()
                   (setq-local minibuffer-completion-table completion-table)))
      (when-let ((result (completing-read prompt completion-table
                                          predicate require-match nil
                                          'buffer-name-history def)))
        (cond
         ((get-buffer result) result)
         ((file-exists-p result) (buffer-name (find-file-noselect result)))
         (t result))))))

(setq read-buffer-function #'th/read-buffer-or-recentf)
--8<---------------cut here---------------end--------------->8---

But why is that needed?  I had expected that when I give a completion
table to completing-read, it would be used...

And a bonus question: I use vertico + marginalia, so the buffers (and
files in C-x C-f, variables/functions in C-h v/f,...) are nicely
annotated, e.g., with the buffer's mode or the file's permissions.  What
do I have to do that the recentf candidates also get those niceties?
Wrap the recentf completion table so that it responds to metadata
requests?

Bye,
Tassilo

`when-let' (was: Re: My read-buffer-function doesn't work when called by switch-to-buffer)

[Emanuel Berg]

Tassilo Horn wrote:

> (when-let ((result (completing-read [...] ))))

`when-let' is so cool!

I didn't use it once. But I used `let' 457 times, and `when'
161 times.

So what about this

(setq initial-scratch-message nil)
(when-let ((buf (get-buffer "*scratch*")))
  (kill-buffer buf) )

?

There is also `if-let'. But no "unless-let". Yeah, like if all
of these are nil, then do something with them ...

There are also the sequential/non-"parallel"/self-refering
`when-let*' and `if-let*'!

Actually, there are

  and-let*
  cl--loop-let
  if-let[*]
  macroexp-let*
  macroexp-let2[*]
  map-let
  named-let
  pcase-let[*]
  rx-let
  seq-let
  when-let[*]
  while-let

and more!

I have 6 `pcase-let' and 1 `pcase-let*' and - now -
1 `when-let'.

Yeah - it's not a lot :(

-- 
underground experts united
https://dataswamp.org/~incal

Re: My read-buffer-function doesn't work when called by switch-to-buffer

[Michael Heerdegen]

Tassilo Horn <tsdh@gnu.org> writes:

> But why is that needed?  I had expected that when I give a completion
> table to completing-read, it would be used...

AFAIU both the outer call and your (original!) `completing-read' call
both set `minibuffer-completion-table' - the former (via
`read-buffer-to-switch') directly by using `minibuffer-with-setup-hook',
your code indirectly by calling `completing-read' (see
`completing-read-default' which does

  (setq-local minibuffer-completion-table collection)

in a `minibuffer-with-setup-hook').  Which one wins?  Since
`minibuffer-with-setup-hook' prepends to the hook by default, the outer
call wins.

I think the lesson is that it's better to avoid nested `completing-read'
calls.  It is probably not intended to allow to change the completion
table in `read-buffer-function'.  Maybe an advice of a higher level
function would better fit your wished-for behavior.

> And a bonus question: I use vertico + marginalia, so the buffers (and
> files in C-x C-f, variables/functions in C-h v/f,...) are nicely
> annotated, e.g., with the buffer's mode or the file's permissions.  What
> do I have to do that the recentf candidates also get those niceties?
> Wrap the recentf completion table so that it responds to metadata
> requests?

Dunno.  I don't use these packages.  Maybe using :annotation-function in
`completion-extra-properties' also works?  How do these packages achieve
this?


Michael.

Re: My read-buffer-function doesn't work when called by switch-to-buffer

[Tassilo Horn]

Michael Heerdegen <michael_heerdegen@web.de> writes:

Hi Michael,

> I think the lesson is that it's better to avoid nested
> `completing-read' calls.  It is probably not intended to allow to
> change the completion table in `read-buffer-function'.

Seems so but it nevertheless works extremely well since I strongly
stated my wish by appending to minibuffer-with-setup-hook. :-)

> Maybe an advice of a higher level function would better fit your
> wished-for behavior.

Do you have one in mind?

>> And a bonus question: I use vertico + marginalia, so the buffers (and
>> files in C-x C-f, variables/functions in C-h v/f,...) are nicely
>> annotated, e.g., with the buffer's mode or the file's permissions.
>> What do I have to do that the recentf candidates also get those
>> niceties?  Wrap the recentf completion table so that it responds to
>> metadata requests?
>
> Dunno.  I don't use these packages.  Maybe using :annotation-function
> in `completion-extra-properties' also works?  How do these packages
> achieve this?

I haven't checked their implementation but completion-extra-properties
is nil and the metadata completion requests also don't return an
annotation-function or affixation-function.  Nevertheless, all standard
completions are annotated.  I'll have a look how marginalia does it.

Anyway, I've played a bit with responding to metadata requests and it
looks like it's not really suited for my goal because it seems to assume
that the metadata won't change (e.g., category from buffer to file)
during a single completion which is obviously a reasonable assumption.

Bye,
Tassilo

Re: My read-buffer-function doesn't work when called by switch-to-buffer

[Michael Heerdegen]

Tassilo Horn <tsdh@gnu.org> writes:

> > Maybe an advice of a higher level function would better fit your
> > wished-for behavior.
>
> Do you have one in mind?

`read-buffer-to-switch'.

> I haven't checked their implementation but completion-extra-properties
> is nil and the metadata completion requests also don't return an
> annotation-function or affixation-function.  Nevertheless, all standard
> completions are annotated.  I'll have a look how marginalia does it.

They do this:

(advice-add #'completion-metadata-get
            :before-until #'marginalia--completion-metadata-get)

Looks like this is how they install their annotation-function.

> Anyway, I've played a bit with responding to metadata requests and it
> looks like it's not really suited for my goal because it seems to assume
> that the metadata won't change (e.g., category from buffer to file)
> during a single completion which is obviously a reasonable assumption.

Hmm, yes, that's a problem.  Seems a completion table can only provide
one ind of metadata for all completions and what you want doesn't fit at
all into that approach.  See the comment in `completion-table-in-turn'.


Michael.

Re: My read-buffer-function doesn't work when called by switch-to-buffer

[Tassilo Horn]

Michael Heerdegen <michael_heerdegen@web.de> writes:

Hi Michael,

>> > Maybe an advice of a higher level function would better fit your
>> > wished-for behavior.
>>
>> Do you have one in mind?
>
> `read-buffer-to-switch'.

Hm, but then it would be pretty specific to `switch-to-buffer' instead
of a general `read-buffer-function'.  E.g., I just found out it's quite
convienient to be able to use `append-to-buffer' to append to a buffer
whose file I haven't found already but is in recentf-list.

>> I haven't checked their implementation but completion-extra-properties
>> is nil and the metadata completion requests also don't return an
>> annotation-function or affixation-function.  Nevertheless, all standard
>> completions are annotated.  I'll have a look how marginalia does it.
>
> They do this:
>
> (advice-add #'completion-metadata-get
>             :before-until #'marginalia--completion-metadata-get)
>
> Looks like this is how they install their annotation-function.

Yes, thanks for the pointer.

>> Anyway, I've played a bit with responding to metadata requests and it
>> looks like it's not really suited for my goal because it seems to
>> assume that the metadata won't change (e.g., category from buffer to
>> file) during a single completion which is obviously a reasonable
>> assumption.
>
> Hmm, yes, that's a problem.  Seems a completion table can only provide
> one kind of metadata for all completions and what you want doesn't fit
> at all into that approach.  See the comment in
> `completion-table-in-turn'.

Agreed.

Bye,
Tassilo

Re: My read-buffer-function doesn't work when called by switch-to-buffer

[Michael Heerdegen]

Tassilo Horn <tsdh@gnu.org> writes:

> >> > Maybe an advice of a higher level function would better fit your
> >> > wished-for behavior.
> >>
> >> Do you have one in mind?
> >
> > `read-buffer-to-switch'.
>
> Hm, but then it would be pretty specific to `switch-to-buffer' instead
> of a general `read-buffer-function'.  E.g., I just found out it's quite
> convienient to be able to use `append-to-buffer' to append to a buffer
> whose file I haven't found already but is in recentf-list.

Depends on what you want of course.  It doesn't make sense for
everything (not for `kill-buffer' for example).  If you are not confused
by getting file completions for `kill-buffer' then you can just keep
what you have.

This one of those things one needs to try out.


Michael.

Re: My read-buffer-function doesn't work when called by switch-to-buffer

[Tassilo Horn]

Michael Heerdegen <michael_heerdegen@web.de> writes:

>> Hm, but then it would be pretty specific to `switch-to-buffer'
>> instead of a general `read-buffer-function'.  E.g., I just found out
>> it's quite convienient to be able to use `append-to-buffer' to append
>> to a buffer whose file I haven't found already but is in
>> recentf-list.
>
> Depends on what you want of course.  It doesn't make sense for
> everything (not for `kill-buffer' for example).

Good point (although it doesn't really hurt).  I now made my function a
bit customizable depending on `this-command'.

--8<---------------cut here---------------start------------->8---
(defconst th/read-buffer-or-recentf-command-alist
  "Alist with entries of the form (CMD . COMPLETES).
COMPLETES is a list defining what's completed where entries can
be:

- `buffers':        completion for all buffers
- `buffers-except': completion for all buffers except the current one
- `recentf':        completion for recent files which will be found on demand"
  '((kill-buffer buffers)
    (switch-to-buffer buffers-except recentf)
    (pop-to-buffer buffers-except recentf)))

(defun th/read-buffer-or-recentf (prompt &optional
                                         def require-match predicate)
  (let* ((tables (or
                  (mapcar
                   (lambda (syms)
                     (pcase syms
                       ('buffers #'internal-complete-buffer)
                       ('buffers-except (internal-complete-buffer-except
                                         (current-buffer)))
                       ('recentf (completion-table-dynamic
                                  (lambda (s) recentf-list)))
                       (unknown  (error "Unknown case %S" unknown))))
                   (cdr (assoc this-command
                               th/read-buffer-or-recentf-command-alist)))
                  (list #'internal-complete-buffer
                        (completion-table-dynamic
                         (lambda (s) recentf-list)))))
         (completion-table (apply #'completion-table-in-turn tables)))
    ;; Bind minibuffer-setup-hook to nil so that we can be sure our completion
    ;; table is used.  `read-buffer-to-switch' (called by `switch-to-buffer')
    ;; sets `internal-complete-buffer' using `minibuffer-with-setup-hook'
    ;; before `read-buffer-function' is invoked by `read-buffer', so we'd be
    ;; restricted to buffers.
    (let ((minibuffer-setup-hook nil))
      (when-let ((result (completing-read prompt completion-table
                                          predicate require-match nil
                                          'buffer-name-history def)))
        (cond
         ((get-buffer result) result)
         ((file-exists-p result) (buffer-name (find-file-noselect result)))
         (t result))))))

(setopt read-buffer-function #'th/read-buffer-or-recentf)
--8<---------------cut here---------------end--------------->8---

Bye,
Tassilo

Re: My read-buffer-function doesn't work when called by switch-to-buffer

[Tassilo Horn]

Tassilo Horn <tsdh@gnu.org> writes:

>> Depends on what you want of course.  It doesn't make sense for
>> everything (not for `kill-buffer' for example).
>
> Good point (although it doesn't really hurt).  I now made my function a
> bit customizable depending on `this-command'.
>
> (defconst th/read-buffer-or-recentf-command-alist
>   "Alist with entries of the form (CMD . COMPLETES).
> COMPLETES is a list defining what's completed where entries can
> be:
>
> - `buffers':        completion for all buffers
> - `buffers-except': completion for all buffers except the current one
> - `recentf':        completion for recent files which will be found on demand"
>   '((kill-buffer buffers)
>     (switch-to-buffer buffers-except recentf)
>     (pop-to-buffer buffers-except recentf)))

Gosh, that's what you get when adding a docstring before posting!  Of
course, you put it at the wrong location.  So here it is fixed.

--8<---------------cut here---------------start------------->8---
(defconst th/read-buffer-or-recentf-command-alist
  '((kill-buffer buffers)
    (switch-to-buffer buffers-except recentf)
    (pop-to-buffer buffers-except recentf))
  "Alist with entries of the form (CMD . COMPLETES).
COMPLETES is a list defining what's completed where entries can
be:

- `buffers':        completion for all buffers
- `buffers-except': completion for all buffers except the current one
- `recentf':        completion for recent files which will be found on demand")
--8<---------------cut here---------------end--------------->8---

Bye,
Tassilo

Re: My read-buffer-function doesn't work when called by switch-to-buffer

[Michael Heerdegen]

Hi Tassilo,

your approach looks reasonable,

>     (let ((minibuffer-setup-hook nil))

but are you sure you always want to nullify `minibuffer-setup-hook'?
There might still be something in this hook that you want/need.

Michael.

Re: My read-buffer-function doesn't work when called by switch-to-buffer

[Tassilo Horn]

Michael Heerdegen <michael_heerdegen@web.de> writes:

Hi Michael,

> your approach looks reasonable,

Thanks.

>>     (let ((minibuffer-setup-hook nil))
>
> but are you sure you always want to nullify `minibuffer-setup-hook'?
> There might still be something in this hook that you want/need.

I replaced my previous "append my own minibuffer-setup-hook function
which sets minibuffer-completion-table to what I want" to this simpler
one and don't feel that I'm missing something.  But you are right, there
could be other things which I'm suppressing now.  Will have a look.

Bye,
Tassilo