-
Notifications
You must be signed in to change notification settings - Fork 22
allow users to config the escape taptime #16
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
base: main
Are you sure you want to change the base?
Conversation
jasonrudolph
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for this thoughtful pull request, and for taking the time to update the docs as well. When you have a moment, please see the question below.
init.lua
Outdated
| self.controlKeyTimer = hs.timer.delayed.new(CANCEL_DELAY_SECONDS, function() | ||
| self.controlKeyTimer = hs.timer.delayed.new(self.tapTime, function() |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
👋 @quangd42: I tried testing this locally, but it seems to always use the default value of 150 ms, even if I follow the updated instructions in the README.
I have a feeling that this is due to hs.timer.delayed.new(self.tapTime, ...) being called in obj:init() at initialization time, and therefore any changes made to self.tapTime after object initialization have no effect on the timer. That's my best guess at least.
If you're able to get this pull request working as desired and you can describe how to verify the new behavior, I'm happy to take another look.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for looking at this. To be honest I was not so sure how to verify the delay amount of the controlKeyTimer, as hs.timer does keep an internal state for the delay amount.
So I ended up moving the initialization of the timer to the start method so it's clear what value of CANCEL_DELAY is being passed to the timer, and it only happens once. Hopefully this is straightforward enough to eliminate guess work.
Thanks!
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
👋 Hi @quangd42 - I've checked out the latest updates. They do look like they should work, but I still haven't been able to definitively verify that they work. Before accepting any changes, it's important to me that I'm able to verify the new behavior. Can you describe a process for manually testing this behavior to verify that it's working as you expect?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hi @jasonrudolph - In my latest commit you can find a few commented lines of code that will log elapsed time since the timer starts until either 'escape' is sent or the timer is canceled, using https://www.hammerspoon.org/docs/hs.timer.html#absoluteTime. I pushed the code as comments so that the PR can be merged as is and avoid the extra overhead.
Here are the steps:
- Uncomment the logging code.
- In init.lua, call start(200) on the module. Open hammerspoon console and reload config.
- Hold 'control' key. Expect to see 'timer canceled after {time} ms' in the console, where time is approximately 200.
- Press and quickly release 'control' key. Expect to see 'escape sent after {time} ms' in the console, where time is less than 200.
Let me know if that will work.
This allows Hammerspoon users to update the
escapetap time in their config without meddling with the code of the Spoon itself