equivalent of ivy-re-builders-alist for selectrum?

[aspiers]

Hi there,

I'm trying to switch from ivy/counsel/swiper to selectrum/consult/prescient, and am wondering if it's possible to have different candidate filtering queries for different types of candidate lists? For example, I might want fuzzy matching by default for find-file but not switch-to-buffer.

In ivy this is possible via the ivy-re-builders-alist variable, and indeed I see that ivy-prescient.el adds ivy-prescient-re-builder for use with this custom variable, and I also see #82 from someone who wants to continue with this mixed approach when using ivy-prescient.

However I would like to do it with the selectrum + prescient combination.

I appreciate that I can interactively toggle the various filter methods via M-s (selectrum-prescient-toggle-map), but this would require doing every time if I have different preferences for different contexts.

So is it possible to configure the default filtering per completion context in selectrum-prescient?

If not, please can this be considered a feature request?

Many thanks for this great software!

[raxod502]

This is not built in because I don't see a robust way to identify a key to denote the "current context". I think that Ivy uses this-command, which has various non-obvious pitfalls and edge cases. Of course, I have no issue with users keying off this variable if they are comfortable with the implications, but I'm hesitant to put this logic directly into the library.

However, here is one option that occurs to me. We could make it so prescient-filter-method can be a function, in which case it's called to get the real value. Then, if you wanted to have a different filter method depending on the context (or depending on anything else), you can simply write your own function that does a case off this-command. What do you think of that solution?

[aspiers]

Thanks for the quick reply!

@raxod502 commented on December 14, 2020 11:16 PM:

This is not built in because I don't see a robust way to identify a key to denote the "current context". I think that Ivy uses this-command, which has various non-obvious pitfalls and edge cases.

this-command is the third of three things it uses. M-x describe-variable ivy-re-builders-alist gives a bit more detail:

An alist of regex building functions for each collection function.

Each key is (in order of priority):

1. The actual collection function, e.g. ‘read-file-name-internal’.
2. The symbol passed by :caller into ‘ivy-read’.
3. ‘this-command’.
4. t.

but I previously filed swiper#2620 because the documentation does not make it entirely clear how that works. As you can see, I got some helpful replies. Now that I understand it, that approach seems reasonable, as the first two options are more reliable than this-command.

Of course, I have no issue with users keying off this variable if they are comfortable with the implications, but I'm hesitant to put this logic directly into the library.

Yeah, relying solely on this-command sounds fragile. What do you think about ivy's approach above? Like I said, it seems reasonable to me (and in my experience, works fine once you understand how to correctly configure ivy-re-builders-alist) but I could be missing something.

However, here is one option that occurs to me. We could make it so prescient-filter-method can be a function, in which case it's called to get the real value. Then, if you wanted to have a different filter method depending on the context (or depending on anything else), you can simply write your own function that does a case off this-command. What do you think of that solution?

That sounds fantastic to me for ultimate flexibility! Although even if that was added, I think it would probably still be worth adding something along ivy's approach above, because the custom function approach would require each user to write some code, and there are plenty of emacs users (in fact, most likely the majority) who like to tweak their config but don't know elisp well enough to do this. The counter-argument to that is "well they can just copy and paste some elisp from a README or a wiki page" but the counter-counter-argument to that is that copying and pasting code you don't understand is (IMHO) a butt-ugly approach which rapidly leads to brittle outdated config which is hard to maintain ;-)

[raxod502]

That all makes sense. I am still not sure I think it would be a good idea to introduce an alist for this, however:

1. The actual collection function will in many cases not be a symbol.
2. We don't have a :caller argument, which is intentional because we are not trying to wrap every Emacs command in a special wrapper to make them work with prescient.el and related packages.
3. this-command is very unreliable and I don't want to encourage its use explicitly.

I think that in most cases the key used by Ivy is not a good idea to rely on, at least in the prescient.el context. You are certainly right that it's not ideal to expect people to write code when something else would do. However:

