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

Fix the 'delay' option for hotkey events. #1201

Merged
merged 6 commits into from Dec 13, 2012

Conversation

@pjrobertson
Copy link
Member

@pjrobertson pjrobertson commented Nov 10, 2012

I've never used this before, but the 'delay' option wasn't working for me (10.8, that is). Looking at the code, I don't think it should have ever worked :/

Also: if the hotkey conditions aren't met (e.g. not long enough delay, no repeats etc.) pass the keypress down to the frontmost app.

Scenario: I have (like many of us) multi stroke key bindings set up. ⌃M followed by ⌃D gives you the '⌫' key. Handy :)
But I have ⌃D bound to 'Open Downloads folder' as a trigger in Quicksilver.
What I want is to be able to press and hold the ⌃D for 0.5s to open the Downloads folder. Otherwise, send the key press down the line.

… and that's what I've done :)

pjrobertson added 6 commits Nov 10, 2012
I've never used this before, but it wasn't working for me.
Also: if the hotkey conditions aren't met (e.g. not long enough delay, no repeats etc.) pass the keypress down to the frontmost app.
If the 'repeat' interval isn't set, don't assume it's 0s! (Show a notif instead)
Make sure repeat triggers don't continue ad infinitum. Fixes #583
…s are active, pass the keypress down to the next app
@pjrobertson
Copy link
Member Author

@pjrobertson pjrobertson commented Nov 11, 2012

Meh, I thought that tiny fix was a bit lame. So I did some more :)

I've now made it possible to create triggers with the same hotkey, but scoped to different apps :)
There's currently no way of validating scoping (e.g. making sure you haven't scoped the same trigger to the same app more than once) but I think people messing with scoping and multiple triggers should be able to handle this.

Now, you can have triggers as follows:

⌃�D Open Downloads (Enabled in Safari)
⌃D Open Documents (Enabled in Microsoft Word)
⌃D Display "DDDDD" as large type (Disabled in Safari, Microsoft Word)

etc. etc. Options are endless :)

@pjrobertson

This comment has been minimized.

Copy link
Owner Author

@pjrobertson pjrobertson commented on 68a7684 Nov 11, 2012

This commit looks scary. All I did was add a for() loop to iterate over the triggers, and then re-indent everything

@skurfer
Copy link
Member

@skurfer skurfer commented Nov 12, 2012

Well, speaking of the Show Window feature: I just happened to be testing the delay setting on a trigger where I had previously enable Show window, and it turns out that the Show window pref makes it ignore the delay and run immediately (or close to it - if I tap the key quickly, the window starts to zoom in, but the action doesn't run).

The same-trigger-in-different-scopes thing is awesome (and seems to work in my testing). A lot of people are going to like that. :-)

@pjrobertson
Copy link
Member Author

@pjrobertson pjrobertson commented Nov 12, 2012

The same-trigger-in-different-scopes thing is awesome (and seems to work
in my testing). A lot of people are going to like that. :-)

Great :)

I just happened to be testing the delay setting on a trigger where I had
previously enable Show window, and it turns out that the Show window pref
makes it ignore the delay and run immediately

Yay, looks like another bug in OS X :)
http://www.cocoabuilder.com/archive/cocoa/323690-strange-event-result.html

I've kind of gone off the idea of hiding the window after a delay, since
its effect is different depending on whether or not the hotkey was
successful. It if wasn't, it shrinks, if it was, it 'explodes'. Nice touch.

On 12 November 2012 19:44, Rob McBroom notifications@github.com wrote:

Well, speaking of the Show Window feature: I just happened to be testing
the delay setting on a trigger where I had previously enable Show window,
and it turns out that the Show window pref makes it ignore the delay and
run immediately (or close to it - if I tap the key quickly, the window
starts to zoom in, but the action doesn't run).

The same-trigger-in-different-scopes thing is awesome (and seems to work
in my testing). A lot of people are going to like that. :-)


Reply to this email directly or view it on GitHubhttps://github.com//pull/1201#issuecomment-10301353.

@HenningJ
Copy link
Member

@HenningJ HenningJ commented Nov 12, 2012

Just wanted to mention #914. :-)

@pjrobertson
Copy link
Member Author

@pjrobertson pjrobertson commented Nov 12, 2012

Not sure if mention is enough? Do you have to say either closes #914 or
fixes #914 ? :P

I've pushed a couple of commits that fix a few things… going as far as I
think I can with this. With a delay enabled, changing the modifier keys
cancels it, but a key up doesn't (that's why I think it's an Apple bug)

On 12 November 2012 21:21, Henning Jungkurth notifications@github.comwrote:

Just wanted to mention #914#914.
:-)


Reply to this email directly or view it on GitHubhttps://github.com//pull/1201#issuecomment-10304833.

@skurfer
Copy link
Member

@skurfer skurfer commented Nov 13, 2012

Not sure if mention is enough? Do you have to say either closes #914 or fixes #914?

Either should work in a commit message. Neither will work in these comments. :-) But we can just close it manually when this is merged.

With a delay enabled, changing the modifier keys cancels it, but a key up doesn't (that's why I think it's an Apple bug)

It's an edge case. Best effort is good enough here, I think.

@skurfer
Copy link
Member

@skurfer skurfer commented Nov 21, 2012

I could have sworn this was working before, and it was. Just did a git bisect and it turns out the problem was introduced in e39dc5e0594bbed9d92bbb1f536ceba77bfddd92.

I have the Increase and Decrease Volume triggers from iTunes set to "On Press" and "Repeat Every 0.2s". Nothing else enabled. They were working as expected prior to that commit, but now, if I use one, the volume just goes up or down forever and Quicksilver becomes unresponsive.

@skurfer
Copy link
Member

@skurfer skurfer commented Dec 13, 2012

This has been working well for me (as long as I revert the last commit). What problem was it meant to solve?

skurfer added a commit that referenced this pull request Dec 13, 2012
Fix the 'delay' option for hotkey events.
@skurfer skurfer merged commit f534d26 into quicksilver:master Dec 13, 2012
pjrobertson added a commit that referenced this pull request Jan 27, 2013
@pjrobertson pjrobertson deleted the pjrobertson:hotKeyDelays branch Feb 22, 2013
@pjrobertson pjrobertson restored the pjrobertson:hotKeyDelays branch Feb 22, 2013
@pjrobertson pjrobertson deleted the pjrobertson:hotKeyDelays branch Feb 22, 2013
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Linked issues

Successfully merging this pull request may close these issues.

None yet

3 participants
You can’t perform that action at this time.