-
-
Notifications
You must be signed in to change notification settings - Fork 28
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
[WIP] Only wait for stdin for a little bit #19
Conversation
For this module, we only really care about stdin that is written right away, like when the process is piped. This has the benefit of letting us easily add a fallback when there's no stdin. Fixes #13
Not sure what you're trying to achieve with this. Whenever I see timeouts I'm always skeptical. Stdin can block waiting for the source stream to actually write. That should be intended. Even if your program doesn't receive input for a while, that's usually fine - normally a shell will start every process in a chain right as the command is run. For example: $ cat /dev/urandom | gzip | cat In the above, even before the first What's the problem that is trying to be fixed? This PR is kind of trying to solve a problem that isn't really a problem - stdin is a stream, not a body of data. There are use cases, of course, for buffering the entirety of stdin and then operating on its contents as a whole, but timing out is going cause massive confusion for those that are expecting a command line interface to behave in a normal way. Two examples of commonly used utilities that buffers all input before creating output are If you do $ cat /dev/urandom | sort it'll never return nor will it output anything - as any normal user would expect (though it might error out given that the input sequence isn't valid, but that's irrelevant). I am against this PR full-heartedly. I think this solves "confusion" in extremely rare and uncalled for scenarios. |
I'm not sure this is a good idea, what if I had an executable that outputs some data from a sensor on change. Then I pipe that output into a node script and handle it with Not sure how common this behaviour would be. Might be worth releasing as a major change or might cause too many problems to be worth it. |
Yeah, I don't think this is really a good idea. I'd make the timeout user-configurable with a default value of |
I agree that if it's added, then at the most we should do what @silverwind suggests. I'd go so far as to say this would be misused by those that don't understand stdin very well, ultimately causing bugs even they don't understand. |
So I've been researching this and I think the core issue is that a script like const rs = process.stdin;
rs.on('data', () => console.log('data', data));
rs.on('end', () => console.log('end'));
rs.resume(); does not receive a I understand that the stdio streams are kind of neverending by design, but I'd be interested in how other languages solve this dilemma. I see that Python has |
For this module, we only really care about stdin that is written right away, like when the process is piped.
This has the benefit of letting us easily add a fallback when there's no stdin.
Fixes #13
I'm not sure this is the right solution, but according to the internet there's no other way than setting a timeout. What should the timeout value be? I have no idea, 100ms seems ok though. @Qix- What do you think?
// @silverwind @SamVerschueren @lukechilds