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

[Daniel Mendler]

On 5/18/23 07:39, Eli Zaretskii wrote:
>> Date: Thu, 18 May 2023 04:01:46 +0200
>> Cc: Augusto Stoffel <arstoffel@gmail.com>, Eli Zaretskii <eliz@gnu.org>,
>>  61283@debbugs.gnu.org, liuhui1610@gmail.com
>> From: Daniel Mendler <mail@daniel-mendler.de>
>>
>> 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.
> 
> Why do you think it will be safe for emacs-29?
> 
> And why do we have to fix this in Emacs 29.1?  This only affects
> Eshell, AFAIU, is that true?

The patch I've sent changes `completion--twq-all' in such a way that the
original candidate string including its properties is preserved if the
string content was not changed at all by the quoting mechanism.

This is a localized change which adjusts only `completion--twq-all', the
problematic function, which causes this issue and avoids refactorings at
other places. In my test this modification fixes the annotation issue in
Eshell. Annotations are also shown in Shell.

Maybe Stefan can take a look at tell if he considers the approach safe?
Usually candidates passed through `completion--twq-all' are not
propertized, since they are file names. In cases where candidates are
still propertized as with Pcomplete here, property preservation is
likely preserved.

Daniel

------

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