Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Sorting (sort-fn) function does not work #24

Closed
cuprum opened this issue Jan 30, 2023 · 7 comments
Closed

Sorting (sort-fn) function does not work #24

cuprum opened this issue Jan 30, 2023 · 7 comments

Comments

@cuprum
Copy link

cuprum commented Jan 30, 2023

When I took a custom sort function and passed it to consult-org-roam-node-read, it didn't work.
I'm not an expert in this, but it looks like consult package somehow affects it.

After reading consult's docs, about consult--read in particular, I changed the value of :sort to nil

:sort sort-fn
and everything worked out.

@jgru
Copy link
Owner

jgru commented Jan 31, 2023

Hi @cuprum,

thanks for your message. Can you give an MWE here?
Did you passt a sort-fn when calling consult-org-roam-node-read?
I am asking since org-roam-node-read is overriden here via advice-add.

Best regards,
jgru

@cuprum
Copy link
Author

cuprum commented Jan 31, 2023

Here is https://gist.github.com/cuprum/20af58bc1f09460e552baf4726662c4f from my init.el.
I use this code to preview my daily notes strickly reverse alphabetically.

@jgru
Copy link
Owner

jgru commented Jan 31, 2023

Hi @cuprum,

Here is https://gist.github.com/cuprum/20af58bc1f09460e552baf4726662c4f from my init.el. I use this code to preview my daily notes strickly reverse alphabetically.

thanks for sending this snippet. Could you clarify why setting sort-fn to nil works?
I am asking because you pass your custom sort function in your snippet here, and this should then never be used as far as I see. Can you clarify this for me?

Best regards,
jgru

@cuprum
Copy link
Author

cuprum commented Jan 31, 2023

Can you clarify this for me?

I'm use bundling of Vertico + Marginalia + Orderless + Consult. So, the preceding code does not work without :sort nil in consult-org-roam.el. More precisely, in minibuffer with :sort sort-fn still show mtime's sorting order for my daily notes when i call yr/orm-node-find-wrapper.

@jgru
Copy link
Owner

jgru commented Jan 31, 2023

Can you clarify this for me?

I'm use bundling of Vertico + Marginalia + Orderless + Consult. So, the preceding code does not work without :sort nil in consult-org-roam.el. More precisely, in minibuffer with :sort sort-fn still show mtime's sorting order for my daily notes when i call yr/orm-node-find-wrapper.

Okay, thanks. Probably, this goes back to passing the parameter sort-fn already to org-roam-node-read--completions

Would you mind to test your specific use case whether supplying nil to this func(org-roam-node-read--completions) and your sort-fn to consult-read?

Best regards,
jgru

@cuprum
Copy link
Author

cuprum commented Jan 31, 2023

Would you mind to test your specific use case whether supplying nil to this func(org-roam-node-read--completions) and your sort-fn to consult-read?

With the specified conditions, I see the mtime's order.

In any case, Org-roam is responsible for sorting (more precisely, org-roam-node-read--completions in this case). Apparently, one should not use :sort sort-fn additionally. Moreover, docs for consult--read says

SORT should be set to nil if the candidates are already sorted.

@jgru jgru closed this as completed in 3c2701e Jan 31, 2023
@jgru
Copy link
Owner

jgru commented Jan 31, 2023

Thanks for researching and checking this behaviour, @cuprum.
As suggested by you, I changed :sort to nil in 3c2701e.
This should fix the issue.

Best regards,
jgru

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants