Add stdin and input handling #22
Conversation
SamVerschueren
force-pushed
the
iss20
branch
from
0a4ed1e
to
0157dc0
Compare
|
Will fix the tests tomorrow. Not much time at the moment. |
|
Can you rebase when you have a chance? |
SamVerschueren
force-pushed
the
iss20
branch
from
bb76539
to
78ddc20
Compare
SamVerschueren
force-pushed
the
iss20
branch
from
78ddc20
to
af1f9dc
Compare
|
I might have done something wrong. I'm not able to squash those commits. The first commit also has the changes of the previous PR (#19 - add type inference option) which I merged from master. You're probably way better in it then me :). |
|
@sindresorhus ping I know you're quite busy lately, just reply when you find the time. |
| showHelp: showHelp, | ||
| stdinOrInput: function () { | ||
| if (!_ && process.stdin.isTTY) { | ||
| return Promise.reject(new Error('input required')); |
|
There are two more use-cases. Just getting stdin without input, and getting stdin or file. Maybe we should also add: |
|
|
||
| cli.stdinOrInput() | ||
| .then(function (input) { | ||
| input.forEach(function (element) { |
There was a problem hiding this comment.
The main use-case is one input argument, so would really like to optimize for that, but still support the edge-case of multiple arguments.
Good point, it makes the module more complex. Not sure what you think about a wrapper like const meowStdin = require('meow-stdin');
const meow = meowStdin(require('meow'));
// and so onOr a separate package that uses meow in the background but adds const meow = require('meow-stdin');
// and so on |
| const input = await execa('./fixture.js', ['foo', 'bar']); | ||
|
|
||
| t.is(stdin.stdout, 'meow\nfoo bar\n'); | ||
| t.is(input.stdout, 'meow\nfoo\nbar'); |
There was a problem hiding this comment.
This isn't how stdin is intended. At least, this is completely counterintuitive for any other program out there. Usually how it works is program file.txt or cat file.txt | program.
This assumes the command line input is instead input as well. This will confuse a ton of people if Meow's code handling assumes this.
There was a problem hiding this comment.
Is there a big difference in echo "foo bar" | cli and cat file.txt | cli where the content of file.txt is foo bar? I assumed, if I test echo "foo bar" | cli, then cat file.txt | cli will work as well?
There was a problem hiding this comment.
That's not what I mean. Usually programs that accept filenames as arguments are the ones that employ the use of stdin.
For instance:
cat > some-file.txt <<EOF
some text
EOF
grep "text" some-file.txt
echo "some text" | grep "text"
sed 's/some/thing/' some-file.txt
echo "some text" | sed 's/some/thing/'Instead, this PR seems to support the usage of:
cat > some-prog.js <<EOF
// ...
console.log(meow.input);
// ...
EOF
some-prog.js This Is Some Text
# doing the same as
echo This Is Some Text | some-prog.jsWhich is going to confuse a lot of people.
There was a problem hiding this comment.
Usually programs that accept filenames as arguments are the ones that employ the use of stdin.
Sure, but I also often use it for CLI's that accept text input too, to make it easier for everyone to use it.
Then they could use either of these:
$ cli-app foo
$ echo foo | cli-app
There was a problem hiding this comment.
Actually, I take back my last comment. Better to use stdin for just string input. No point in doing both, and stdin should be preferred as it works nicely in a pipe chain.
|
|
||
| // get the uncached parent | ||
| delete require.cache[__filename]; | ||
| var parentDir = path.dirname(module.parent.filename); | ||
|
|
||
| global.Promise = Promise; |
There was a problem hiding this comment.
Isn't it against the rules or am I missing something?
There was a problem hiding this comment.
get-stdin does not support 0.10 and thus does not come bundled with pinkie-promise. So either we drop 0.10 in meow, or add pinkie-promise to get-stdin in order to get rid of this.
fixes #20
Feedback is more then welcome.
Note: PR is made from branch iss17.