Re: master 431f8ff1e38: * lisp/imenu.el: Support more values for imenu-flatten (bug#70846)

[Juri Linkov <juri@linkov.net>]

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

>> because when `completing-read-default` calls `read-from-minibuffer`,
>> it uses the value of `minibuffer-allow-text-properties` from
>> the original buffer:
>>
>>   val = read_minibuf (keymap, initial_contents, prompt,
>> 		      !NILP (read),
>> 		      histvar, histpos, default_value,
>> 		      minibuffer_allow_text_properties,
>> 		      !NILP (inherit_input_method));
>
> Damn!  This is too bad: a buffer-local setting of
> `minibuffer_allow_text_properties` can basically never be used then,
> because it's read from the wrong buffer.
>
>> Then `read_minibuf` uses its argument `allow_props`:
>>
>>   if (allow_props)
>>     val = Fminibuffer_contents ();
>>   else
>>     val = Fminibuffer_contents_no_properties ();
>>
>> Maybe it could use `minibuffer-allow-text-properties` directly here?
>
> Indeed: since a buffer-local setting can't work, we know that all
> callers must use a plain let-binding so the binding will be active
> during the whole minibuffer session, so we may as well read it at the
> end (in the minibuffer) rather than at the beginning (in the
> `minibuffer--original-buffer`).

Ok, then here is a complete patch with documentation updates
where many documentation changes are fixing the documentation
to describe the current behavior existed even before applying
these code changes.

> PS: Git shows that `minibuffer-allow-text-properties` was introduced
> eons ago (and basically never touched since then, even the docstring is
> mostly unchanged).  And Grep shows it's not used very often (and
> several of those uses are around `completing-read`).
> Funnily enough, one of the few uses in our tree binds it to nil,
> I wonder why that was needed.

Probably the nil let-binding was intended to negate the effect
of a non-nil let-binding higher in the stack of recursive minibuffers.
So this is another reason to prefer buffer-local settings.


[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: minibuffer_allow_text_properties.patch --]
[-- Type: text/x-diff, Size: 8774 bytes --]

diff --git a/etc/NEWS b/etc/NEWS
index 4e52d4dccb2..cefbc0eb938 100644
--- a/etc/NEWS
+++ b/etc/NEWS
@@ -2033,6 +2033,17 @@ UTF-8 byte sequence, and the optional parameter MULTIBYTE of
 'dbus-string-to-byte-array' should be a regular Lisp string, not a
 unibyte string.
 
++++
+** 'minibuffer-allow-text-properties' now can be set buffer-local.
+'read-from-minibuffer' and functions that use it can take the
+buffer-local value from the minibuffer.
+
++++
+** 'minibuffer-allow-text-properties' also affects completions.
+When it has a non-nil value, then completion functions like
+'completing-read' don't discard text properties from the returned
+completion candidate.
+
 \f
 * Lisp Changes in Emacs 30.1
 
diff --git a/doc/lispref/minibuf.texi b/doc/lispref/minibuf.texi
index 8f2d0d702f9..af5552851e2 100644
--- a/doc/lispref/minibuf.texi
+++ b/doc/lispref/minibuf.texi
@@ -185,7 +185,8 @@ Text from Minibuffer
 History}.
 
 If the variable @code{minibuffer-allow-text-properties} is
-non-@code{nil}, then the string that is returned includes whatever text
+non-@code{nil}, either let-bound or buffer-local in the minibuffer,
+then the string that is returned includes whatever text
 properties were present in the minibuffer.  Otherwise all the text
 properties are stripped when the value is returned.  (By default this
 variable is @code{nil}.)
@@ -352,28 +353,27 @@ Text from Minibuffer
 
 @defvar minibuffer-allow-text-properties
 If this variable is @code{nil}, the default, then
-@code{read-from-minibuffer} and @code{read-string} strip all text
+@code{read-from-minibuffer}, @code{read-string}, and all
+functions that do minibuffer input with completion strip all text
 properties from the minibuffer input before returning it.  However,
-@code{read-no-blanks-input} (see below), as well as
 @code{read-minibuffer} and related functions (@pxref{Object from
-Minibuffer,, Reading Lisp Objects With the Minibuffer}), and all
-functions that do minibuffer input with completion, remove the
+Minibuffer,, Reading Lisp Objects With the Minibuffer}), remove the
 @code{face} property unconditionally, regardless of the value of this
 variable.
 
-If this variable is non-@code{nil}, most text properties on strings
-from the completion table are preserved---but only on the part of the
-strings that were completed.
+If this variable is non-@code{nil}, either let-bound or buffer-local in
+the minibuffer, most text properties on strings from the completion
+table are preserved---but only on the part of the strings that were
+completed.
 
 @lisp
 (let ((minibuffer-allow-text-properties t))
   (completing-read "String: " (list (propertize "foobar" 'data 'zot))))
-=> #("foobar" 3 6 (data zot))
+=> #("foobar" 0 6 (data zot))
 @end lisp
 
 In this example, the user typed @samp{foo} and then hit the @kbd{TAB}
-key, so the text properties are only preserved on the last three
-characters.
+key, so the text properties are preserved on all characters.
 @end defvar
 
 @vindex minibuffer-mode-map
@@ -433,18 +433,6 @@ Text from Minibuffer
 keymap as the @var{keymap} argument for that function.  Since the keymap
 @code{minibuffer-local-ns-map} does not rebind @kbd{C-q}, it @emph{is}
 possible to put a space into the string, by quoting it.
