Progress for bug#59314 for emacs-29

Progress for bug#59314 for emacs-29

[Thomas Fitzsimmons]

[-- Attachment #1: Type: text/plain, Size: 129 bytes --]

Hi,

Alexander posted the attached patch, which helps with bug#59314.  Can I
push it to the emacs-29 branch?

Thank you,
Thomas


[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: 0001-Untangle-EUDC-s-completion-at-point-function-from-me.patch --]
[-- Type: text/x-diff, Size: 4558 bytes --]

From 658b5ff4dad7f392a9d09f28f6c2bb97f5ae93e8 Mon Sep 17 00:00:00 2001
From: Alexander Adolf <alexander.adolf@condition-alpha.com>
Date: Fri, 9 Dec 2022 22:15:42 +0100
Subject: [PATCH] Untangle EUDC's completion-at-point function from
 message-mode

The default completion-styles turned out to not be generous enough to
allow useful email address completion. Hence, this commit sets the
buffer-local value of completion-styles to a better suited value when
performing email completion via eudc-capf-complete, which is also no
longer added to completion-at-point-functions in message-mode.
message-mode has EUDC support already, and that would have been
duplicated functionality. Users are still free to add
eudc-capf-complete to completion-at-point-functions at their own
discretion (for example in mail-mode).

* lisp/net/eudc-capf.el (eudc-capf-complete): Set
completion-styles buffer locally to a more generous value, so that
more candidates can pass the filtering.
(eudc-capf-message-expand-name): Rename to
eudc-capf--message-expand-name to mark it as an internal use
function, and improved the doc string.
* lisp/gnus/message.el (message-mode): No longer add
eudc-capf-complete to the buffer-local value of
completion-at-point-functions.
---
 lisp/gnus/message.el  |  1 -
 lisp/net/eudc-capf.el | 46 ++++++++++++++++++++-----------------------
 2 files changed, 21 insertions(+), 26 deletions(-)

diff --git a/lisp/gnus/message.el b/lisp/gnus/message.el
index e7d11b597b3..6c10a4ae976 100644
--- a/lisp/gnus/message.el
+++ b/lisp/gnus/message.el
@@ -3191,7 +3191,6 @@ message-mode
     (mail-abbrevs-setup))
    ((message-mail-alias-type-p 'ecomplete)
     (ecomplete-setup)))
-  (add-hook 'completion-at-point-functions #'eudc-capf-complete -1 t)
   (add-hook 'completion-at-point-functions #'message-completion-function nil t)
   (unless buffer-file-name
     (message-set-auto-save-file-name))
