Re: Thoughts on Refactoring In-Buffer Completion In message.el

[Alexander Adolf]

Hello Stefan,

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

>>    ┌────
>>    │ (defcustom message-completion-alist
>>    │   `((,message-newgroups-header-regexp
>>    │      . '(:capf-style 'newsgroup
>>    │          :capf-funs  '(gnus-capf-complete)))    ;; hypothetical
>>    │     (,message-email-recipient-header-regexp
>>    │       . '(:capf-style 'email
>>    │           :capf-funs  '(eudc-capf-complete))))  ;; exists
>>    │   "docstring"
>>    │   )
>>    └────
>
> Sounds fine, tho "capf-style" sounds wrong: these are the names of
> categories, not a styles.

These names were just for illustration, so happy to use better fitting
names.

Perhaps `completion-style-category`? A shorter version?

> Also `capf-funs` sounds wrong: [...]

They are either tables, or functions that return tables. As of today,
the entries in message-completion-alist are functions.

> Currently we have `message-completion-alist` which decides on
> a line-by-line basis what kind of completion we're doing, and then we
> have a specialized function that decides what's the BEG..END to use and
> which backend to use.
>
> Maybe `message-completion-alist` should be beefed up so that it cuts
> this middle man (specialized function): i.e. provide not just
> a line-based regexp to decide which specialized function to use, but
> also provide some way to specify the BEG..END and the backend*S*.

Good point, and agreed that determining (beg . end) should also be done
in message.Eli.

> Maybe have it be a list of (FUNCTION CATEGORY . BACKENDS) ?
> where FUNCTION should return (BEG . END) if this completion applies?

In some situations this will be called for every keystroke. Would the
extra overhead of a function call seem acceptable for this?

OTOH, the code of each of those functions would probably look extremely
similar. The format of header lines where specific completion is to be
attempted always seems to be:

    header-label ':' record | record-list

(I'm thinking of To/Cc/etc. and Newsgroups/Followup-To/etc.)

Thus, the only parameters for such functions would seem to be a regular
expression to match the header-label, and the character (or regex?) that
is used as the record separator. So we might just as well put the
header-label regex, and the record separator char in
`message-completion-alist`?

>> 2) Refactor ecomplete, mailabbrev, and bbdb stuff out of message.el as
>>    much as possible.
>
> AFAIK this is already done for `ecomplete`.
> [...]

Searching for `ecomplete` in message.el (tip of master as of this
writing):

message.el
1390:The default is `abbrev', which uses mailabbrev.  `ecomplete' uses
1396:            (const :tag "Use ecomplete" ecomplete)
1400:  "List of `self-insert-command's used to trigger ecomplete.
1402:header, ecomplete will suggest the candidates of recipients (see also
1937:(defvar message-inhibit-ecomplete nil)
3097:  (when (and (message-mail-alias-type-p 'ecomplete)
3181:   ((message-mail-alias-type-p 'ecomplete)
3182:    (ecomplete-setup)))
4445:      ;; Do ecomplete address snarfing.
4446:      (when (and (message-mail-alias-type-p 'ecomplete)
4447:            (not message-inhibit-ecomplete))
4448:   (message-put-addresses-in-ecomplete))
8021:       (message-inhibit-ecomplete t)
8344:`ecomplete', `message-self-insert-commands' should probably be
8414:      (ecomplete-setup)
8420:                   (when (and (bound-and-true-p ecomplete-database)
8421:                              (fboundp 'ecomplete-completion-table))
8423:                                           (ecomplete-completion-table 
'mail)
8628:(declare-function ecomplete-add-item "ecomplete" (type key text))
8629:(declare-function ecomplete-save "ecomplete" ())
8631:(defun message-put-addresses-in-ecomplete ()
8632:  (require 'ecomplete)
8640:   (ecomplete-add-item 'mail (car (mail-header-parse-address string))
8642:  (ecomplete-save))
8644:(autoload 'ecomplete-display-matches "ecomplete")
8666:               (ecomplete-display-matches 'mail word choose))))
8671:(defun message-ecomplete-capf ()
8674:  (when (and (bound-and-true-p ecomplete-database)
8675:             (fboundp 'ecomplete-completion-table)
8683:      `(,start ,end ,(ecomplete-completion-table 'mail)))))

That's 40 hits, and looking at the matching lines, my first impression
would not frankly be that ecomplete integration has been reduced to a
bare minimum.


Looking forward to your thoughts,

  --alexander