-
-This function discards text properties, regardless of the value of
-@code{minibuffer-allow-text-properties}.
-
-@smallexample
-@group
-(read-no-blanks-input @var{prompt} @var{initial})
-@equiv{}
-(let (minibuffer-allow-text-properties)
-  (read-from-minibuffer @var{prompt} @var{initial} minibuffer-local-ns-map))
-@end group
-@end smallexample
 @end defun
 
 @c Slightly unfortunate name, suggesting it might be related to the
diff --git a/src/minibuf.c b/src/minibuf.c
index 9c1c86680d4..f2a76086361 100644
--- a/src/minibuf.c
+++ b/src/minibuf.c
@@ -563,7 +563,8 @@ DEFUN ("minibuffer-contents-no-properties", Fminibuffer_contents_no_properties,
 
    DEFALT specifies the default value for the sake of history commands.
 
-   If ALLOW_PROPS, do not throw away text properties.
+   If ALLOW_PROPS or `minibuffer-allow-text-properties' is non-nil,
+   do not throw away text properties.
 
    if INHERIT_INPUT_METHOD, the minibuffer inherits the
    current input method.  */
@@ -928,7 +929,7 @@ read_minibuf (Lisp_Object map, Lisp_Object initial, Lisp_Object prompt,
 
   /* Make minibuffer contents into a string.  */
   Fset_buffer (minibuffer);
-  if (allow_props)
+  if (allow_props || minibuffer_allow_text_properties)
     val = Fminibuffer_contents ();
   else
     val = Fminibuffer_contents_no_properties ();
@@ -1321,7 +1322,8 @@ DEFUN ("read-from-minibuffer", Fread_from_minibuffer,
 Seventh arg INHERIT-INPUT-METHOD, if non-nil, means the minibuffer inherits
  the current input method and the setting of `enable-multibyte-characters'.
 
-If the variable `minibuffer-allow-text-properties' is non-nil,
+If the variable `minibuffer-allow-text-properties' is non-nil
+ (either let-bound or buffer-local in the minibuffer),
  then the string which is returned includes whatever text properties
  were present in the minibuffer.  Otherwise the value has no text properties.
 
@@ -2464,9 +2466,10 @@ syms_of_minibuf (void)
   DEFVAR_BOOL ("minibuffer-allow-text-properties",
 	       minibuffer_allow_text_properties,
 	       doc: /* Non-nil means `read-from-minibuffer' should not discard text properties.
-This also affects `read-string', but it does not affect `read-minibuffer',
-`read-no-blanks-input', or any of the functions that do minibuffer input
-with completion; they always discard text properties.  */);
+The value could be let-bound or buffer-local in the minibuffer.
+This also affects `read-string', or any of the functions that do
+minibuffer input with completion, but it does not affect `read-minibuffer'
+that always discards text properties.  */);
   minibuffer_allow_text_properties = 0;
 
   DEFVAR_LISP ("minibuffer-prompt-properties", Vminibuffer_prompt_properties,
diff --git a/lisp/simple.el b/lisp/simple.el
index bcd26da13ed..8fbfa683e97 100644
--- a/lisp/simple.el
+++ b/lisp/simple.el
@@ -10127,9 +10127,9 @@ choose-completion
           (completion-no-auto-exit (if no-exit t completion-no-auto-exit))
           (choice
            (if choose-completion-deselect-if-after
-               (if-let ((str (get-text-property (posn-point (event-start event)) 'completion--string)))
-                   (substring-no-properties str)
-                 (error "No completion here"))
+               (or (get-text-property (posn-point (event-start event))
+                                      'completion--string)
+                   (error "No completion here"))
            (save-excursion
              (goto-char (posn-point (event-start event)))
              (let (beg)
@@ -10144,8 +10144,7 @@ choose-completion
                (setq beg (or (previous-single-property-change
                               beg 'completion--string)
                              beg))
-               (substring-no-properties
-                (get-text-property beg 'completion--string)))))))
+               (get-text-property beg 'completion--string))))))
 
       (unless (buffer-live-p buffer)
         (error "Destination buffer is dead"))
diff --git a/lisp/imenu.el b/lisp/imenu.el
index ea097f5da3a..93d84106ec1 100644
--- a/lisp/imenu.el
+++ b/lisp/imenu.el
@@ -752,6 +752,7 @@ imenu--completion-buffer
     ;; Display the completion buffer.
     (minibuffer-with-setup-hook
         (lambda ()
+          (setq-local minibuffer-allow-text-properties t)
           (setq-local completion-extra-properties
                       `( :category imenu
                          ,@(when (eq imenu-flatten 'annotation)
@@ -765,10 +766,12 @@ imenu--completion-buffer
 				  nil t nil 'imenu--history-list name)))
 
     (when (stringp name)
-      (setq choice (assoc name prepared-index-alist))
-      (if (imenu--subalist-p choice)
-	  (imenu--completion-buffer (cdr choice) prompt)
-	choice))))
+      (or (get-text-property 0 'imenu-choice name)
+	  (progn
+	    (setq choice (assoc name prepared-index-alist))
+	    (if (imenu--subalist-p choice)
+		(imenu--completion-buffer (cdr choice) prompt)
+	      choice))))))
 
 (defun imenu--mouse-menu (index-alist event &optional title)
   "Let the user select from a buffer index from a mouse menu.
@@ -798,7 +801,9 @@ imenu--flatten-index-alist
 	    (new-prefix (and concat-names
 			     (if prefix
 				 (concat prefix imenu-level-separator name)
-			       name))))
+			       (if (eq imenu-flatten 'annotation)
+                                   (propertize name 'imenu-choice item)
+                                 name)))))
        (cond
 	((not (imenu--subalist-p item))
 	 (list (cons (if (and (eq imenu-flatten 'annotation) prefix)