bug#61283: 29.0.60; pcomplete-completions-at-point loses text properties, breaking pcomplete-from-help annotations

[Daniel Mendler]

On 5/17/23 23:20, Stefan Monnier wrote:
> [ BTW, I just noticed that Augusto (original author of that code) was
>   not in the Cc.  Augusto, you may want to go and read the other
>   messages in bug#61283.  ]
> 
>>> This is obviously a pretty invasive change and would require changing
>>> a number of internal assumptions in Pcomplete (though I don't think it would
>>> break the public API). If this sounds even remotely like a reasonable way to
>>> go forward, I'll try to clean up my patch a bit more and post it for
>>> feedback; it's currently a bit of a mess, I'm afraid...
>>
>> It sounds like a sane approach, so yes, I'd be happy to look at
>> the patch.
>
> Rather than fetch the info directly from `cand`, we should maybe
> lookup the info from elsewhere (maybe `completions`?).  Then again, I'm
> not sure how much `cand` can differ from the matching entry in
> `completions`: it's affected by `completion-table-subvert` and
> `completion-table-with-quoting` so in general it can be non-trivial to
> figure out which entry of `completions` corresponds to the `cand` we
> got :-(

Yes, this occurred to me too. I assume relying on the original string
should work since quoting should not change the string. Options like
"--color=" are not affected by quoting.

But this means that the following simple patch to `completion--twq-all'
also fixes the issue. It ensures that the original string is preserved
in the common case where string quoting was ineffective. This approach
may be better than refactoring pcomplete.el. The patch may even be safe
enough for emacs-29.

From: Daniel Mendler <mail@daniel-mendler.de>
Date: Wed, 17 May 2023 10:23:25 +0200
Subject: [PATCH] Preserve pcomplete annotation and help (bug#61283)

lisp/minibuffer.el (completion--twq-all): Preserve string properties
in if quoted string equals original string.
---
 lisp/minibuffer.el | 5 +++++
 1 file changed, 5 insertions(+)

diff --git a/lisp/minibuffer.el b/lisp/minibuffer.el
index 747c9443af..804c5d38ca 100644
--- a/lisp/minibuffer.el
+++ b/lisp/minibuffer.el
@@ -690,6 +690,11 @@ completion--twq-all
                                              'completions-common-part)
                                qprefix))))
                         (qcompletion (concat qprefix qnew)))
+                   ;; Preserve original string with properties if
+                   ;; quoting did not change it.  See bug#61238, which
+                   ;; needs string property preservation.
+                   (when (equal qcompletion completion)
+                     (setq qcompletion completion))
                   ;; FIXME: Similarly here, Cygwin's mapping trips this
                   ;; assertion.
                    ;;(cl-assert