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

Bug when typing "s"/"d" with another character in rapid succession #40

Closed
olliecheng opened this issue Jun 11, 2018 · 9 comments
Closed

Comments

@olliecheng
Copy link

olliecheng commented Jun 11, 2018

Steps to reproduce: Type "s" or "d" followed by another character in rapid succession (< MAX_TIME_BETWEEN_SIMULTANEOUS_KEY_PRESSES), such as se really fast. Instead of inserting se, es is inserted.
Running on macOS 10.13.5 with Hammerspoon 0.9.66. This issue has been consistently reproduced with Hammerspoon running, and has been consistently absent with Hammerspoon closed.

@olliecheng olliecheng changed the title Bug when typing " Bug when typing "s"/"d" with another character in rapid succession Jun 11, 2018
@olliecheng
Copy link
Author

This issue is resolved by pull request #41, which will merely wait for a key press during the interval in which the program checks for the other key which composes the Super Duper shortcut to be pressed. Afterwards, it inserts both characters in correct succession.

@olliecheng
Copy link
Author

Friendly ping?

@jasonrudolph
Copy link
Owner

jasonrudolph commented Jun 20, 2018

@denosawr: Thanks for logging this issue, and thanks for the proposed fix in #41! It might be a while before I can properly review that pull request, but it's on my list. 🙂

@olliecheng
Copy link
Author

Awesome, good to know it's been acknowledged!

@rickharris
Copy link

If there's anything I can do to help with this, I'd be glad to. Been using this for a few years and love it! But every day there's enough out-of-order "s" and "d" characters to make me want to disable super-duper mode.

Is the linked PR a good starting point, or do other options need to be explored?

@jasonrudolph
Copy link
Owner

Been using this for a few years and love it!

@rickharris: Awesome! I'm so glad to hear that! 💟

Is the linked PR a good starting point, or do other options need to be explored?

#41 will probably work for you most of the time, but it's susceptible to a race condition as described in #41 (comment). With a super short timeout, you probably won't run into the problems caused by the race condition, but you might.

Due to the potential for the race condition, I'm not comfortable merging #41 in its current state. But if it you want to give it a shot, you can use the fork at https://github.com/denosawr/keyboard or apply 111b1f3 to your local clone of https://github.com/jasonrudolph/keyboard.

I hope that context is helpful.

@olliecheng
Copy link
Author

Okay, so setting aside the... uh, issue of me accidentially closing the PR, I have a new commit which seems to fix the race condition, as well as addressing some issues the previous implementation caused (namely inability to use s/d + special keys like space). On my end, this PR seems to work.

Note that I have found instances where I type "s" and then "e" and "es" is typed. However, using if you log the calls, it turns out that this happens only when the "e" is typed just after the timeout. I can't really think of a way to address this well, and it should only show up in a very short time window, very rarely. If anyone has any ideas, that'd be awesome! :)

@acheronfail
Copy link

In order to fix this problem, I moved the trigger out of Hammerspoon and into Karabiner, and it works much, much better in my opinion.

Basically, remove the triggers in super duper mode in Hammerspoon, and expose a way to toggle super duper mode via the command line (see https://www.hammerspoon.org/docs/hs.ipc.html#cliInstall).

I did it by exporting the superDuperMode table which contains the superDuperMode:enter() and superDuperMode:reset() functions:

-- ... the rest of the super duper module ...

-- Expose superDuperMode so we can control it from other modules (and the CLI).
return superDuperMode

Then, add a complex modification like this to karabiner:

{
    "manipulators": [
        {
            "description": "(S)uper (D)uper mode trigger",
            "from": {
                "simultaneous": [
                    {
                        "key_code": "s"
                    },
                    {
                        "key_code": "d"
                    }
                ],
                "simultaneous_options": {
                    "to_after_key_up": [
                        {
                            "shell_command": "/usr/local/bin/hs -c 'superDuperMode:reset()'"
                        }
                    ]
                }
            },
            "parameters": {
                "basic.simultaneous_threshold_milliseconds": 40
            },
            "to": [
                {
                    "repeat": false,
                    "shell_command": "/usr/local/bin/hs -c 'superDuperMode:enter()'"
                }
            ],
            "type": "basic"
        }
    ]
}

That will toggle the mode if both S and D are pressed, and Karabiner is much better at timing the events and making sure everything works as expected.

@jasonrudolph
Copy link
Owner

This is likely resolved by #61, so I'm going to close this issue.

If you're still able to reproduce the problem using the latest code on the master branch, please let me know, and I'll re-open this issue. 🙇

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

Successfully merging a pull request may close this issue.

4 participants