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

Incremental history search stopped working #124

Closed
gcv opened this issue Jul 10, 2020 · 9 comments
Closed

Incremental history search stopped working #124

gcv opened this issue Jul 10, 2020 · 9 comments

Comments

@gcv
Copy link

gcv commented Jul 10, 2020

Incremental history search stopped working at some point since the last time I updated Selectrum (I guess back in April). :(

Original discussion in ticket #49.

@clemera
Copy link
Collaborator

clemera commented Jul 11, 2020

Can you provide more details what stopped working for you? You can use selectrum-select-from-history (bind to M-r) which is an improved version for history search.

@raxod502 raxod502 added the waiting on response Needs more info or follow-up, will be closed after 90 days if no response label Jul 11, 2020
@gcv
Copy link
Author

gcv commented Jul 11, 2020

Steps to reproduce (from a sandbox Emacs, only Selectrum is installed): start any command (M-x, C-x C-f), hit C-r or C-s to start incremental history search. This pops up an I-search prompt. Start typing, it says Text is read-only. Then hit C-g to try bailing, and it says Wrong type argument: isearch--state, nil.

selectrum-select-from-history looks nice and does what I want, but I think either C-r/C-s should be fixed, or (cheap simple workaround) just rebound to selectrum-select-from-history by default.

Update: After working with selectrum-select-from-history a bit: hitting C-g to bail out of it sticks a big old Quit on the entire completion display which takes a while to disappear. It should just immediately cancel the history selection and go back to whatever command I was in. Should I open a separate ticket for this?

@clemera
Copy link
Collaborator

clemera commented Jul 11, 2020

start any command (M-x, C-x C-f), hit C-r or C-s to start incremental history search. This pops up an I-search prompt. Start typing, it says Text is read-only

I can't reproduce this in Emacs 26.3. C-r and C-s are bound to regular isearch commands and those work as expected for me. Maybe you have rebound them in the sandbox to previous/next-matching-history-element? previous-matching-history-element is already remapped to selectrum-select-from-history (M-r is the default key).

Update: After working with selectrum-select-from-history a bit: hitting C-g to bail out of it sticks a big old Quit on the entire completion display which takes a while to disappear.

Thanks, this is a known issue and mentioned in the caveats section of the README:

In Emacs 26 and earlier, the way that messages are displayed while the minibuffer is active is unworkably bad: they block out the entire minibuffer as long as they are displayed, and then mess up redisplay. This issue has been fixed in Emacs 27, and I suggest upgrading. I think the best solution for people running Emacs 26 would be the development of a small third-party package which backports the improvement from Emacs 27. That way all minibuffer-based packages can benefit from the improvement.

@gcv
Copy link
Author

gcv commented Jul 11, 2020

Fair enough on the messages display and Emacs 27.

With regard to incremental history search in Selectrum: of course the sandbox doesn't have any customizations, or anything other than selectrum installed. That's why it's a sandbox. Here's a screencast which shows me deleting the existing sandbox and installing from scratch and showing the error.

screencast-780p

Steps to reproduce: open a file to seed find-file history, kill the buffer with the file, find-file, C-r to start reverse history search, start typing.

@clemera
Copy link
Collaborator

clemera commented Jul 11, 2020

Okay, thanks now I can reproduce it. The behaviour you described seems to happen only when isearch starts going through the history which I had not tried (I only searched for text within the current prompt). I did not even know that isearch automatically starts doing this within the minibuffer.

@clemera
Copy link
Collaborator

clemera commented Jul 11, 2020

@raxod502
The problematic function is goto-history-element called by minibuffer-history-isearch-wrap and minibuffer-history-isearch-pop-state. goto-history-element is also the reason that other history commands of Selectrum are wrapped within inhibit-read-only and narrowed to current input.

When selectrum switches to use overlays (like mentioned in #116) for displaying the candidates that problem should be gone, too. Maybe for now we should advice goto-history-element?

@raxod502
Copy link
Member

Wow. That's a pretty wild implementation of history Isearch. I guess adding an advice would be a reasonable solution, as that way we won't have to worry about other commands that happen to use it. But it does indeed seem like this is getting slightly out of hand, and if we can handle the minibuffer in such a way that functions like goto-history-element will work by default, that would be much better.

@raxod502 raxod502 removed the waiting on response Needs more info or follow-up, will be closed after 90 days if no response label Jul 13, 2020
@clemera
Copy link
Collaborator

clemera commented Jul 28, 2020

Should be fixed by #133 can you confirm?

@clemera clemera added the waiting on response Needs more info or follow-up, will be closed after 90 days if no response label Jul 28, 2020
@gcv
Copy link
Author

gcv commented Jul 30, 2020

Looks good! Thank you.

@gcv gcv closed this as completed Jul 30, 2020
@raxod502 raxod502 removed the waiting on response Needs more info or follow-up, will be closed after 90 days if no response label Jul 30, 2020
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