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

Consult narrowing is broken due to computing table outside minibuffer #320

Closed
minad opened this issue Jan 5, 2021 · 9 comments
Closed

Comments

@minad
Copy link
Contributor

minad commented Jan 5, 2021

The offending commit is b462920, found via bisect. It worked before that commit and it still works with icomplete. Some problem with the predicate. My suggestion would be to revert b462920 at first.

@minad minad changed the title Consult narrowing is broken Bug: Consult narrowing is broken due to recent Selectrum refactoring Jan 5, 2021
@minad
Copy link
Contributor Author

minad commented Jan 5, 2021

Okay now I understand the problem - my predicate relies on being executed in the minibuffer :(

And this works with icomplete so I wonder what the correct behavior is.

@clemera
Copy link
Collaborator

clemera commented Jan 5, 2021

The commit was to speed up for describe-variable, why does narrowing need the minibuffer buffer to be current?

@minad
Copy link
Contributor Author

minad commented Jan 5, 2021

@clemera It relies on minibuffer local variables.

EDIT: I could also fix this on the side of consult trying to obtain the minibuffer local variable. But I think we should figure out instead what the correct behavior is. Maybe really special case describe variable? I mean introducing a buffer switch is not natural, so it is unexpected that this happens by default. For describe-variable we need the special case though. What is ivy doing exactly? Are they also doing exactly this - switching to the latest buffer?

EDIT2: I think the natural thing is that all the callbacks invoked from completing-read live inside the minibuffer. I think this is what I implicitly assumed all the time and I think this assumption did hold until now. Also for the annotators I think.

@clemera clemera changed the title Bug: Consult narrowing is broken due to recent Selectrum refactoring Consult narrowing is broken due to computing table outside minibuffer Jan 5, 2021
@clemera
Copy link
Collaborator

clemera commented Jan 5, 2021

I agree the buffer switch can always be unexpected. Ivy does compute the candidates before even entering the minibuffer. In the light of this I think special casing is probably the best here.

@minad
Copy link
Contributor Author

minad commented Jan 5, 2021

Maybe it makes sense to special case the exact predicate function. I guess it is a named predicate for describe-variable, not a lambda?

@clemera
Copy link
Collaborator

clemera commented Jan 5, 2021

No, it is a lambda I think we can do it for #'help--symbol-completion-table

@minad
Copy link
Contributor Author

minad commented Jan 5, 2021

@clemera Okay, that's also good!

@clemera
Copy link
Collaborator

clemera commented Jan 5, 2021

The other commands that use it are describe-function and describe-symbol and they do no buffer switching so they aren't affected by this

@clemera
Copy link
Collaborator

clemera commented Jan 5, 2021

Can be closed?

@clemera clemera added the waiting on response Needs more info or follow-up, will be closed after 90 days if no response label Jan 5, 2021
@minad minad closed this as completed Jan 5, 2021
@raxod502 raxod502 removed the waiting on response Needs more info or follow-up, will be closed after 90 days if no response label Jan 6, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

No branches or pull requests

3 participants