-
Notifications
You must be signed in to change notification settings - Fork 33
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
Rework state management #322
Comments
Good idea, thanks! To summarize: Remove the global state macro in favour of local session state variables. |
One more: Declare all variables as defvar-local if they are only intended to be used in the minibuffer. This makes their scope more clear. And I should add the motivation: This will fix all recursive minibuffer issues forever! Hopefully it will also require less code. |
For some variables we needed the first (global) value but when selectrum-repeat can be replaced by something better as discussed on the embark side this might not be the case anymore.
I wonder if there an easy way to retrieve local values from the previous minibuffer session? |
I only know the general way, find minibuffer-buffers and extract the local value from them. Why do we need this? I would like the recursive sessions to be independent! Is this not possible or not desired in some cases? |
I don't think we need it but it can be handy at times, I don't remember exactly where I have seen it unfortunately. One benefit of the let bound variables was that the values are available whatever buffer is selected, when removing this we have to make sure to always query the values from the minibuffer. |
I don't understand this. You are also using setq-local at some places so you can never be sure. This is something I am very much against. This is like some kind of Heisen state, maybe being available maybe not. From my pov, the variables should be defined as local and should only be set in the minibuffer. This is how I am doing things in consult. And there I am only doing that when I cannot use closures. |
Yes when adding new variables I usually set them local since we already discussed removing the macro in the past. This is just how things evolved I have not said I like the situation but it also is possible to fix, we just need to be a bit careful not to break things. Would be nice to have tests already ;) |
Happy to close this issue! Thank you for tackling the state management, @clemera 👍 |
Thank you, too for the discussion and help! |
See https://www.reddit.com/r/emacs/comments/kqutap/selectrum_prescient_consult_embark_getting_started/gi77l5e/?context=3
@clemera I took the freedom to create an issue here regarding this in order to keep it in mind.
The text was updated successfully, but these errors were encountered: