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

[Daniel Mendler]

On 5/2/22 10:11, Lars Ingebrigtsen 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.

I argue that the strings should not differ in artifical ways. If you
want to select from different candidates which are actually different in
some ways. For example movies with the same name from the same year
certainly have different directors and different actors. The user can
then match against the name of the director when searching for the
movie. If the candidates would be truly "more equal", such that the
disambiguation suffix would be truly artificial, then why would you
distinguish the candidates in the first place?

>>> 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?
> 
> I.e., if we add an interface to allow completion to not strip text
> properties, is that going to lead to bugs?

I mentioned this in my other mail. The text properties don't necessarily
materialize in the first place, because it is just textual input by the
user. If a UI wants to enforce that text properties are returned it
could do so by performing a lookup in the final step and by setting
minibuffer-allow-text-properties. By stripping the text property we
ensure that the result is normalized and uniform across all scenarios.
As I mentioned before, text properties are only meaningful for
REQUIRE-MATCH non-nil and also not for the "null completion" (empty string).

Btw, I consider the "null completion" a design mistake which should be
corrected. If REQUIRE-MATCH is non-nil, no null completion should be
allowed. If the caller of completing-read truly wants the null
completion they can either modify the test-completion action of the
programmable completion table or pass the empty string as DEFAULT
argument to completing-read.

Daniel