bug: cmdline args need jsonification; tty support needs adjustment #27
Conversation
joesuf4
commented
May 29, 2021
joesuf4
commented
- switch shell to bash so we can use arrays and bulk element transforms
- use [ -t 0 ] to baseline whether we need terminal support for the invocation
joesuf4
force-pushed
the
jsonify-entrypoint
branch
2 times, most recently
from
3f4b412 to
8fca66b
Compare
- switch shell to bash so we can use arrays and bulk element transforms - use [ -t 0 ] to baseline whether we need terminal support for the invocation - send human-interest commentary to stderr instead of stdout
joesuf4
force-pushed
the
jsonify-entrypoint
branch
from
8fca66b to
9df9374
Compare
|
The motivation for this patch is that I want to use node-shell in CLI pipelines, so I am not interested in the default tty login shell API. I don't want "stuff" printed to stdout unless I'm expecting it from my CLI invocation, so I redirect that output to stderr. Also I have embedded newlines and double-quote characters in some of my CLI tooling that wraps node-shell, so I need better CLI->json-string translation than before I needed now. |
|
Hi thank you for your PR, I'll review it in a while 👍 |
There was a problem hiding this comment.
Hi @joesuf4, sorry for late reply.
Your PR adds two new dependencies bash and sed.
I agree that using bash arrays makes it more convenient, but the original goal for this plugin was to keep it simplest and compatible with all possible architectures and any shell.
I believe that there is no reasons to upgrade shell version. Please take a look on these changes, I think I was able to make your changes compatible with standard bourne shell
|
Ok. |
|
Fighting with GitHub MR ui. Bear with me. |
|
Merged in |
