Re: Add integration to gnus-search library, if present

[Eric Abrahamsen]

"Roland Winkler" <winkler@gnu.org> writes:

> On Wed Nov 11 2020 Eric Abrahamsen wrote:
>> Okay, here's a complete and tested patch. You can try it out by applying
>> the patch and then evaluating the below, which is a simple emulation of
>> how an external package might make use of this completion table.
>
> Thanks!  Do I understand correctly that the new dynamic completion table
> has really nothing specific regarding completion-at-point, but it
> can possibly likewise be used in any other context when minibuffer
> completion based on BBDB is desired?  Then, I suggest to give it a
> more generic name such as
>
>   bbdb-dynamic-completion-table  instead of  bbdb-completion-at-point-table
>
> and
>
>   bbdb-dynamic-completion  instead of  bbdb-completion-at-point

Yes, that's correct. The same `bbdb-completion-at-point-table' can be
fed to `completing-read' as the COLLECTION argument and it will behave
as expected. It would definitely make sense to rename accordingly.

[...]

>> Also, bbdb-dynamic-completion-table should get a docstring,
>> explaining that this completion table can be customized by (let-)
>> binding bbdb-completion-list. (The docstring of
>> bbdb-completion-list needs to be updated, too!)
> 
> One more thought regarding bbdb-completion-list. As its current
> docstring says, this variable was specifically intended for
> bbdb-complete-mail.  So for using the corresponding machinery in a
> very different context like completion-at-point, it seems rather
> likely to me that one would like to give it a different value.
> Would it make sense to let-bind bbdb-completion-list inside
> bbdb-dynamic-completion to the value of some other user variable
> that facilitates customization?  (Of course, the default value of
> the new variable could be the value of bbdb-completion-list.)

Yes, originally I thought there were two potential improvements that we
might want to make right away: one would be providing a separate
customization option that governed the building of the table (as you
describe above), the other would be caching the built strings.

I think it would make sense to define a new option (but what to call it?
`bbdb-dynamic-completion-list'?). If `nil', use the value of
`bbdb-completion-list'. If `t' or a list of symbols, behave as
`bbdb-completion-list'. Then `bbdb-completion-at-point' (to be renamed
to `bbdb-dynamic-completion') would look like:

(defun bbdb-completion-at-point (_str)
  "Return a list of strings for completion from the database.
Meant to be used as a completion table in a `completion-at-point'
or `completing-read' context."
  (let ((comp-list (or bbdb-dynamic-completion-list
                       bbdb-completion-list))
        strings)
    (if (consp comp-list)
        (maphash (lambda (str records)
                   (dolist (record records)
                     (when (catch 'bbdb-hash-ok
                             (bbdb-hash-p str record comp-list))
                       (push str strings))))
                 bbdb-hashtable)
      (setq strings (hash-table-keys bbdb-hashtable)))
    strings))

I haven't tested that, but that's the basic idea.

The other issue is caching the list of strings. This is premature
optimization and should probably be ignored, but it's also true that
minibuffer completion can call these functions many times during the
course of simple completion: hitting TAB once might result in four or
five calls to the function.

If we wanted to, we could have a `bbdb-dynamic-completion--cache'
variable that held the built list of strings. Then maybe a function on
`bbdb-change-hook' could set this to nil, forcing a rebuild the next
time completion was attempted. Anyway, just a thought.

Eric