Rework state management

[clemera]

See #322, also improved selectrum-repeat so it can be used for repeating recursive sessions, too.

[minad]

Thank you for working on this! There are still a few defvar which are non-local. Maybe there is a reason, but I am sure more could be made local, e.g. selectrum-active-p. Just to make sure, it makes sense to go over each of the variables and check if it should be global and if yes document it. It is a bit of work but I hope that this resolves #322 once and for all :)

[minad]

;; possibly local?
(defvar selectrum-should-sort-p t
(defvar selectrum--minibuffer-default-in-prompt-regexps
(defvar selectrum--candidates-overlay nil
(defvar selectrum--dynamic-candidates nil
(defvar selectrum--crm-p nil
(defvar selectrum-active-p nil
(defvar selectrum--total-num-candidates nil

;; configuration (okay)
(defvar selectrum--crm-separator-alist
(defvar selectrum--candidates-buffer " *selectrum*"
(defvar selectrum-minibuffer-map

;; global old variables (okay)
(defvar selectrum--old-completing-read-function nil
(defvar selectrum--old-completion-in-region-function nil
(defvar selectrum--old-read-buffer-function nil
(defvar selectrum--old-read-file-name-function nil

[clemera]

For now I have only converted those handled by the state macro, which is gone now. Some of those you listed are set locally but not declared locally which can be changed of course. I will have to look into those next time.

[minad]

Yes, it is great that the state macro is gone! My personal style is to only use setq and defvar-local, but it is also perfectly valid what you do with the very explicit usage of setq-local everywhere. However one thing I would not do is to define a variable as defvar and then only set it via setq-local. This happens for selectrum-active-p afaik. But I guess you agree with that. I just took a superficial look at your PR! Thanks again!

[clemera]

Personally I don't like the implicit behaviour of setq and prefer to use setq-local to clearly indicate that something is set locally. I agree that it is better to use defvar-local for those variables which are meant to be set locally from the outset. Thank you also for pushing for this, it will also make some things easier I plan to do with completions outside the minibuffer.

[minad]

Personally I don't like the implicit behaviour of setq and prefer to use setq-local to clearly indicate that something is set locally.

I do not necessarily disagree with your statement. However given how variables are actually implemented, setq-local is the one doing the magic. setq is just accessing the dynamic context as defined by Emacs and by definition it can have these local variables. The same applies when reading variables. So by using my style I just accept how things are implemented and reflect that in the code 😄