Update Unshaky/ShakyPressPreventer.m

[ghost711]

Hi, this appears to fix the issue with the command key.
Thanks a ton for creating this by the way.
I was going nuts with a double space-bar problem.

[aahung]

This is great, finally an answer to this long-lived issue #1.

In this PR, you prevent Unshaky from dismissing the event when command mask is on. I think I might need to modify a little bit.

Image this scenario:

Scenario: my v key is in trouble, which means my CMD+v (paste) could be registered twice, so I want Unshaky to dismiss the second hit.

I would suggest: If the first and second have different masks, disable Unshaky. Otherwise, let it function as normal. What do you think?

[aahung]

Your PR is great, but I cannot merge it right now, since it contains changes not strictly relevant to this topic (some refactoring is really nice), and also the code style is not consistent with what we have right now (e.g. ( something ) instead of (something)). I will try to do some modifications based on your commit.

[aahung]

Can you try this one in dev branch? I simplified some changes as well. @ghost711

https://github.com/aahung/Unshaky/tree/dev

[ghost711]

Sorry, this is actually my first time contributing to a GitHub project.
I modified the project heavily for my own use, but tried to change the minimal amount for the pull request version.
The few changes that I let through that weren't strictly about the issue at hand were optimizations that seemed pretty important for efficiency, since this is running on each and every keystroke. Simple things like exiting the method as early as possible, or consolidating redundant calls.
I'm not quite clear on what you mean by "try this one in dev branch." Do you mean resubmit the pull request to that branch or something? When I click on the link and look at the code it appears to be the same as the master branch.

[ghost711]

Oh, nevermind, I see the edits now. I'll try it on my computer.

[aahung]

Totally agree, these are nice changes and they should go into this repo. Since your single commit contains several independent changes, I cannot pick which one to adopt.

I made one implementation on dev branch based on your codes, but deal with this extra scenario

Scenario: my v key is in trouble, which means my CMD+v (paste) could be registered twice, so I want Unshaky to dismiss the second hit.

[aahung]

Really appreciate your suggestion. The latest two commits here are about your refactoring suggestion. https://github.com/aahung/Unshaky/commits/dev feel free to refactor more if you want to. Thanks!

[ghost711]

Hmm... Your version seems to cover the scenario you mentioned (simulated by setting V key to 200 ms and rapidly hitting CMD+v), but they don't seem to fix the CMD+space issue.

The weird thing is that the double event only seems to happen with CMD+space key, not with CMD+(anything else), or (any other modifier key)+space. It must be tied to the spotlight shortcut somehow.

It also looks like it won't work to cover the scenario you mentioned as well as this issue with the same code, because it appears critical to NOT dimiss the 2nd (duplicate) event generated by CMD+space, but it will get dismissed with the current version because its flags don't match the previous event.

This is what my modified log looks like with the current version during a CMD+space (with the keyboard not causing any problems itself, but the spotlight issue present):

  77447  Key(49)   Event: U, DISMISSING UP
  77446  Key(49)   Event: D, DISMISSING DOWN: 1ms
  77445  Key(49)   Event: U, flags: 0x100000
  77381  Key(49)   Event: D, flags: 0x100000

And this is what it looks like when it's working (with my earlier code):

  75   Key(49)   Event: U, flags: 0x100000 
  74   Key(49)   Event: D, flags: 0x100000, NOT DISMISSING DOWN (CMD Pressed): 2ms
  72   Key(49)   Event: U, flags: 0x0
  0    Key(49)   Event: D, flags: 0x100000

[ghost711]

It looks like my initial comment wasn't quite right when I wrote:

 /** The command key was pressed, which will cause the event to be reported twice
   in rapid succession (first without the modifier flag, and then again with it). */

Your most recent edit would work if that was the case.
Instead, it looks like CMD+space does this if the CMD key is released first (which both versions can handle):

  1477886  Key(49)  Event: U, flags: 0x100000 
  1477885  Key(49)  Event: D, flags: 0x100000, NOT DISMISSING DOWN (CMD Pressed): 1ms
  1477883  Key(49)  Event: U, flags: 0x0
  1477677  Key(49)  Event: D, flags: 0x100000 

And like this if the space bar is released first (which won't work with the current version):

  1594394  Key(49)  Event: U, flags: 0x100000 
  1594393  Key(49)  Event: D, flags: 0x100000, NOT DISMISSING DOWN (CMD Pressed): 0ms
  1594392  Key(49)  Event: U, flags: 0x100000
  1594300  Key(49)  Event: D, flags: 0x100000

[ghost711]

I submitted a new pull request that's just like my earlier one, except it only modifies normal operation for the case of CMD+space.
I wrote up my findings from this thread in the comment for the section:

            if( (keyCode == 49) && (CGEventGetFlags(event) & kCGEventFlagMaskCommand) ) {
                /** CMD+Space was pressed, which causes a duplicate pair of down/up
                 keyEvents to occur 1-5 msecs after the "real" pair of events.
                 - If the CMD key is released first, it will look like:
                        CMD+Space Down
                            Space Up
                        CMD+Space Down
                        CMD+Space Up
                 - Whereas if the space bar is released first, it will be:
                        CMD+Space Down
                        CMD+Space Up
                        CMD+Space Down
                        CMD+Space Up
                 - The issue only appears to happen with CMD+Space,
                   not CMD+<any other key>, or <any other modifier key>+Space.
                 - So it looks like the simplest thing is to just ignore if both
                   CMD and space are pressed at the same time. */

[aahung]

Closed due to a newer PR #19