• I think it's very important that we do not explicitly support this brittle feature. If we do, then people will reasonably get the impression that it is supposed to always work, and if it doesn't then there is a bug in prescient.el. This happened to Ivy and I don't want to repeat it here.
• Given that it is unlikely for this feature to always work, I actually think being forced to write code is better for everyone, even the people who are not very comfortable with writing code. Because if you are forced to write code to implement this logic, then it will be readily apparent where the problem is if it doesn't work (it must be in the few lines of code you just wrote, rather than somewhere in the monolith of the library package that is trying to do it automatically and failing), and it's also a lot easier to debug (for example, you could add a (message "%S" ...) line into your code to see what is going on). With some examples given in the documentation, I think this could be a perfectly user-friendly approach. I just can't imagine debugging the alist, which happened all the time with Ivy, being a friendly experience for someone not comfortable with Elisp. Either it works or it doesn't, and there is no visibility into why.

[aspiers]

I think you've mostly convinced me. And anyway, allowing prescient-filter-method to be a function is a fantastic first step and doesn't prevent extending it to support further flexibility in the future.

That said, more detailed replies follow below ...

That all makes sense. I am still not sure I think it would be a good idea to introduce an alist for this, however:

1. The actual collection function will in many cases not be a symbol.

Not surprised to hear that, but (forgive my ignorance) what else will it be?

2. We don't have a :caller argument, which is intentional because we are not trying to wrap every Emacs command in a special wrapper to make them work with prescient.el and related packages.
3. this-command is very unreliable and I don't want to encourage its use explicitly.

I think that in most cases the key used by Ivy is not a good idea to rely on, at least in the prescient.el context.

That seems totally fair to me.

You are certainly right that it's not ideal to expect people to write code when something else would do. However:

• I think it's very important that we do not explicitly support this brittle feature. If we do, then people will reasonably get the impression that it is supposed to always work, and if it doesn't then there is a bug in prescient.el. This happened to Ivy and I don't want to repeat it here.

Also totally fair.

• Given that it is unlikely for this feature to always work, I actually think being forced to write code is better for everyone, even the people who are not very comfortable with writing code. Because if you are forced to write code to implement this logic, then it will be readily apparent where the problem is if it doesn't work (it must be in the few lines of code you just wrote, rather than somewhere in the monolith of the library package that is trying to do it automatically and failing), and it's also a lot easier to debug (for example, you could add a (message "%S" ...) line into your code to see what is going on).

While that might be true for those who dabble in elisp, I'm pretty sure there will be a large group of users, probably even a majority, who literally know nothing about elisp and their interactions with it will be limited to copying and pasting code they don't understand and have no idea how to debug. (This is based on the assumption that many many people use emacs for coding totally unrelated languages like C, C++, Java, Python, Javascript, and some even use it without doing any coding at all, e.g. just for things like Org mode.)

For those people, it will not be readily apparent where things fail, or how to deal with the failures. Now of course I am not saying that this means it's your responsibility to maintain brittle code on their behalf! That would be ludicrous ;-) But there may be others who are willing to do this maintenance. So I think your proposal of allowing prescient-filter-method to be a function should do the job nicely, since it would allow third parties to provide functions for reuse, and potentially even distribute them via maintained repositories (although maybe that would be overkill, I'm not sure yet).

With some examples given in the documentation, I think this could be a perfectly user-friendly approach. I just can't imagine debugging the alist, which happened all the time with Ivy, being a friendly experience for someone not comfortable with Elisp. Either it works or it doesn't, and there is no visibility into why.

Ivy has things like ivy-alist-setting which could be made more user-friendly by adding debugging (optional or otherwise). For example I like the way straight.el debugs some stuff into *straight-process*. use-package and others adopt a similar approach. It would be easy for Ivy and other packages to adopt a similar approach, so that when the user isn't getting the behaviour they expect, they can just switch to that internals buffer to get some hints on what might have gone wrong.

[raxod502]

The actual collection function [...] what else will it be?

I am assuming by "collection function" that Ivy is referring to the COLLECTION argument of completing-read, which can be a function. For example, read-file-name-default passes the symbol read-file-name-internal as this argument.