diff --git a/lisp/net/eudc-capf.el b/lisp/net/eudc-capf.el
index e2bbd5b28b6..c655c14df6d 100644
--- a/lisp/net/eudc-capf.el
+++ b/lisp/net/eudc-capf.el
@@ -101,34 +101,30 @@ eudc-capf-complete
 The return value is either nil when no match is found, or a
 completion table as required for functions listed in
 `completion-at-point-functions'."
-  (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)))
+  (when (and (or eudc-server eudc-server-hotlist)
+             (seq-some #'derived-mode-p eudc-capf-modes)
+             (let ((mail-abbrev-mode-regexp message-email-recipient-header-regexp))
+               (mail-abbrev-in-expansion-header-p)))
+    (setq-local completion-styles '(substring partial-completion))
+    (eudc-capf--message-expand-name)))
 
 ;;;###autoload
-(defun eudc-capf-message-expand-name ()
-  "Email address completion function for `message-completion-alist'.
+(defun eudc-capf--message-expand-name ()
+  "Helper for `eudc-capf-complete'.
 
-When this function is added to `message-completion-alist',
-replacing any existing entry for `message-expand-name' there,
-with an appropriate regular expression such as for example
-`message-email-recipient-header-regexp', then EUDC will be
-queried for email addresses, and the results delivered to
-`completion-at-point'."
-  (if (or eudc-server eudc-server-hotlist)
-      (progn
-        (let* ((beg (save-excursion
-                      (re-search-backward "\\([:,]\\|^\\)[ \t]*")
-                      (match-end 0)))
-               (end (point))
-               (prefix (save-excursion (buffer-substring-no-properties beg end))))
-          (let ((result
-                 (eudc-query-with-words (split-string prefix "[ \t]+") t)))
-            (when result
-              (list beg end
-                    (completion-table-with-cache
-                     (lambda (_) result) t))))))))
+Computes a completion table as required for functions listed in
+`completion-at-point-functions'."
+  (let* ((beg (save-excursion
+                (re-search-backward "\\([:,]\\|^\\)[ \t]*")
+                (match-end 0)))
+         (end (point))
+         (prefix (save-excursion (buffer-substring-no-properties beg end))))
+    (let ((result
+           (eudc-query-with-words (split-string prefix "[ \t]+") t)))
+      (when result
+        (list beg end
+              (completion-table-with-cache
+               (lambda (_) result) t))))))
 
 (provide 'eudc-capf)
 ;;; eudc-capf.el ends here
-- 
2.33.1

Re: Progress for bug#59314 for emacs-29

[Stefan Monnier]

> +  (when (and (or eudc-server eudc-server-hotlist)
> +             (seq-some #'derived-mode-p eudc-capf-modes)
> +             (let ((mail-abbrev-mode-regexp message-email-recipient-header-regexp))
> +               (mail-abbrev-in-expansion-header-p)))
> +    (setq-local completion-styles '(substring partial-completion))
> +    (eudc-capf--message-expand-name)))

Please don't do that.  `completion-styles` is a user-config, we
shouldn't mess with it.  Instead you should use
`completion-category-defaults` (and use a completion table which returns
an appropriately chosen `category`).  Then users can decide to override
your choice with `completion-category-overrides`.


        Stefan

Re: Progress for bug#59314 for emacs-29

[Eli Zaretskii]

> From: Thomas Fitzsimmons <fitzsim@fitzsim.org>
> Cc: Alexander Adolf <alexander.adolf@condition-alpha.com>
> Date: Tue, 20 Dec 2022 21:59:41 -0500
> 
> Alexander posted the attached patch, which helps with bug#59314.  Can I
> push it to the emacs-29 branch?

Is this a bug or just a lack of a nice-to-have feature?  If the
former, since when do we have this bug?

Also, I'd like to reassess this once you have a final version, per
Stefan's comments, which avoids tramping a user option.

Thanks.

Re: Progress for bug#59314 for emacs-29

[Thomas Fitzsimmons]

Eli Zaretskii <eliz@gnu.org> writes:

>> From: Thomas Fitzsimmons <fitzsim@fitzsim.org>
>> Cc: Alexander Adolf <alexander.adolf@condition-alpha.com>
>> Date: Tue, 20 Dec 2022 21:59:41 -0500
>> 
>> Alexander posted the attached patch, which helps with bug#59314.  Can I
>> push it to the emacs-29 branch?
>
> Is this a bug or just a lack of a nice-to-have feature?  If the
> former, since when do we have this bug?
>
> Also, I'd like to reassess this once you have a final version, per
> Stefan's comments, which avoids tramping a user option.

OK.  The issue itself, a regression, is fixed by just this part of the
patch (see long discussion on the bug report):

diff --git a/lisp/gnus/message.el b/lisp/gnus/message.el
index e7d11b597b3..6c10a4ae976 100644
--- a/lisp/gnus/message.el
+++ b/lisp/gnus/message.el
@@ -3191,7 +3191,6 @@ message-mode
     (mail-abbrevs-setup))
    ((message-mail-alias-type-p 'ecomplete)
     (ecomplete-setup)))
-  (add-hook 'completion-at-point-functions #'eudc-capf-complete -1 t)
   (add-hook 'completion-at-point-functions #'message-completion-function nil t)
   (unless buffer-file-name
     (message-set-auto-save-file-name))

Can I push that?

Thanks,
Thomas

Re: Progress for bug#59314 for emacs-29

[Eli Zaretskii]

> From: Thomas Fitzsimmons <fitzsim@fitzsim.org>
> Cc: emacs-devel@gnu.org,  alexander.adolf@condition-alpha.com
> Date: Wed, 21 Dec 2022 07:54:56 -0500
> 
> Eli Zaretskii <eliz@gnu.org> writes:
> 
> >> From: Thomas Fitzsimmons <fitzsim@fitzsim.org>
> >> Cc: Alexander Adolf <alexander.adolf@condition-alpha.com>
> >> Date: Tue, 20 Dec 2022 21:59:41 -0500
> >> 
> >> Alexander posted the attached patch, which helps with bug#59314.  Can I
> >> push it to the emacs-29 branch?
> >
> > Is this a bug or just a lack of a nice-to-have feature?  If the
> > former, since when do we have this bug?
> >
> > Also, I'd like to reassess this once you have a final version, per
> > Stefan's comments, which avoids tramping a user option.
> 
> OK.  The issue itself, a regression, is fixed by just this part of the
> patch (see long discussion on the bug report):
> 
> diff --git a/lisp/gnus/message.el b/lisp/gnus/message.el
> index e7d11b597b3..6c10a4ae976 100644
> --- a/lisp/gnus/message.el
> +++ b/lisp/gnus/message.el
> @@ -3191,7 +3191,6 @@ message-mode
>      (mail-abbrevs-setup))
>     ((message-mail-alias-type-p 'ecomplete)
>      (ecomplete-setup)))
> -  (add-hook 'completion-at-point-functions #'eudc-capf-complete -1 t)
>    (add-hook 'completion-at-point-functions #'message-completion-function nil t)
>    (unless buffer-file-name
>      (message-set-auto-save-file-name))
> 
> Can I push that?

Yes, thanks.

Re: Progress for bug#59314 for emacs-29

[Alexander Adolf]

Thomas Fitzsimmons <fitzsim@fitzsim.org> writes:

> [...]
> Alexander posted the attached patch, which helps with bug#59314.  Can I
> push it to the emacs-29 branch?
> [...]

Looks all good for pushing to me.


Many thanks and cheers,

  --alexander

Re: Progress for bug#59314 for emacs-29

[Alexander Adolf]

Eli Zaretskii <eliz@gnu.org> writes:

>> From: Thomas Fitzsimmons <fitzsim@fitzsim.org>
>> Cc: emacs-devel@gnu.org,  alexander.adolf@condition-alpha.com
>> Date: Wed, 21 Dec 2022 07:54:56 -0500
>> 
>> [...] 
>> OK.  The issue itself, a regression, is fixed by just this part of the
>> patch (see long discussion on the bug report):
>> 
>> diff --git a/lisp/gnus/message.el b/lisp/gnus/message.el
>> index e7d11b597b3..6c10a4ae976 100644
>> --- a/lisp/gnus/message.el
>> +++ b/lisp/gnus/message.el
>> @@ -3191,7 +3191,6 @@ message-mode
>>      (mail-abbrevs-setup))
>>     ((message-mail-alias-type-p 'ecomplete)
>>      (ecomplete-setup)))
>> -  (add-hook 'completion-at-point-functions #'eudc-capf-complete -1 t)
>>    (add-hook 'completion-at-point-functions #'message-completion-function nil t)
>>    (unless buffer-file-name
>>      (message-set-auto-save-file-name))
>> 
>> Can I push that?
>
> Yes, thanks.

Works for me, too.

  --alexander

Re: Progress for bug#59314 for emacs-29

[Alexander Adolf]

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

>> +  (when (and (or eudc-server eudc-server-hotlist)
>> +             (seq-some #'derived-mode-p eudc-capf-modes)
>> +             (let ((mail-abbrev-mode-regexp message-email-recipient-header-regexp))
>> +               (mail-abbrev-in-expansion-header-p)))
>> +    (setq-local completion-styles '(substring partial-completion))
>> +    (eudc-capf--message-expand-name)))
>
> Please don't do that.  `completion-styles` is a user-config, we
> shouldn't mess with it.  Instead you should use
> `completion-category-defaults` (and use a completion table which returns
> an appropriately chosen `category`).  Then users can decide to override
> your choice with `completion-category-overrides`.
> [...]

This was designed as an intermediary step, enabling "early access" to
EUDC in completion-at-point.

The real McCoy will be the changes to message.el I'm working on, and of
which I had circulated a draft earlier. That will have the completion
function responding to the 'metadata request as you describe, of course.
For the purposes of message-mode (which is the only mode I'm using for
email), that's all that'll be needed. eudc-capf.el would be for other
email composing modes (e.g. mail-mode) only.

In this light, instead of investing the work to implement something in
eudc-capf.el now, of which I know that I will have to move from there to
message.el anyways, I think it seems more advisable to drop the
intermediary step altogether, i.e. to drop eudc-capf.el, and put the new
feature off until my updates to message.el will be in a better shape.


Many thanks and looking forward to your thoughts,

  --alexander