bug#28774: Master, emacs-26: Can't add text property to built-in function name.

bug#28774: Master, emacs-26: Can't add text property to built-in function name.

[Ilya Khaprov]

Hi

After commit :3db388b0bf the following stopped working:

(global-set-key
 "\M-x"
 (lambda ()
   (interactive)
   (call-interactively
    (intern
     (ido-completing-read
      "M-x "
      (all-completions "" obarray 'commandp))))))

This package no longer works too: https://github.com/DarwinAwardWinner/ido-completing-read-plus

Example error message:
Error in post-command-hook (ido-exhibit): (error "Attempt to modify read-only object" "rename-buffer")

On the surface it looks like if I try to complete function defined in C (i.e, built-in),
I get this error because the symbol/name is read only.

Call chain like this :
ido-completions
put-text-property
add_text_properties_1
validate_inerval_range
create_root_interval
CHECK_IMPURE
pure_write_error

I still reproduce it on

emacs-26 - 5d51403ceb
master      - 6abff55b55

Temporary fixed with ido-name override:

(defun ido-name (item)
  ;; Return file name for current item, whether in a normal list
  ;; or a merged work directory list.
  (concat (if (consp item) (car item) item)))


Thanks,
Ilya

bug#28774: Master, emacs-26: Can't add text property to built-in function name.

[Noam Postavsky]

Ilya Khaprov <ilya.khaprov@publitechs.com> writes:
>
> After commit :3db388b0bf the following stopped working:
>
> (global-set-key
>  "\M-x"
>  (lambda ()
>    (interactive)
>    (call-interactively
>     (intern
>      (ido-completing-read
>       "M-x "
>       (all-completions "" obarray 'commandp))))))

It seems ido-completions relies on (format "%s" str) to return a copy of
str.  This fixes it:

--- i/lisp/ido.el
+++ w/lisp/ido.el
@@ -4701,7 +4701,7 @@ ido-completions
     (if (and ido-use-faces comps)
 	(let* ((fn (ido-name (car comps)))
 	       (ln (length fn)))
-	  (setq first (format "%s" fn))
+	  (setq first (copy-sequence fn))
 	  (put-text-property 0 ln 'face
 			     (if (= (length comps) 1)
                                  (if ido-incomplete-regexp

bug#28774: Master, emacs-26: Can't add text property to built-in function name.

[Drew Adams]

> It seems ido-completions relies on (format "%s" str) to return a copy of
> str.  This fixes it:

If `format' no longer always copies STR in this context then
its doc should be updated, I think.

This:

 Format a string out of a format-string and arguments.
 The first argument is a format control string.
 The other arguments are substituted into it to make the
 result, a string.

suggests that it creates a new string, or at most reuses
the format string (e.g., "%s"), modifying it by substituting
STR for %s in it.  If in fact it can sometimes simply return
STR then this should be mentioned explicitly, to avoid confusion.

(And why was this change made?  Was it just to save a string
copy?)

This is an incompatible Lisp change, if it is new.  In that
case, it should be documented as such.

bug#28774: Master, emacs-26: Can't add text property to built-in function name.

[Noam Postavsky]

On Tue, Oct 10, 2017 at 10:01 AM, Drew Adams <drew.adams@oracle.com> wrote:
>> It seems ido-completions relies on (format "%s" str) to return a copy of
>> str.  This fixes it:
>
> If `format' no longer always copies STR in this context then
> its doc should be updated, I think.

It has been, see https://debbugs.gnu.org/cgi/bugreport.cgi?bug=28625

bug#28774: Master, emacs-26: Can't add text property to built-in function name.

[Noam Postavsky]

retitle 28774 [ido] Can't add text property to built-in function name.
found 28774 26.0.90
tags 28774 fixed
close 28774
quit

Noam Postavsky <npostavs@users.sourceforge.net> writes:

> -	  (setq first (format "%s" fn))
> +	  (setq first (copy-sequence fn))

Pushed to emacs-26.

[1: b78332c3c6]: 2017-10-11 20:53:22 -0400
  Don't use (format "%s" ...) for string copying (Bug#28774)
  http://git.savannah.gnu.org/cgit/emacs.git/commit/?id=b78332c3c646be12d252a637ce0fc949919a840b