bug#59314: 29.0.50; EUDC and message-mode header completion

[Alexander Adolf <alexander.adolf@condition-alpha.com>]

Hello Thomas,

Thomas Fitzsimmons <fitzsim@fitzsim.org> writes:

> [...]
>>> This may be a clue; while I was experimenting, I found that this:
>>>
>>> @@ -8432,7 +8438,7 @@ message--name-table
>>>                            (all-completions string
>>>                                             (ecomplete-completion-table 'mail)
>>>                                             pred)))))
>>> -	   (if action candidates (try-completion string candidates))))))))
>>> +	   (if action (cdr candidates) (try-completion string candidates))))))))
>>> [...]
>>
>> Interesting; thanks for sharing this. Effectively, you're dropping the
>> first entry from the `candidates` list before returning it. What does
>> your `candidates` list contain at that point (before removal)?
>
> ("Emacs ERT3 <emacs-ert-test-3@bbdb.gnu.org>"
>  "Emacs ERT4 <emacs-ert-test-4@bbdb.gnu.org>")

Looking at the code in `completion-pcm--merge-try` (which is part of the
partial-completion style, and gets called from the basic, substring, and
flex completion styles), reducing it to a single candidate may make a
difference; the function begins like this:

---------------------------- Begin Quote -----------------------------
(defun completion-pcm--merge-try (pattern all prefix suffix)
  (cond
   ((not (consp all)) all)
   ((and (not (consp (cdr all)))        ;Only one completion.
         ;; Ignore completion-ignore-case here.
         (equal (completion-pcm--pattern->string pattern) (car all)))
    t)
   (t
[...]
----------------------------- End Quote ------------------------------

I.e. the behaviour for a single candidate (second condition case) is
different from when there's more than one candidate (third, and default
condition case). This could explain why you change seemed to improve
things.

> [...]
> OK, interesting; maybe we've found a bug in the completion engine
> itself.

That may well be the case.

And as the completion styles generously call each other, any change in
that area will require extra care.

> [...]
> diff --git a/lisp/minibuffer.el b/lisp/minibuffer.el
> index 5faa3c8d4e8..455135628c8 100644
> --- a/lisp/minibuffer.el
> +++ b/lisp/minibuffer.el
> @@ -1191,7 +1191,7 @@ completion--nth-completion
>                                 (error "Invalid completion style %s" style))
>                             string table pred point)))
>                 (and probe (cons probe style))))
> -           (completion--styles md)))
> +           (let ((styles (completion--styles md))) (message "STYLES: %S" styles) styles)))
>
> When I press TAB, it prints:
>
> STYLES: (substring partial-completion basic emacs22)
>
> even though globally, completion-styles is set to its default,
> (basic partial-completion emacs22)

Yes, the the "completion-category-defaults" logic does work, and what
you're seeing is that `completion-category-defaults` (or *-overrides)
when non-nil gets pre-pended to the buffer-local value of
`completion-styles`.


Cheers,

  --alexander