search

akavel / up

Ultimate Plumber is a tool for writing Linux pipes with instant live preview
Apache License 2.0
8.39k stars 129 forks source link

Add delay to unsafe-full-throttle mode #39

Open pianohacker opened 5 years ago

pianohacker commented 5 years ago

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 commented 5 years ago

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.

pianohacker commented 5 years ago

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

pianohacker commented 5 years ago

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 commented 5 years ago

:heart: 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 commented 5 years ago

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 commented 5 years ago

@roryokane Thanks; there's already a PR at https://github.com/akavel/up/pull/41 however, and I added some initial review comments there. Sorry for not acknowledging this explicitly here earlier!