[#107] Return chosen candidate without text properties

[clemera]

This fixes #107. Emacs default completion functions return the string of a candidate without any text properties. I also removed selectrum--result because it is not needed anymore (it was there for the old implementation of selectrum-completing-read-multiple).

[okamsn]

Hello.

I wrote a command selectrum-swiper in the wiki that uses text properties to attach a line number to a line's content. Do you know the proper way to associate the information, since a line need not be distinct?

The other way I know of is to prepend the line number to the candidate, but then the line number shows up in the history, which is less desirable.

Thank you.

[clemera]

You could retrieve the string with properties from the original collection like I did in this PR for selectrum-read-library-name. So you would get the text property from (car (member chosen-line line-choices)). But this assumes there are no duplicates in the list not sure how you would deal with dups.

[okamsn]

You could retrieve the string with properties from the original collection like I did in this PR for selectrum-read-library-name. So you would get the text property from (car (member chosen-line line-choices)). But this assumes there are no duplicates in the list not sure how you would deal with dups.

Since it works with completing-read, I decided to just edit the history variable after selection with setcar. I will improve it if I think of a better way later. Thank you for your help.

[clemera]

That sounds like a good approach in this case!

[raxod502]

But, in the case of duplicates (this will happen when there are load-path shadows), how can you be sure of picking the right one? That's the original reason that I wanted to use text properties. If, indeed, it is part of the completing-read API to strip text properties, I am not sure what the right thing to do is... but I am pretty sure we need to handle duplicates properly, at least for this case.

Perhaps there could be a dynamically bound variable, non-nil by default, for whether to strip text properties.

[clemera]

Oh, right I did not thought about load path shadows, sorry. I just checked and the regular completing-read does remove any duplicates in the collection so that is just a case completing-read does not handle.

Perhaps there could be a dynamically bound variable, non-nil by default, for whether to strip text properties.

Would it be better to make that an selectrum-read argument? Because commands which rely on this feature would break when completing-read is used without selectrum. Or maybe we shouldn't allow duplicates, too and make sure library names always have a distinct name for the completion.

[clemera]

I have pushed a commit which includes the prefix in the candidate name. I think this way there shouldn't be any dups for load path shadows. It also has the benefit that you can match against the prefix.

[raxod502]

I don't really like this approach mainly because the sorting and filtering makes less sense to me. In selectrum-prescient.el, for example, it is assumed that shorter candidates should be put at the front. But with this change, any libraries that have load-path shadows will seem like they have long names, and will be sorted last. Also, the feature where typing in org will force each copy of the org library to the front of the list (i.e. exact matching) no longer works.

I also suspect that others will find it useful for text properties to be preserved. How would the Swiper-like command from the wiki be ported to this version of Selectrum? I don't think adding line numbers directly to the candidates would work very well -- it would make it confusing to search for numbers, for example, and it's in conflict with how other tools work, such as Swiper itself.

But now that I'm thinking, what's wrong with just making selectrum-completing-read strip text properties from the candidate returned by selectrum-read? Wouldn't that solve this problem without really any downside?

[okamsn]

I also suspect that others will find it useful for text properties to be preserved. How would the Swiper-like command from the wiki be ported to this version of Selectrum? I don't think adding line numbers directly to the candidates would work very well -- it would make it confusing to search for numbers, for example, and it's in conflict with how other tools work, such as Swiper itself.

The command uses completing-read, following the advice of the wiki. Since completing-read strips the text properties, I did change it to include the line numbers in the candidate.

You are correct that it is worse for searching for numbers (which I didn't consider), but it works well enough otherwise. Is it common it is to search for numbers? My experience is that important numbers are usually given a name.

Adding the line numbers also helps to make the candidates distinct. By doing it this way, it also works in Icomplete and the regular *Completions* buffer. Currently, the command manually removes the line numbers from its history variable, since I think that is more expected.

Please try it and let me know what you think. It currently satisfies my needs, but it is better to be useful for many.

Thank you.

[clemera]

@raxod502

Good points, I reverted the commit and now strip the text properties only for the built-in completion functions.

[raxod502]

Looks great! Want to add a changelog entry? (Man, I really have to make a release at some point, this is getting ridiculous.)

[clemera]

Want to add a changelog entry?

Done

[raxod502]

Thanks!