search

emacsorphanage / key-chord

Map pairs of simultaneously pressed keys to commands
http://emacswiki.org/emacs/download/key-chord.el
116 stars 23 forks source link

Improve interval check method #2

Closed conao3 closed 1 year ago

conao3 commented 4 years ago

Those commit are commit in @zk-phi's fork (repository)

Accoding his blog, this idea is like below.

So what I thought was to reduce the number of false inputs by invoking when "one tempo delay from other keystrokes", instead of just checking for "a short enough interval between two keystrokes".

before

 \ カタカタ /      \ バン /     \ カタカタ /
h o g e h o g e      hj       h o g e h o g e . . .
----------------------------------------------------
                  |< 0.1s|

after

 \ カタカタ /      \ バン /     \ カタカタ /
h o g e h o g e      hj       h o g e h o g e . . .
----------------------------------------------------
                 |< 0.15s|
              |> 0.1s||> 0.25s|
irigone commented 3 years ago

Why don't they accept the patch? Also, is the package unmaintained anymore?

conao3 commented 3 years ago

What do you think about this PR, @tarsius? And are there any potential maintainers?

tarsius commented 3 years ago

What do you think about this PR

Please put some more work into it before asking me to review. It appears that you are making some tweaks and then tweak the same things in the next commit or so. It is not at all obvious to me why you are doing that. Maybe there are reasons for doing that; if so, then please document them.

conao3 commented 3 years ago

Fix commit history and force pushed.

This PR was originally a separate fork by zk-phi, which he continued to develop as a fork, not to create a such PR. It was my mistake to make it into a PR as it is. I apologize for the hassle and thanks for the good review.

tarsius commented 3 years ago

I actually wanted to try this package for a long time and now I finally have.

I also cleaned it. Please rebase this pr on top of that cleanup. Sorry for the additional work this causes.

This PR was originally a separate fork by zk-phi, which he continued to develop as a fork, not to create a such PR. It was my mistake to make it into a PR as it is. I apologize for the hassle and thanks for the good review.

No problem. I didn't realize what was going on.

So I assume you tried this for a while. In your experience, how (much) doee it improve the user experience? I don't have enough experience using this package even without this change, so I intend to test both approaches for a while before making a decision.

conao3 commented 3 years ago

rebase done.

To be honest, I'm not an enthusiastic fan of this package, but I thought zk-phi's suggestion made sense, so I suggested this PR. (When he wrote the blog, upstream was already inactive and he was wondering where to send the patch)

It is reasonable to assume that the "key-chord decision" will be a little slower than the surrounding input speed, and I feel that this change has reduced the number of unintended command executions.

tarsius commented 1 year ago

Finally merged.

conao3 commented 1 year ago

thanks!

BlueDrink9 commented 10 months ago

@tarsius would you consider reverting this merge?

New issues created by merge

This has introduced #6, #7 and #8. ghost-of-freedom makes what I think is a good point, albeit brusquely: Neither author nor merger uses this package much from what we can read here (correct me if this is wrong), whereas at least 11 people that do use it have been significantly enough disrupted by this PR to come here and mention it.

No old issues solved by merge

This repo had sat for 4 years without accruing many issue reports. Two issues are referenced as being fixed by this PR, of which neither are actually solved by it. #5 had an existing correct solution found by the author (but they never closed the issue). #4 seems unrelated, presumably closed by mistake.

tarsius commented 10 months ago

I agree. Sorry about that.