Update Unshaky/ShakyPressPreventer.m #19

ghost711 · 2018-11-04T21:56:36Z

More targeted approach for CMD+space issue after more investigation. Wrote up details of findings in comment.

More targeted approach CMD+space issue after more investigation. Wrote up details of findings in comment.

aahung · 2018-11-05T18:13:24Z

Thanks for this PR, it feels like the only solution to go for.

aahung

Thanks a million for this PR. I started couple of conversations you can find below. What do you think of them?

aahung · 2018-11-05T18:17:15Z

Unshaky/ShakyPressPreventer.m


    // The incoming keycode.
    CGKeyCode keyCode = (CGKeyCode)CGEventGetIntegerValueField(event, kCGKeyboardEventKeycode);

    // ignore unconfigured keys
    if (keyDelays[keyCode] == 0) return event;
+
+    // keyboard type, dismiss if it is not built-in keyboard
+    if (ignoreExternalKeyboard) {


I do not understand why we need to move ignoreExternalKeyboard here. If the user types on an external keyboard, we do not have to extract the keyCode at all.

Makes sense. I figured that the minority of people would be using this with an external keyboard, since the butterfly keyboard's issues are the main reason to use it, so I figured that would be the earlier exit. Maybe not though.

I use MBP with external monitor and keyboard at office but Unshaky is always running. So this is useful option.

aahung · 2018-11-05T18:17:44Z

Unshaky/ShakyPressPreventer.m

@@ -12,7 +12,7 @@
 @implementation ShakyPressPreventer {
    NSTimeInterval lastPressedTimestamps[128];
    CGEventType lastPressedEventTypes[128];
-    CGEventFlags lastEventFlagsAboutModifierKeys[128];
+    //CGEventFlags lastEventFlagsAboutModifierKeys[128];


just delete it

My mistake, forgot to do that before pushing.

aahung · 2018-11-05T18:18:11Z

Unshaky/ShakyPressPreventer.m

-        int64_t type = CGEventGetIntegerValueField(event, kCGKeyboardEventKeyboardType);
-        if (type != 58) return event;
-    }
+/** Current version (for fork/pull request): */


we do not need this line either.

Yeah, just forgot to delete again.

aahung · 2018-11-05T18:18:47Z

Unshaky/ShakyPressPreventer.m


+    double      stamp     = [[NSDate date] timeIntervalSince1970];


can you change this variable to currentTimestamp?

Sure, go ahead.

aahung · 2018-11-05T18:19:08Z

Unshaky/ShakyPressPreventer.m


+    double      stamp     = [[NSDate date] timeIntervalSince1970];
+    double      lastStamp = lastPressedTimestamps[keyCode];


can you change this variable to lastTimestamp, what do you think?

Sure, go ahead.

aahung · 2018-11-05T18:20:03Z

Unshaky/ShakyPressPreventer.m

-             interfere in such case. So I add this following checking */
-            && lastEventFlagsAboutModifierKeys[keyCode] == eventFlagsAboutModifierKeys
-            && 1000 * ([[NSDate date] timeIntervalSince1970] - lastPressedTimestamps[keyCode]) < keyDelays[keyCode]) {
+        if (eventType == kCGEventKeyDown && lastPressedEventTypes[keyCode] == kCGEventKeyUp ) {


Just a comment about the styling, I found there is an extra space before the closing ).

Cool. I remembered to eliminate the first space since you mentioned not liking it earlier, but I missed that one.

aahung · 2018-11-05T18:24:25Z

Unshaky/ShakyPressPreventer.m


-    if (lastPressedTimestamps[keyCode] != 0.0) {
+    if (lastStamp != 0.0) {
        if (dismissNextEvent[keyCode]) {


Can you move the double stamp = [[NSDate date] timeIntervalSince1970]; here? Since this value is not needed until here, and for more key press event, this variable is not required.

Are you sure about that? Looks to me like it's determined at the last possible place before it's used.

aahung · 2018-11-05T18:30:37Z

Unshaky/ShakyPressPreventer.m

@@ -57,58 +57,73 @@ - (void)loadIgnoreExternalKeyboard {
    ignoreExternalKeyboard = [defaults boolForKey:@"ignoreExternalKeyboard"]; // default No
 }

- (CGEventRef)filterShakyPressEvent:(CGEventRef)event {


Also, I don't see why this line has been changed...

Has it even been changed? I don't understand why it's been flagged.

ghost711 · 2018-11-06T04:45:00Z

Hi, definitely most thanks are due to you for creating this, so thanks again.

I replied to all of your points, but I'm not familiar with the GitHub interface for this, so please feel free to just implement any of those changes as you see fit.

Most of them were just copying errors on my part with the exception of the one about performing the timestamp calculation later, which I believe is already done at the latest possible point.

aahung · 2018-11-06T04:52:13Z

It just came to my mind that there is another scenario, which I think makes remembering lastMask necessary

The issue is with space key. CMD+SPACE is likely to be triggered multiple times if Unshaky chooses not to do anything...

aahung · 2018-11-19T23:30:54Z

Since you have not yet responded to change requests and I plan to implement an improved version directly. So I am gonna close this PR, thanks!

Add a workaround for #1, improved on #19

Update Unshaky/ShakyPressPreventer.m

c506b9c

More targeted approach CMD+space issue after more investigation. Wrote up details of findings in comment.

aahung requested changes Nov 5, 2018

View reviewed changes

aahung mentioned this pull request Nov 5, 2018

Update Unshaky/ShakyPressPreventer.m #18

Closed

aahung reviewed Nov 5, 2018

View reviewed changes

aahung mentioned this pull request Nov 19, 2018

When is active, the Spotlight shortcut stopped working #1

Closed

aahung closed this Nov 19, 2018

aahung added a commit that referenced this pull request Nov 20, 2018

Add a workaround for #1, improved on #19

9e63a77

aahung added a commit that referenced this pull request Nov 20, 2018

Add a workaround for #1, improved on #19

17c7dea

aahung added a commit that referenced this pull request Nov 20, 2018

Add a workaround for #1, improved on #19

ad18c59

aahung added a commit that referenced this pull request Nov 20, 2018

Merge pull request #21 from aahung/fix/issue-1

9294ed7

Add a workaround for #1, improved on #19

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Update Unshaky/ShakyPressPreventer.m #19

Update Unshaky/ShakyPressPreventer.m #19

ghost711 commented Nov 4, 2018

aahung commented Nov 5, 2018

aahung left a comment

aahung Nov 5, 2018

ghost711 Nov 6, 2018

anfomin Nov 20, 2018

aahung Nov 5, 2018

ghost711 Nov 6, 2018

aahung Nov 5, 2018

ghost711 Nov 6, 2018

aahung Nov 5, 2018 •

edited

Loading

ghost711 Nov 6, 2018

aahung Nov 5, 2018 •

edited

Loading

ghost711 Nov 6, 2018

aahung Nov 5, 2018

ghost711 Nov 6, 2018

aahung Nov 5, 2018

ghost711 Nov 6, 2018

aahung Nov 5, 2018

ghost711 Nov 6, 2018

ghost711 commented Nov 6, 2018

aahung commented Nov 6, 2018

aahung commented Nov 19, 2018


		double stamp = [[NSDate date] timeIntervalSince1970];
		double lastStamp = lastPressedTimestamps[keyCode];

Update Unshaky/ShakyPressPreventer.m #19

Update Unshaky/ShakyPressPreventer.m #19

Conversation

ghost711 commented Nov 4, 2018

aahung commented Nov 5, 2018

aahung left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

aahung Nov 5, 2018 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

aahung Nov 5, 2018 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

ghost711 commented Nov 6, 2018

aahung commented Nov 6, 2018

aahung commented Nov 19, 2018

aahung Nov 5, 2018 •

edited

Loading

aahung Nov 5, 2018 •

edited

Loading