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

Freeze when spamming a digit key #66

Closed
lenormf opened this issue Mar 7, 2017 · 12 comments
Closed

Freeze when spamming a digit key #66

lenormf opened this issue Mar 7, 2017 · 12 comments
Labels

Comments

@lenormf
Copy link

lenormf commented Mar 7, 2017

Hi,

lf completely freezes when I hit any numbered key (to use a command count for example) a few times (around 5 or more).

HTH.

@KenjiTakahashi
Copy link
Contributor

I don't think it really freezes, that looks more like a performance problem. I.e. when I hit 555555j it takes a couple of seconds to execute the command (even though there are about 30 files, so it is really just going to the bottom). The bigger the number, the longer it takes.

@lenormf
Copy link
Author

lenormf commented Mar 7, 2017

Unfortunately simply entering a big count without issuing any commands is sufficient to completely freeze the client.

@KenjiTakahashi
Copy link
Contributor

But how do you verify that it's frozen without actually doing anything :-)? After you hit the numbers, then as soon as you hit anything else, it will try to apply them numbers to your hotkey. I don't think there's any timeout feature, like in vim.

@lenormf
Copy link
Author

lenormf commented Mar 7, 2017

I'm using :, this doesn't use counts, does it?

@KenjiTakahashi
Copy link
Contributor

It doesn't, but... it does. I mean that count doesn't do anything in conjunction with :, but the command interpreter (I guess?) still applies the number to it, anyway, causing the slowdown. Try a big number with some unbound key, this should return immediately.

I managed (with 555555555, I think) to delay it a good couple of minutes :-].

@gokcehan
Copy link
Owner

gokcehan commented Mar 7, 2017

@KenjiTakahashi is right. It just takes an awful amount of time. You can try this with a remote command.

@lenormf Everything is a command in lf including : which is just read command. In this case count doesn't do anything meaningful. It just reads a command to read a command to read a command and so on.

@lenormf
Copy link
Author

lenormf commented Mar 7, 2017

So 2: will try to read two commands from the prompt? In any case, can anything be done against this complete lockup of the explorer?

@gokcehan
Copy link
Owner

gokcehan commented Mar 7, 2017

I guess we can try to add some check to see if the count is meaningful to the command or limit the count in case of a movement command but then again why do you accidentally hit something like 11111111: in the first place? There are easier ways to shoot yourself in the foot.

@lenormf
Copy link
Author

lenormf commented Mar 7, 2017

I was trying to bind to <a-1> (which isn't possible, maybe that deserves its own issue), and I hit the numbered keys a few times to see what was working and what was not.

In any case, unless there's a use to counts to : (is there?), I think this command shouldn't use a count at all, or maybe pass it through to the command typed in the prompt.

@gokcehan
Copy link
Owner

gokcehan commented Mar 7, 2017

We can try to pass the count to the evaluator and handle the count on a case by case basis which requires some refactoring. Alternatively, we can try to set a hard limit to counts (e.g. 10000) which should prevent accidental locks as in your case. We need to think about the consequences though.

Feel free to open an issue for alt key mappings.

@lenormf
Copy link
Author

lenormf commented Mar 7, 2017

That sounds good.

Issue #67 was created consequently.

@gokcehan gokcehan added the bug label Sep 18, 2017
gokcehan added a commit that referenced this issue Apr 12, 2018
gokcehan added a commit that referenced this issue Apr 12, 2018
@gokcehan
Copy link
Owner

I have now refactored the code to apply command counts on a case by case basis. Previously it was repeating for all commands including set commands which was kind of silly. Now it is only applied for up, down (and variants), updir, toggle, search-next, and search-prev. Feel free to report here if there is anything missing in this list.

Also it should be much more efficient. Previously a key push such as 100j was executing down command a hundred times and updating the ui in the end. This was creating a hundred goroutine reading a hundred files and trying to update the preview pane. A key push such as 1000j was already crashing. Now you can easily use command counts up to millions for most commands in the list.

I have also added a key accumulator field to the ruler on the right side of the bottom statline. It also shows you command counts so accidental pushes are more obvious.

I'm closing this issue now.

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

No branches or pull requests

3 participants