Change logic of stdin input to be last and only if required #2822
Conversation
Kubuxu
force-pushed
the
feature/stdin-stable
branch
4 times, most recently
from
33f8192
to
3134bff
Compare
Kubuxu
changed the title
|
@Kubuxu could you provide a couple examples of how this changes command line usage for the end user? Does it have any effect on things like |
|
No it doesn't for the most part. It changes behaviour if parameter is optional user as has to explicitly specify that he wants to use stdin using As argument for add is required it will wait for input. I think we should add a message on stderr about it as currently Something along the lines of: @RichardLitt can you check wording of that? |
| # | ||
| # Based on the Bash documentation example. | ||
|
|
||
| # Hello Chet, |
|
Looks fine to me from a wording perspective. Not sure what that shell script is. |
or if user asks for it License: MIT Signed-off-by: Jakub Sztandera <[email protected]>
License: MIT Signed-off-by: Jakub Sztandera <[email protected]>
…fig"" This reverts commit 77ef391. License: MIT Signed-off-by: Jakub Sztandera <[email protected]>
This tests expects ipfs init with no arguments to work even with occuipied stdin. License: MIT Signed-off-by: Jakub Sztandera <[email protected]>
Kubuxu
force-pushed
the
feature/stdin-stable
branch
from
3134bff
to
628e458
Compare
|
tried this out, works good on my end. makes docker a bit happier. Thanks @Kubuxu ! |
| # Upon time-out expiration SIGTERM (15) is sent to the process. If the signal | ||
| # is blocked, then the subsequent SIGKILL (9) terminates it. | ||
| # | ||
| # Based on the Bash documentation example. |
There was a problem hiding this comment.
@Kubuxu did you write this? if not, what is the license?
There was a problem hiding this comment.
It is Public Domain, I had previously credited the original author but removed it with other comments as for @RichardLitt 's request. I shouldn't have removed the author line, it was a signature for a short letter there.
There was a problem hiding this comment.
licensing and author lines need to be back before release.
There was a problem hiding this comment.
This should address the license issue: #3152
|
IIRC, this PR introduced a bunch of complexity and breaking changes. Have these been resolved? if so, where? (this may be ahead in my CR) |
|
Found it #2902 |
|
More here #2952 |
|
would be good to isolate all these commits and see a diff between |
I tried to somewhat group the 0.4.3-rc1 changelog entries logically, but the ones for stdin didn't turn out that well :) some entries are pretty good though and we should continue to aim for properly editorialized changelogs (i.e. logically grouped and structured) |
tl;dr merges don't map to changelog entries |
| @@ -0,0 +1,83 @@ | |||
| #!/bin/bash | |||
There was a problem hiding this comment.
I am a bit concerned that this script is written in bash.
There was a problem hiding this comment.
@chriscool we removed this script in #3152
There was a problem hiding this comment.
Yeah, I saw it afterwards. Thanks for having written something more portable!
This PR changes how stdin is used while parsing arguments. Current behaviour is that stdin is used for non-required and variadic arguments which causes confusion.
After the change the stdin is used only in case of missing required argument or if explicitly mentioned by the user with
-.Question: Should we print on stderr when ipfs command is listening for stdin to clear up possible confusing hang even more?
Aims to resolve #2748
License: MIT
Signed-off-by: Jakub Sztandera [email protected]