Re: [PATCH] EUDC email addresses via completion-at-point in message-mode

[Filipp Gunbin]

I've only skimmed through previous discussion, but will point out a few
things in the patch.

Thanks.

On 26/04/2022 16:39 +0200, Alexander Adolf wrote:

> +;;; Usage:
> +
> +;;    (require 'eudc-capf)
> +;;    (add-hook 'completion-at-point-functions #'eudc-capf-complete -1 t)

As you're showing add-hook with LOCAL t, perhaps you should also show
some context, to avoid users pasting this directly into their .emacs.

> +(defconst eudc-capf-modes '(message-mode) "List of modes in which email \
> +address completion is to be attempted.")

Put docstring on its own line.

> +  (if (and (seq-some #'derived-mode-p eudc-capf-modes)
> +           (let ((mail-abbrev-mode-regexp 
> message-email-recipient-header-regexp))
> +             (mail-abbrev-in-expansion-header-p)))
> +      (eudc-capf-message-expand-name)

You don't need to specify else-branch for an if.  Just omit it.  (same
below)

> +`completion-at-point'."
> +  (if (or (and (boundp 'eudc-server) eudc-server)
> +          (and (boundp 'eudc-server-hotlist) eudc-server-hotlist))

You're requiring eudc, why should these be unbound?

> +      (progn
> +        (setq-local completion-styles '(substring partial-completion))
> +        (let* ((beg (save-excursion
> +                      (if (re-search-backward "\\([:,]\\|^\\)[ \t]*"
> +                                              (point-at-bol) 'move)

t instead of 'move?

I also don't like the side-effect of setting completion-styles for the
user, although only locally.  Can it be done in some other way?