However, as per the docstring of completing-read:

COLLECTION can be a list of strings, an alist, an obarray or a hash table.
COLLECTION can also be a function to do the completion itself.

So not only can the collection function be a lambda rather than a symbol (anything that counts as a function will do), it can also be a literal collection, rather than a function. And in fact this is the more common case, since using a function is only really needed when you are generating the candidates dynamically, as in filename completions.

third parties to provide functions for reuse, and potentially even distribute them via maintained repositories

Yes, that was exactly my thought. The idea would be that prescient.el itself can continue being a simple package that works in all circumstances, and it can provide hooks that other packages, which do not make such guarantees about working in all circumstances, can use to implement these kinds of features.

It's rather like the divide between https://github.com/raxod502/selectrum and https://github.com/minad/consult, where the one package is limited to providing the general interface, and the other package gets into the grimy details of all those special cases to give people the specific functionality they want in each case.

We can just start off with random code snippets that can be copied and pasted, and if it turns out to be a popular thing people want, then we could condense that work into a package that could be installed without the need for copying and pasting anything.

they can just switch to that internals buffer to get some hints on what might have gone wrong

This is a fair suggestion. I'd still like to keep the command-specific logic out of prescient.el proper, but an extension package which implemented this kind of conditional logic would greatly benefit from such a debugging interface, like you suggested, I think.

[aspiers]

OK, so it sounds like we have a plan. Is it basically just a question of changing prescient-filter-regexps to check whether prescient-filter-method is a function, and if it is, calling it and using the result?

If so, I could maybe take a stab at that, but I'm currently blocked on a lack of understanding of how the context (in particular, COLLECTION) would be provided to that function, because it's not (currently) passed down the call stack. I see that it's passed from selectrum-completing-read to selectrum-read as minibuffer-completion-table, but it only seems to be used for a non-nil check. And like you said, relying purely on this-command would not be very reliable.

Would we need to pass the context as an extra argument, or perhaps provide it via a dynamically bound variable?

[aspiers]

I just noticed that https://github.com/raxod502/selectrum/issues/114 also talks about handling of minibuffer-completion-table; not sure if that's relevant somehow.

[minad]

@aspiers I am not sure if I got the full discussion. But I think what you should use here is the completion category metadata. Based on the category select the desired completion styles/prescient filtering styles.

[minad]

See also https://github.com/minad/consult/issues/123#issuecomment-753940017 for a short discussion regarding different completion styles per command in consult with orderless. @protesilaos simply introduced wrappers for the commands with a special style. But you could also achieve this via some advice at the right place and then using the completion category.

[aspiers]

@minad Thanks - forgive my ignorance but I'm not sure what you mean by "completion category metadata" in this context? I'm guessing this is the same thing as the context which in my previous comment I was saying that I don't know how to obtain.

I can see how wrappers or advice would work fine, but I presume they would have the drawback of being much harder for the average user to set up, which is a concern @raxod502 and I discussed quite a bit further up in this issue.

[minad]

Thanks - forgive my ignorance but I'm not sure what you mean by "completion category metadata" in this context? I'm guessing this is the same thing as the context which in my previous comment I was saying that I don't know how to obtain.

See here for an example, each completing-read/consult--read can have a category: https://github.com/minad/consult/blob/c607afa65ae203a8904e7d18ddfbbccc53c1c680/consult.el#L2451. You can obtain it from the current minibuffer completion table by accessing the metadata.

[aspiers]

Apologies in advance if I'm misunderstanding any of this. I'm still not very familiar with how everything fits together.

@minad commented on January 9, 2021 10:45 PM:

Thanks - forgive my ignorance but I'm not sure what you mean by "completion category metadata" in this context? I'm guessing this is the same thing as the context which in my previous comment I was saying that I don't know how to obtain.

See here for an example, each completing-read/consult--read can have a category: minad/consult@c607afa/consult.el#L2451.

OK, that's very similar to COLLECTION referred to above. But my previous question remains: how would that metadata get passed from selectrum-read to prescient-filter-method?

