bug#55205: 28.1.50; completion--replace illegally mutates completion candidates

[Stefan Monnier]

Lars Ingebrigtsen [2022-05-02 10:11:35] wrote:
> Stefan Monnier <monnier@iro.umontreal.ca> writes:
>> I'm a strong proponent of "different completions should be selectable by
>> different strings", for the kinds of reasons exposed by Daniel: it makes
>> it possible to use more UI styles than just selection (and it interacts
>> better with other things like elimination of duplicates).
> If we have completions that are textually different, then that's no
> problem, of course.  But requiring the callers to construct strings that
> differ in artificial ways seems less than optimal.

The strings used for completions are the "identity" of each completion.
so if there are two distinct completions, they should have
distinct strings.  I don't see what's artificial about it.

Somehow the user (and the code) needs to be able to distinguish between
the various identically named movies.  You do that with a poster image
and I'm suggesting that this poster image should be covering some
"unique" identification information.  I.e. something like:

    (concat movie-name (propertize movie-id 'display movie-poster))

The `movie-id` could be derived from the poster itself if needed.
I can't see why you'd find that to be a problem.

You can also move the movie to an "annotation" and make movie names
unique by adding a number, something like:

    (defun movie-make-unique (movies)
      "MOVIES is a list of (MOVIE . IMAGE).
    Return a new list of (MOVIE . IMAGE) where every MOVIE is unique."
      (let ((ht (make-hash-table :test 'equal)))
        (mapcar (lambda (m)
                  (let* ((name (car m))
                         (old (gethash name ht))
                         (new (if (null old) m
                                (unless (cdr old)
                                  (setf (caar old) (concat (car old) " (1)")))
                                (cons (format "%s (%d)" name (1+ (length old)))
                                      (cdr m)))))
                    (setf (gethash name ht) (cons new old))
                    new))
                movies)))

We could add such a function to `minibuffer.el`.

>>> If I remember correctly, I ended up copying most of the completion
>>> machinery into the package just to avoid the stripping.
>> We should fix the code so it's not necessary.
> Which brings me back to my original question.  😀  Why are we stripping
> text properties?  Is this to fix some bug or something?

The rest of the discussion made me realize that maybe I misunderstood
your question.  Are you talking about the stripping that takes places
*during completion* (e.g. when clicking in *Completions*) or are you
talking about the stripping that takes place just before returning the
value of `completing-read`?  Some other?

> I.e., if we add an interface to allow completion to not strip text
> properties, is that going to lead to bugs?

What do you mean by "interface"?  You mean a UI or an API?
For an API it would probably lead to this API being virtually unusable
for some UIs.


        Stefan