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

Add delay to unsafe-full-throttle mode #39

Open
pianohacker opened this issue Dec 2, 2018 · 6 comments
Open

Add delay to unsafe-full-throttle mode #39

pianohacker opened this issue Dec 2, 2018 · 6 comments
Labels
enhancement New feature or request

Comments

@pianohacker
Copy link

The new press-enter-to-run mode makes up a lot safer, but it would still be nice to have something like the old mode. Maybe a new mode where the command is automatically run, but only after a configurable amount of time without any input?

Probably still shouldn't be the default, but it would be a nice in-between option.

(Would happily dive in and make a PR to implement this, if you're interested!)

@akavel
Copy link
Owner

akavel commented Dec 4, 2018

Very interesting idea! Perhaps could be configured with the same flag, just by adding a time value to it, e.g. something like: --unsafe-full-throttle=3s.

One thing I'd like to note up front, so it won't surprise you at later time, is that I'd like most if you could start with a quick prototyping/experimental phase initially. That is, if you would implement it, and then we'd both try to use it once or twice each, just to verify if it feels to us like the feature makes sense in practice. If we reach this point, then whether we decide "go" or "no go", I'd already treat it as an important contribution from you, and I'd like to add your name to the AUTHORS file at this point if you won't mind. Also, for this initial experimental phase, I'm interested in testing the functionality, so you can implement it in any way that's easiest for you, I would not review the code itself yet till this point. After we (most probably) pass a "go", we'd then get back to the code to try and sculpt it (maybe even rewrite) into some (hopefully) reasonably small and innocuous change.

@akavel akavel added the enhancement New feature or request label Dec 4, 2018
@pianohacker
Copy link
Author

Sounds excellent! I'll work on getting a prototype done.

@pianohacker
Copy link
Author

https://github.com/pianohacker/up/tree/autorun-delay-spike

Some less-than-ideal parts, but it's a prototype. There are probably better sentinel values to put in the EventInterrupt, for one.

@akavel
Copy link
Owner

akavel commented Dec 16, 2018

❤️ Can you make it a pull request? Would make it much easier for me to see the differences, and point to specific lines/fragments of code when further discussing with you! Or do you know if there's a way I myself could make a PR from it?

@roryokane
Copy link

If you go to pianohacker’s repository and click GitHub’s “New pull request” button next to the branch selection menu, you can see a preview of the diff without creating a pull request: https://github.com/akavel/up/compare/master...pianohacker:autorun-delay-spike?expand=1. Or you can use just Git, no GitHub needed, by adding someone else’s repo as a remote and then using the git diff command to compare your branch with their branch.

@akavel
Copy link
Owner

akavel commented Mar 9, 2019

@roryokane Thanks; there's already a PR at #41 however, and I added some initial review comments there. Sorry for not acknowledging this explicitly here earlier!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

No branches or pull requests

3 participants