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

Required String arugments with stdin handling enabled block --help #2868

Closed
Kubuxu opened this issue Jun 18, 2016 · 6 comments
Closed

Required String arugments with stdin handling enabled block --help #2868

Kubuxu opened this issue Jun 18, 2016 · 6 comments
Labels
kind/bug A bug in existing code (including security flaws) regression status/in-progress In progress

Comments

@Kubuxu
Copy link
Member

Kubuxu commented Jun 18, 2016

Introduced in: #2822

As the string arguments from stdin are resolved before the command is run it halts execution of the command and waits for the input.

The simplest and the most sane solution would be to remove stdin from String arguments.
All Unix based system all include xargs command that does exactly that and current behaviour of ipfs command rises more confusion than it is useful.

My proposition would be to remove EnableStdin from non file type arguments and also log it/crash as an error in command creation routines.

@Kubuxu Kubuxu added kind/bug A bug in existing code (including security flaws) regression labels Jun 18, 2016
@Kubuxu Kubuxu added the topic/discovery Topic discovery label Jun 18, 2016
@whyrusleeping
Copy link
Member

@Kubuxu which commands does this affect?

@Kubuxu
Copy link
Member Author

Kubuxu commented Jun 20, 2016

Many of them, there are about 30 StringArgs with Stdin handling enabled.

@whyrusleeping
Copy link
Member

hrm... I like being able to pipe arguments directly to the commands. Maybe theres a way to defer the processing of the arguments until later?

@Kubuxu
Copy link
Member Author

Kubuxu commented Jun 20, 2016

Not without introducing terminal vs pipe inconsistency again: #2877

I didn't know that you could directly pipe into commands for quite some time.
Using xargs is just natural.

@Kubuxu Kubuxu added this to the Ipfs 0.4.3 milestone Jun 21, 2016
@Kubuxu Kubuxu added status/in-progress In progress and removed topic/discovery Topic discovery labels Jun 24, 2016
@whyrusleeping
Copy link
Member

@Kubuxu is this resolved? If so, can you PR in a test that just runs ipfs block get --help and makes sure it returns?

@Kubuxu
Copy link
Member Author

Kubuxu commented Jun 28, 2016

#2913

@Kubuxu Kubuxu closed this as completed Jun 28, 2016
Kubuxu added a commit that referenced this issue Jun 28, 2016
License: MIT
Signed-off-by: Jakub Sztandera <[email protected]>
whyrusleeping added a commit that referenced this issue Jun 28, 2016
sharness: Tests for #2868 - --help hanging on stdin
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
kind/bug A bug in existing code (including security flaws) regression status/in-progress In progress
Projects
None yet
Development

No branches or pull requests

2 participants