Candidate order is changed when filtering in fido-mode

[minad]

@oantolin When using consult-line the candidates are first nicely sorted by line, but as as soon as I start filtering, the order gets messed up. I only see this with icomplete and not with selectrum. Do you also have this and is this expected? I am using the identity as cycle-sort-function and display-sort-function in consult--read as you are probably aware.

[oantolin]

I can't reproduce this problem here, the lines stay in order using icomplete-vertical. ☹️ I wonder what's going on.

[minad]

@oantolin I am also testing with icomplete-vertical. I will take another look a closer look soon.

[oantolin]

Have you managed to reproduce this? It doesn't seem to happen with my config, I've been using consult-line with icomplete plenty and haven't seen this.

[minad]

Yes, I mean it happens all the time. I didn't not investigate more carefully yet. I should check with emacs-Q etc.

[minad]

@oantolin I can reproduce it. If I load the following config in emacs -Q Emacs 27.1, call consult-line on a file, enter for example defun and scroll up down, then some candidates are "out of line", e.g., line 200 coming after 600 or something like this.

(require 'icomplete)
(load "~/.config/emacs/elpa/icomplete-vertical-20201206.759/icomplete-vertical.el")
(load "~/.config/emacs/elpa/consult-0.1/consult.el")
(setq completion-styles '(substring))
(define-key icomplete-minibuffer-map [down] 'icomplete-forward-completions)
(define-key icomplete-minibuffer-map [up] 'icomplete-backward-completions)

[oantolin]

Thanks! I'll look into it.

[oantolin]

I'm stumped, @minad. I couldn't reproduce the problem. The list of lines stays cyclically ordered here, with the exception of the current line which is the default value and thus goes to the top of the list.

I hesitate to bring this up, but maybe since you are used to Selectrum you don't expect it: in icomplete the list wraps around, so after the last matching line, you'll see the first line again. And also, whatever the default candidate is gets moved to the top of the candidate list. Other than those two things, which are normal with icomplete, I didn't see any disorder.

[minad]

@oantolin No no, it is not the cycle. I would obviously see that. There is some actual disorder. I think I have to do some debugging.

[oantolin]

Just to be super explicit: it's not just the cycle, also the current line is out of order and placed at the top of the list. But it was more disorder than even that, right @minad?

[minad]

Sure, the out of order top is expected. The default-top keyword argument in consult is only used for selectrum.

Actual disorder :(

[minad]

I wouldn't call a single out of place item disorder. I would call it an impurity ;)

[minad]

@oantolin The problem seems to be fido-mode. Default completion and icomplete-mode+icomplete-vertical work without disorder. Maybe you can confirm this?

[jixiuf]

emacs-mirror/emacs@dc6e616 maybe ralated bug 19031

[minad]

@jixiuf I don't see how it is related? I am not seeing anything related to disorder.

[oantolin]

Yep, fido-mode scrambles everything! Will investigate, @minad. (I already had some ill-will towards fido-mode since it overrides completion-styles, and now this...)

[minad]

Maybe you could work a bit with upstream, also with respect to your icomplete-vertical mode. I guess you could make a difference there!

[minad]

@oantolin I tried with fido since I was accustomed to ido before I made the switch to selectrum.

[oantolin]

Ah they're not "scrambled" in fido-mode! They are sorted in the order completion-all-sorted-completions. It's weird: icomplete does respect the sorting metadata without fido-mode.

[minad]

Ah they're not "scrambled" in fido-mode! They are sorted in the order completion-all-sorted-completions. It's weird: icomplete does respect the sorting metadata without fido-mode.

Yes I suspected some weird sorting is going on. But note that I am opting out of sorting via the metadata.

[oantolin]

But note that I am opting out of sorting via the metadata.

I know! And icomplete alone respects that, but fido-mode doesn't?! I don't understand it yet, it would seem from the code that fido doesn't touch the sorting, let's icomplete do it.

[oantolin]

Ha! It's not fido-mode directly! It's the flex completion style: fido-mode overrides completion-styles setting it to '(flex). But if you turn off fido-mode and test with (setq completion-styles '(flex)) the strange reordering happens anyway.

[oantolin]

Found the culprit! It was completion--flex-adjust-metadata. There is a notion of how good each flex completion match is, and that function overrides cycle-sort-function to first sort by whatever cycle-sort-function metadatum existed, and then sort those results again by "completion-score" (which is stored in the candidates in a text property). It does the analogous thing for display-sort-function.

[oantolin]

So the flex matching style doesn't ignore your custom cycle-sort-function and display-sort-function, but it does compose them with another function that sorts by completion-score.

People who want to opt out of this behavior can use (put 'flex 'completion--adjust-metadata nil).

[minad]

@oantolin So should this be reported as a bug to upstream or do you think this is acceptable behavior? It is good to know for us but clearly not a bug in consult, therefore I close the issue here.

[minad]

And thank you very much for investigating, I should say!

[oantolin]

I didn't know about this completion--adjust-metadata property. Apparently you can put it on a symbol naming a completion-style, setting it to some function to compute new metadata given the old metadata. The only use of that property in all of minibuffer.el is to do this extra sorting step for the flex completion style.

[minad]

Is completion--adjust-metadata something we should use in marginalia? Since we are adjusting metadata there.

[oantolin]

And thank you very much for investigating, I should say!

I love these sorts of mysteries! 😄

So should this be reported as a bug to upstream or do you think this is acceptable behavior?

I actually think this is probably acceptable behavior, since flex matching produces so many results, I think it is reasonable for flex to mess with your ordering. Also, we can actually fix it in consult-line (and other functions that need candidates in a fixed order): we can temporarily (put 'flex 'completion--adjust-metadata nil) and use unwind-protect to ensure it goes back to (put 'flex 'completion--adjust-metadata 'completion--flex-adjust-metadata) when consult-line is done.

Is completion--adjust-metadata something we should use in marginalia? Since we are adjusting metadata there.

No, because it is a property you put on completion styles, not on categories or on commands or anything like that.

[oantolin]

You know, I think we should probably do this temporary disabling of the flex reordering. I mean, consult does do whatever it takes to keep the lines in the right order. Doesn't it also have to use selectrum-should-sort-p (or whatever it is called)? If it has a special case for selectrum it should also have one for flex: flex is built-in!

[oantolin]

Oh, I was wrong, consult does not use selectrum-should-sort-p, since selectrum now respects display-sort-function. I still think we could do the fix for flex.

[oantolin]

I made a pull request,with the strategy I mentioned for flex, see if you agree with it @minad.

[oantolin]

I'll also ask emacs-devel if flex shouldn't maybe only sort if there are no sorting functions. But in the meantime, my PR is an easy way to get all the consult functions that want a fixed order of candidates to work with fido-mode.

[oantolin]

I reported this flex thing as a bug. Let's see what the maintainers say about it.