bug#45234: Annotation function vs. Affixation function

bug#45234: Annotation function vs. Affixation function

[clemera]

On current master the results of the new annotation function are passed 
to the affixation function in minibuffer-completion-help:

     (when ann-fun
       (setq completions
             (mapcar (lambda (s)
                       (let ((ann (funcall ann-fun s)))
                         (if ann (list s ann) s)))
                     completions)))     (when aff-fun
       (setq completions
             (funcall aff-fun completions)))

If the annotation function returns a result it's packed into a list
which should be an unexpected format for the affixation function?
Maybe when both are defined only the affixation function should be used?

bug#45234: Annotation function vs. Affixation function

[Clemens]

 > On current master the results of the new annotation function are passed
to the affixation function in minibuffer-completion-help:

I formulated that wrongly, there is nothing new about the annotation 
function, only the affixation function is.

bug#45234: Annotation function vs. Affixation function

[Juri Linkov]

> On current master the results of the new annotation function are passed to
> the affixation function in minibuffer-completion-help:
>
>     (when ann-fun
>       (setq completions
>             (mapcar (lambda (s)
>                       (let ((ann (funcall ann-fun s)))
>                         (if ann (list s ann) s)))
>                     completions)))
>     (when aff-fun
>       (setq completions
>             (funcall aff-fun completions)))
>
> If the annotation function returns a result it's packed into a list
> which should be an unexpected format for the affixation function?
> Maybe when both are defined only the affixation function should be used?

The current implementation assumes that the affixation function
can be smart enough to recognize that completions were modified
by a previous call of the annotation function.

If you can demonstrate that this is not always the case, then
the implementation could be changed, there is enough time before
the next release, after that it would be harder to change this.

bug#45234: Annotation function vs. Affixation function

[Clemens]

> The current implementation assumes that the affixation function
> can be smart enough to recognize that completions were modified
> by a previous call of the annotation function.

The doc says it receives a list of completions so I think currently  you 
would have to look at the code to figure that out. Maybe we could avoid 
that complication? Currently it seems every affixation would need to 
check if it got a list of strings or a list of items as returned by a 
possibly defined annotation function.

bug#45234: Annotation function vs. Affixation function

[Juri Linkov]

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

> The doc says it receives a list of completions so I think currently  you
> would have to look at the code to figure that out. Maybe we could avoid
> that complication? Currently it seems every affixation would need to check
> if it got a list of strings or a list of items as returned by a possibly
> defined annotation function.

I agree.  So this could be changed like this:


[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: affixation-function-over-annotation-function.patch --]
[-- Type: text/x-diff, Size: 2245 bytes --]

diff --git a/lisp/minibuffer.el b/lisp/minibuffer.el
index 456193d52e..404b1236b1 100644
--- a/lisp/minibuffer.el
+++ b/lisp/minibuffer.el
@@ -123,7 +123,8 @@ completion-metadata
 - `affixation-function': function to prepend/append a prefix/suffix to
    entries.  Takes one argument (COMPLETIONS) and should return a list
    of completions with a list of three elements: completion, its prefix
-   and suffix.
+   and suffix.  This function takes precedence over `annotation-function'
+   when both are provided, so only this function is used.
 - `display-sort-function': function to sort entries in *Completions*.
    Takes one argument (COMPLETIONS) and should return a new list
    of completions.  Can operate destructively.
@@ -1926,6 +1927,8 @@ completion-extra-properties
    completions.  The function must accept one argument, a list of
    completions, and return a list where each element is a list of
    three elements: a completion, a prefix and a suffix.
+   This function takes precedence over `:annotation-function'
+   when both are provided, so only this function is used.
 
 `:exit-function': Function to run after completion is performed.
 
@@ -2056,15 +2059,16 @@ minibuffer-completion-help
                               (if sort-fun
                                   (funcall sort-fun completions)
                                 (sort completions 'string-lessp))))
-                      (when ann-fun
+                      (cond
+                       (aff-fun
+                        (setq completions
+                              (funcall aff-fun completions)))
+                       (ann-fun
                         (setq completions
                               (mapcar (lambda (s)
                                         (let ((ann (funcall ann-fun s)))
                                           (if ann (list s ann) s)))
-                                      completions)))
-                      (when aff-fun
-                        (setq completions
-                              (funcall aff-fun completions)))
+                                      completions))))
 
                       (with-current-buffer standard-output
                         (setq-local completion-base-position

bug#45234: Annotation function vs. Affixation function

[Clemens]

> I agree.  So this could be changed like this:

Looks good to me, too. Thank you!

bug#45234: Annotation function vs. Affixation function

[Juri Linkov]

tags 45234 fixed
close 45234 28.0.50
thanks

>> I agree.  So this could be changed like this:
>
> Looks good to me, too. Thank you!

Thanks for the suggestion.  Now pushed to master.