search

emacs-helm / helm

Emacs incremental completion and selection narrowing framework
https://emacs-helm.github.io/helm/
GNU General Public License v3.0
3.36k stars 389 forks source link

Wrong candidate when typing too fast #450

Closed michael-heerdegen closed 10 years ago

michael-heerdegen commented 10 years ago

Yes, this was discussed several times before:

(setq helm-input-idle-delay 1.0)

(completing-read "Input: " '(a b c))

Type b and hit enter immedaitely. You get "a" instead of "b". The situation with a smaller helm-input-idle-delay is not much different, the only difference is that the slip-up happens less frequently, but it stil happens when typing fast, and this is annoying, it can even be dangerous.

In addition, it is not good that you have to choose a small value for helm-input-idle-delay in your config, or as default value in helm, only for the sake of preventing this issue.

Is there any chance to fix this? Can't we just do add something to `helm-exit-minibuffer' that checks (for non-delayed sources) whether the current candidate is "outdated"? Or add a trivial timeout that waits until the timeout is over? Or is there any reason that prevents us from treating this case?

IMHO, the goal should be that for completion with non-delayed sources, the behavior should only depend on hit keys, not on the time delays between them.

thierryvolpiatto commented 10 years ago

Keshav Kini notifications@github.com writes:

With the latest code from master, I am no longer able to get "[No match]" with M-x x RET. If I type fast enough, I get "[Display not ready]", and the completion buffer fills with things that have "x" in them.

Good.

But if I type really fast, I get "[Display not ready]", and the completion buffer remains empty even if I continue to wait.

This is not related to this bug, it is something really rare that is triggered by helm-while-no-input' probably, just updating the display by either deleting "x" and reenter it, or hittingC-c C-u' should fill up your helm-buffer again. Not really an issue so.

Thanks for your feedback.

Thierry Get my Gnupg key: gpg --keyserver pgp.mit.edu --recv-keys 59F29997

kini commented 10 years ago

IMHO "it happens rarely" doesn't mean it's not an issue, it just means that maybe nobody cares enough to fix it fully. But thanks for improving the situation, even if the problem is not "fully" fixed. :)

michael-heerdegen commented 10 years ago

Keshav Kini notifications@github.com writes:

IMHO "it happens rarely" doesn't mean it's not an issue, it just means that maybe nobody cares enough to fix it fully. But thanks for improving the situation, even if the problem is not "fully" fixed. :)

The question is whether it happens "in real life". If it hinders your work, it's worth fixing it when it's possible with an acceptable amount of changes.

kini commented 10 years ago

Fair enough!

michael-heerdegen commented 10 years ago

Keshav Kini notifications@github.com writes:

Fair enough!

I guess that answer implies that it indeed hinders you with your work, right? I mean (with the already made changes), you still do not see that problem only under testing conditions, but also in daily life, yes?

Because, I think fixing this would probably be ugly, so I don't know if Thierry would like to do this. When an keyboard event kicks us out of the candidate calculation, we can only guess whether that event was accidentally an too early RET, and start searching anew. That is not much more than guessing when the user probably wants to hit C-c C-u, and do that for him.

BTW, that "kicking out of calculation" in case of input can't be done conditionally in Emacs, Emacs allows only to do it for any input, or to not do it. And we want that keyboard input is processed immediately, so that when you add something to the minibuffer, you get no annoying timeout. That's not at all useful for RET, but we can't fix this directly, we can only try to correct the mischief afterwards.

kini commented 10 years ago

No no, you misunderstand me. By "fair enough" I mean, you're right that it's rare enough that it shouldn't bother me now, but I still think it's an unsolved bug because there exists a way to trigger it, even though it's rare. If software is buggy in extremely rare corner cases, it's still buggy, IMO. But I also respect your decision that if it doesn't affect someone's daily life, it's not worth fixing. Personally I work in the field of software verification so maybe I care more about "exact correctness" of software behavior than most people :P

If the maintainer says that it's not considered a bug, then I won't argue any further, especially since it doesn't really affect me in daily life anymore (unlike before the fix). So thanks for your work, both of you!

AndyMoreland commented 10 years ago

With the default value of 0.3 for helm-exit-idle-delay I was triggering this [Display not ready] message almost every time I used helm. It was driving me insane because the warning flashed so quickly that it took a day to notice it. I thought I was missing the enter key or something.

Changes like this ought to be better communicated and/or documented, I think.

thierryvolpiatto commented 10 years ago

Andrew Moreland notifications@github.com writes:

With the default value of 0.3 for helm-exit-idle-delay I was triggering this [Display not ready] message almost every time I used helm. It was driving me insane because the warning flashed so quickly that it took a day to notice it. I thought I was missing the enter key or something.

Changes like this ought to be better communicated and/or documented, I think.

Default value of `helm-exit-idle-delay' is still in discussion. I have finished implementing this less than a week ago... If you don't want such surprises you can use the tagged versions of helm, actually 1.6.2.

Thierry Get my Gnupg key: gpg --keyserver pgp.mit.edu --recv-keys 59F29997