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

Add support for history #4

Closed
raxod502 opened this issue Jan 21, 2020 · 14 comments
Closed

Add support for history #4

raxod502 opened this issue Jan 21, 2020 · 14 comments

Comments

@raxod502
Copy link
Member

By default, it should be possible to use M-p and M-n to navigate the history of previous user inputs to Selectrum.

@clemera
Copy link
Collaborator

clemera commented Mar 27, 2020

An idea: A nice feature would be to show the last 1-3 history items (configurable) above the selected candidate. I imagine this feature similar to ivys :preselect but with a consistent approach. The downside of M-p and M-n is you don't know (unless you remember) what you will get. Also not having to switch from C-n/C-p to M-n/M-p is nice.

One downside of this approach would be the increased distance from a possibly selectable prompt. But one could jump to it with M-< which is currently not implemented but would make sense, I guess. And of course one could use C-j, too.

@clemera
Copy link
Collaborator

clemera commented Mar 31, 2020

Oh I just realized that by default, minibuffer history stores your actual inputs and not the submitted candidate. Independently of my previous idea above I think for selectrums model it would be better to store the complete candidate. I understand you don't want to divert to much from the default API, though in this case it might make sense to change the history handling as long as the user uses selectrum.

@raxod502
Copy link
Member Author

If you want previously selected candidates to be sorted at the top of the list, you should be using prescient.el.

@clemera
Copy link
Collaborator

clemera commented Mar 31, 2020

Thanks, I know and I have done that before but I like the more snappy behavior I get without prescient sorting and I like to have command based history storage which prescient doesn't do. With command based history I can invoke a command that I haven't invoked for a long time and still have the last used items at the top.

Maybe when minibuffer history for selectrum gets implemented it could be done in a way that so that one has access to the current minibuffer history variable inside selectrum-default-candidate-preprocess-function. This way users can configure something like my proposed behavior easily if they want.

@raxod502
Copy link
Member Author

To be clear: all of the following is just an opinion, and if you think I've missed something, feel free to point it out. I've been wrong about design many times before.

I just realized that by default, minibuffer history stores your actual inputs

This is desired, and I don't want to change the default behavior. Typing M-p should always put you in the same state as your last search, and show you the same candidates.

show the last 1-3 history items

I don't think this is a good road to go down. It complicates the abstraction considerably, and I am not convinced that the relevant use cases cannot be addressed using the existing system.

Selectrum should not maintain any candidate history. This is out of scope for the project, and should be handled by an external package which adjusts the preprocessing and refinement functions, such as prescient.el. The history stored by Selectrum is only the standard minibuffer history.

I like the more snappy behavior I get without prescient sorting and I like to have command based history storage which prescient doesn't do

Is there any reason why you can't implement your desired functionality using the same hooks that prescient.el uses? You can do the same thing as prescient.el except that you don't sort the entire list, you just use a function like selectrum--move-to-front-destructive to move a few history entries to the head of the candidates.

Also, if you can think of a way to implement your own logic that is sufficiently performant for your needs, it's possible that we could simply fix prescient.el to be faster. I have not done intense optimization. I am not sure at present whether I would be open to supporting per-command history, as it is a significant change and I'm not sure there is a way to implement it without causing a lot of problems.

@clemera
Copy link
Collaborator

clemera commented Mar 31, 2020

all of the following is just an opinion

The same goes for my suggestions/ideas which might sometimes be out of selectrums design philosophy ;)

This is desired, and I don't want to change the default behavior. Typing M-p should always put you in the same state as your last search, and show you the same candidates.

I agree that selectrum should probably behave like the default minibuffer. But I still think that the default history feature is sub optimal with completion frameworks like selectrum. Usually I'm interested about candidates I have chosen lately not how I searched for them and what the exact results of the search where. But it's not possible to fix that while staying compatible to stock minibuffer behavior. Maybe selectrum could offer a way to configure how history is stored by a simple selectrum-store-hist-function variabe which defaults to the behavior of stock Emacs.

Is there any reason why you can't implement your desired functionality using the same hooks that prescient.el uses? You can do the same thing as prescient.el except that you don't sort the entire list, you just use a function like selectrum--move-to-front-destructive to move a few history entries to the head of the candidates.

It's certainly possible to implement this using hooks and a custom sort function but because I'm not interested to use the default history input feature (because of the reasons mentioned earlier) I'm looking for way to make use of the existing history mechanism. Things like savehist-mode and history truncation would just work, which is nice.

Also, if you can think of a way to implement your own logic that is sufficiently performant for your needs, it's possible that we could simply fix prescient.el to be faster. I have not done intense optimization. I am not sure at present whether I would be open to supporting per-command history, as it is a significant change and I'm not sure there is a way to implement it without causing a lot of problems.

Making prescient faster would certainly help but having per command history would be more important to me and using a simple sorting mechanism like pushing the last items to the front would be enough for my purposes. I think implementing the default behavior with an easy way to adjust it would be optimal, at least from my point of view ;)

@clemera
Copy link
Collaborator

clemera commented Apr 1, 2020

After experimenting a bit it seems all that is necessary is to pass the history variable to read-from-minibuffer and additionally let bind minibuffer-history-variable in selectrum-read. This would not conflict with your plan to implement the default behavior and would allow me to manipulate the history the way I want in selectrum-candidate-selected-hook.

@clemera
Copy link
Collaborator

clemera commented Apr 2, 2020

Actually my claim was wrong: Ivy does save the actual input but with the default interface your input is the actual chosen candidate anyway.

@gcv
Copy link

gcv commented Apr 13, 2020

Is it too late to request support for history search? I really like how the default find-file supports history isearch with C-r. It also works in Ivy.

@clemera
Copy link
Collaborator

clemera commented Apr 14, 2020

@gcv That is a good idea. It is possible to swap the current candidate set by overwriting selectrum--preprocessed-candidates with an update function. I would prefer M-r because this binding is also used in multiple other contexts to search history (comint, isearch), also M-p and and M-n used for navigating history both use the Meta key.

@raxod502
Copy link
Member Author

551b119 should make the M-r binding available. I'm not sure if it works properly though.

@gcv
Copy link

gcv commented Apr 14, 2020

No on all counts. 🙃

M-r is different from C-r. I specifically want incremental search through history, which should work in the minibuffer. (It does in vanilla Emacs and in Ivy.) M-r sucks in comparison, and I'm annoyed whenever I'm stuck in a mode where it's the only history search I have available.

Looking at the code, selectrum-completing-read explicitly ignores the HIST parameter. In a compliant completing-read implementation, that should point to the command-specific history variable. (ivy-completing-read does this.) This does many good things, among them: (1) plugs into the Emacs savehist framework, and (2) enables command-specific history search.

PS: In any case, M-r from selectrum still breaks: Wrong type argument: sequencep, t.

@raxod502
Copy link
Member Author

History is now implemented. @gcv Would you mind opening new issue(s) for the problems that still exist with C-r and/or M-r in Selectrum when running without CTRLF, if any?

@gcv
Copy link

gcv commented Apr 16, 2020

Done, thanks!

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