Adds optional permission timeout #35
Conversation
|
Why? |
|
To give tools implementing this module the option to gracefully allow their users to skip the permission prompt instead of blocking indefinitely. |
|
It only asks when interactive, meaning when a user is executing it. When non-interactive it will gracefully not show => https://github.com/yeoman/insight/blob/17c012e6811e50b9b04340e5a09adedaf109fc05/lib/index.js#L104 You can also use the Why exactly does this not work for you? |
|
Because checking for interactive doesn't catch all non-user-executed cases, as seen in common CI tools such as Bower. |
Cases like? Please provide an actual example. This might be useful, but I'm not going to add something until I see actual use-cases, not just your words. |
|
An example can be seen here: bower/bower#1102 I know that users could just call the script with the interactive=false flag, but as seen in the issue and the search for "bower install" in GitHub repositories, a lot of users don't do this. |
|
Alright. Not interested in an option, but we can add a default timeout of 30 seconds. We can also a check for |
mex
force-pushed
the
feature/permission-timeout
branch
from
c1915f4 to
30e8aed
Compare
|
I've removed the option for the timeout, defaulted to 30 and added test for this. I also added a |
| @@ -38,6 +38,8 @@ function Insight (options) { | |||
| clientId: options.clientId || Math.floor(Date.now() * Math.random()) | |||
| }); | |||
| this._queue = {}; | |||
|
|
|||
| this.permissionTimeout = 30; | |||
There was a problem hiding this comment.
this._permissionTimeout = 30;
mex
force-pushed
the
feature/permission-timeout
branch
from
30e8aed to
c5042e7
Compare
Adds optional permission timeout
|
Looks good. Thanks :) |
|
Great! Can you ping me, when you bump the version? |
|
Done |
|
Awesome, thanks for the prompt reaction! |
|
@mex Doesn't it trigger callback two times if user answered before timeout? |
|
@sheerun No, because we clear the timeout, when an answer is given: https://github.com/yeoman/insight/blob/0c2be5f9ea1bf927691adb87a33c4058924f9a17/lib/index.js#L120 |
|
Oh, right :) |
Adds the option to pass along
permissionTimeoutduring init, which will let the permission prompt fall back to opt out after X seconds.