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

helm should not exit before end of update #1750

Closed
dwang20151005 opened this issue Apr 19, 2017 · 18 comments
Closed

helm should not exit before end of update #1750

dwang20151005 opened this issue Apr 19, 2017 · 18 comments

Comments

@dwang20151005
Copy link

Expected behavior

If I do

(require 'cl)
(let ((prompt "prompt")
      (candidates (loop for i from 0 to 500000 collecting (format "foo bar baz %S" i)))
      (require-match nil))
  (completing-read "prompt" candidates require-match))

and type jjjjjk quickly, I would expect it to return what I typed but
instead it returns "foo bar baz 0".

This is inconvenient for me because I use helm everywhere (it's a great tool!), and sometimes I completing-read input for production systems.

Actual behavior from emacs-helm.sh if possible (See note at bottom)

See above.

Steps to reproduce (recipe)

See above.

Backtraces if some (M-x toggle-debug-on-error)

No error, just unexpected behavior.

Describe versions of helm, emacs, operating system etc.

helm-20170416.945
GNU Emacs 25.1.2 (x86_64-unknown-linux-gnu, X toolkit, Xaw scroll bars)
of 2016-09-20
CentOS 6
Linux igm-qws-u12112a 2.6.32-642.15.1.el6.x86_64 #1 SMP Fri Feb 24 14:31:22 UTC 2017 x86_64 x86_64 x86_64 GNU/Linux

IMPORTANT NOTE:

If you are using a Unix or GNU-Linux system there is no excuses
to not use emac-helm.sh to reproduce your bug in 99% of the cases.

@thierryvolpiatto
Copy link
Member

thierryvolpiatto commented Apr 20, 2017 via email

@thierryvolpiatto
Copy link
Member

thierryvolpiatto commented Apr 20, 2017 via email

@dwang20151005
Copy link
Author

I'm afraid I don't control the call site, so I cannot substitute helm-comp-read for completing-read. And I wouldn't, anyway, because not everyone who runs this code uses helm.

Also, I think you are treating this as primarily a performance issue, but I claim it is a matter of incorrect behavior. It's okay if helm gets very slow. It's a powerful, general package and I'm willing to pay for that. I wouldn't complain if I could type j j j k RET, wait however long, and get "jjjk". My complaint is that when there are enough candidates, helm returns something that does not match the keystrokes I typed before RET. Does that make sense?

@thierryvolpiatto
Copy link
Member

thierryvolpiatto commented Apr 20, 2017 via email

@thierryvolpiatto
Copy link
Member

capture d ecran_2017-04-20_20-36-24

@dwang20151005
Copy link
Author

Since you say I have to wait for helm to return before hitting RET, does that mean you can reproduce the behavior I reported where if I don't wait for helm before hitting RET, then helm returns the wrong thing?

If so, then I think we agree on the current behavior and only disagree on the desired behavior. I would prefer that the result not depend on the timing of the last keystroke.

@thierryvolpiatto
Copy link
Member

Then when you press RET you have "jjjk" as expected.

@dwang20151005
Copy link
Author

To say this another way: my mental model of helm is that it should consider each keystroke in order, and the result should be independent of the timing between keystrokes.

@thierryvolpiatto
Copy link
Member

Since you say I have to wait for helm to return before hitting RET, does that mean you can reproduce >the behavior I reported where if I don't wait for helm before hitting RET, then helm returns the wrong >thing?

Of course, if you press RET immediately you endup with the first candidate.
He! you have in your example 500000 candidates, it emacs-lisp, it is slow you have to wait.

@jakemcarthur
Copy link

jakemcarthur commented Apr 20, 2017

The bug dwang is trying to report is that this asynchronous behavior is pretty much never what he (or I) actually wants to happen. It would be easier to control your actions if f o o <wait for a while> RET behaved the same as f o o RET <wait for a while>.

@xiongtx
Copy link
Member

xiongtx commented Apr 20, 2017

If you hit RET, Helm immediately executes action on the currently selected candidate. If you haven't waited for Helm to filter candidates and put selection line on the one you want, you'll be executing action on the wrong candidate.

It takes time for Helm to interpret the keystrokes, filter the candidates, and put the selection line on the filtered candidate. Don't think there's a way around that.

@dwang20151005
Copy link
Author

dwang20151005 commented Apr 20, 2017

It sounds like a better mental model is that helm maintains a queue of keystrokes. Some keystrokes, like j j j k, are enqueued and processed in the order they are received. Other keystrokes, like RET, are processed immediately, even if there are still unprocessed keystrokes in the queue.

In that case, would it be hard to make RET go through the queue?

I'm just trying to avoid the behavior where I can type j j j k RET and depending on the timing of my keystrokes I get either foo bar baz 0 or jjjk. (Do you agree that this behavior is surprising?)

@thierryvolpiatto
Copy link
Member

Oh! I understand now, it is a bug in helm--updating-p, I will fix soon.
Thanks.

@dwang20151005
Copy link
Author

Thank you for writing helm!

@xiongtx
Copy link
Member

xiongtx commented Apr 20, 2017

Oh! I understand now, it is a bug in helm--updating-p

I stand corrected 😛.

thierryvolpiatto pushed a commit that referenced this issue Apr 21, 2017
* helm.el: (helm-update): Reset helm--in-update only when candidates list is
non--nil.
(helm-maybe-exit-minibuffer): Hitting RET have maybe stopped update, reload it.
thierryvolpiatto pushed a commit that referenced this issue Apr 21, 2017
* helm-mode.el (helm--completing-read-default): Use in-buffer on fixed lists.
@thierryvolpiatto
Copy link
Member

Should now be fixed, you may not see the fix even when typing fast because I have now optimized completing-read (nearly instant update with your 500000 example).

@thierryvolpiatto thierryvolpiatto changed the title helm takes keystrokes out of order helm should not exit before end of update Apr 22, 2017
@dwang20151005
Copy link
Author

Works for me. And thanks for providing the emacs-helm.sh script. Makes testing so much easier.

@introom
Copy link

introom commented Dec 31, 2017

there should be an option to allow the old behavior of exiting helm when the update is not ready yet.

here is my use case, and I believe it's very general:
say I am working on the kernel code base with helm-gtags.

I type fast: skb_reserve and hit enter. However, helm prompts [Display not ready] and emacs gets stuck for seconds until the helm results are updated.

The thing is that, when I press enter, the highlighted entry is already what I want, even though the helm update is still in progress. It is not meaningful to wait for more seconds for the helm update.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

5 participants