You can obtain it from the current minibuffer completion table by accessing the metadata.

Yeah, I did notice the link with minibuffer-completion-table; in fact I suspect my comment about that was probably what drew your attention to this issue. But like I said, minibuffer-completion-table is only passed as far as selectrum-read, in which it is only used for a non-nil check and otherwise ignored. So to make it available further down the call stack, we'd presumably need to pass it all the way down as an extra argument, or perhaps provide it via a dynamically bound variable?

BTW this all seems slightly at odds with @raxod502's comment:

2. We don't have a :caller argument, which is intentional because we are not trying to wrap every Emacs command in a special wrapper to make them work with prescient.el and related packages.

So I assume any such metadata would have to remain optional from the perspective of selectrum and prescient.

[minad]

From what I understand the global variable minibuffer-completion-table is set by Selectrum. But I don't know much more here, I am not familiar with the prescient internals. And also not an expert on selectrum internals. I would have to dig around. Either @raxod502 or @clemera can probablyhelp.

[aspiers]

Ah, I missed that it was global. That could make things a lot easier. But then I wonder why selectrum is passing it around between functions. Yeah, will probably need advice from a selectrum / prescient guru here ;-)

[clemera]

So to make it available further down the call stack, we'd presumably need to pass it all the way down as an extra argument, or perhaps provide it via a dynamically bound variable?

It is already bound dynamically, we use the somewhat irritating behaviour of cl-defun which binds its arguments dynamically by default, you can check the macro expansion if you don't believe me ;)

But then I wonder why selectrum is passing it around between functions.

As mentioned above we pass the keyword arg to set the dynamic variable.

[aspiers]

@clemera commented on January 10, 2021 7:52 PM:

So to make it available further down the call stack, we'd presumably need to pass it all the way down as an extra argument, or perhaps provide it via a dynamically bound variable?

It is already bound dynamically, we use the somewhat irritating behaviour of cl-defun which binds its arguments dynamically by default, you can check the macro expansion if you don't believe me ;)

Ohhhhh I see! ... Wait, for which arguments is the binding different to defun in this respect? Maybe just for the &key / &aux arguments (which don't exist in defun), but not for the normal / &optional / &rest arguments? I'm not finding any explicit info on this in the cl info pages. Or maybe that depends on what lexical-binding is set to?

[clemera]

When you expand the macro you can see that it let binds the &key and &aux arguments. Maybe we should report that when the docs don't mention it.

[clemera]

@aspiers

So I assume any such metadata would have to remain optional from the perspective of selectrum and prescient.

I think allowing selectrum customizations based on metadata would be nice, this was also discussed here https://github.com/raxod502/selectrum/issues/265. As mentioned there currently you can also manually set selectrum variables locally to customize them for the current session.

Using metadata avoids the problems of this-command as the table specifies this information for the current session. Emacs 28 will also add current-minibuffer-command which can be used instead of this-command and this would be slightly more reliable for cases where a command invokes multiple minibuffer sessions per call but it is still derived from this-command.

[raxod502]

Would we need to pass the context as an extra argument, or perhaps provide it via a dynamically bound variable?

So it sounds like @clemera and @minad covered this, but for completeness: I would think that the feature on the prescient.el side would not include any explicit support for a context argument. It would simply allow for prescient-filter-method to be a function, which would be called with no arguments. That function, provided by the user, could look at this-command, or current-minibuffer-command, or minibuffer-completion-table, or some combination of the above.

If things accessible in the dynamic scope are not enough to satisfy reasonable use cases, then in selectrum-prescient we could consider dynamically binding a variable to the value of the collection, which would then allow the prescient-filter-method function to look at that as well.

Does this seem reasonable?

[aspiers]

Yes it does, very much so. My previous struggles to see a way forward were entirely due to me missing the fact that minibuffer-completion-table is global and dynamically bound. (Well, I was also ignorant of current-minibuffer-command coming in emacs 28, which is more good news.) Everything in your comment makes perfect sense now, and it sounds simple enough that I could probably code the solution myself unless someone beats me to it. Thanks all for your time so far!