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

Latest version breaks org-refile: "sort: Wrong type argument: stringp" #1567

Closed
kirill-gerasimenko opened this issue May 13, 2018 · 6 comments

Comments

@kirill-gerasimenko
Copy link

Hi,

The latest version (3689c8f) breaks 'org-refile' for me.

The error looks like this:
sort: Wrong type argument: stringp, ("Когда едешь назад раздается звук, как хрустит/" "e:/service.org" "^\\(\\*+\\)\\(?: +\\(DONE\\|TODO\\)\\)?\\(?: +\\(\\[#.\\]\\)\\)?\\(?: +\\(?:\\[[0-9%/]+\\] *\\)*\\(Когда едешь назад раздается звук, как хрустит\\)\\(?: *\\[[0-9%/]+\\]\\)*\\)\\(?:[ ]+\\(:[[:alnum:]_@#%:]+:\\)\\)?[ ]*$" 188)

Reverting back to https://github.com/abo-abo/swiper/blob/4fef67fd0bab6c98ed9ad36a4c795515e24e52de/ivy.el makes it work again.

My system is Windows 10, GNU Emacs 26.1 (build 1, x86_64-w64-mingw32) of 2018-04-06

I can provide more info if needed.
Thanks!

@basil-conto
Copy link
Collaborator

I think this is because org-refile's collection is an alist, so trying to sort it in ivy--reset-state with the default fallback string-lessp from ivy-sort-functions-alist fails.

While glancing at ivy--reset-state, I also noticed a less serious potential issue with sorting: caller and this-command aren't consistently considered synonyms.

@basil-conto
Copy link
Collaborator

basil-conto commented May 13, 2018

There is an ambiguity in the ivy-sort-functions-alist API: does the sorting function apply to string candidates regardless of the type of collection, or does it apply directly to the collection as it was originally specified? For example, when the collection is an alist, does the sorting function apply to cells, or to their CARs? FWIW, I think the former is more flexible, but the latter is by far the more user-friendly.

Yet another potential issue is that ivy-sort-functions-alist states that each of its CDRs can be a list of functions, whereas current usage of ivy--sort-function assumes they comprise a single function.

@basil-conto
Copy link
Collaborator

Oh, just noticed this issue is a duplicate of #1565.

@basil-conto
Copy link
Collaborator

For example, when the collection is an alist, does the sorting function apply to cells, or to their CARs? FWIW, I think the former is more flexible, but the latter is by far the more user-friendly.

Ah, but #554 says the sorting function should apply to the collection as it was provided. This rules out string-lessp as a suitable fallback in ivy-sort-functions-alist, right?

basil-conto added a commit to basil-conto/swiper that referenced this issue May 13, 2018
basil-conto added a commit to basil-conto/swiper that referenced this issue May 13, 2018
(ivy-string<): New function.
(ivy-sort-functions-alist): Fall back on it.

Fixes abo-abo#1565, abo-abo#1567
basil-conto added a commit to basil-conto/swiper that referenced this issue May 13, 2018
ivy.el (ivy--reset-state):
Fall back to this-command when ivy-state-caller is nil.

Re: abo-abo#1567
@abo-abo
Copy link
Owner

abo-abo commented May 13, 2018

This seems be fixed in #1568. Please verify.

@kirill-gerasimenko
Copy link
Author

Thanks, it works now!

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

3 participants